Re: [PATCH] smb/client: fix unaligned fallocate emulation with O_DIRECT
hehuiwen <[email protected]>
| Newsgroups | org.kernel.vger.linux-cifs |
|---|---|
| Message-ID | <[email protected]> |
Thanks for pointing this out. I tested the patch against Windows Server, Samba and ksmbd, but not Azure Files. I will look into this further. Thanks, Huiwen 在 2026/7/16 23:09, Paulo Alcantara 写道: > Huiwen He <[email protected]> writes: > >> From: Huiwen He <[email protected]> >> >> Commit 4a7d2729dc99 ("smb: client: fix atomic open with O_DIRECT & O_SYNC") >> made atomic O_DIRECT opens correctly use CREATE_NO_BUFFER. This exposed >> an issue in the fallocate emulation added by >> commit 966a3cb7c7db ("cifs: improve fallocate emulation"), which may >> reuse the handle for unaligned zero writes. >> >> Windows requires the offset and length of writes on an >> unbuffered handle to be sector aligned, and rejects these writes >> with STATUS_INVALID_PARAMETER, causing xfstests generic/760 to fail. >> >> For SMB3.02 and later, fix this by keeping the handle buffered and using >> READ_UNBUFFERED or WRITE_UNBUFFERED only for actual direct I/O requests. >> Determine this from the netfs request origin rather than the file flags, >> since mmap writeback on an O_DIRECT file is still buffered. >> >> For older dialects, retain CREATE_NO_BUFFER since they do not support >> per-I/O unbuffered flags. >> >> With this change, xfstests generic/760 passes against Windows with SMB3.02 >> or later, while Windows SMB3.0 remains affected. >> >> Fixes: 4a7d2729dc99 ("smb: client: fix atomic open with O_DIRECT & O_SYNC") >> Signed-off-by: Huiwen He <[email protected]> >> Reviewed-by: ChenXiaoSong <[email protected]> >> --- >> fs/smb/client/cifsglob.h | 13 +++++++++++-- >> fs/smb/client/dir.c | 2 +- >> fs/smb/client/file.c | 4 ++-- >> fs/smb/client/smb2pdu.c | 6 ++++++ >> 4 files changed, 20 insertions(+), 5 deletions(-) >> >> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h >> index 08e94633a9c1..2e43dbd564de 100644 >> --- a/fs/smb/client/cifsglob.h >> +++ b/fs/smb/client/cifsglob.h >> @@ -2374,17 +2374,26 @@ static inline void cifs_reset_oplock(struct cifsInodeInfo *cinode) >> WRITE_ONCE(cinode->oplock, 0); >> } >> >> +static inline bool >> +cifs_server_supports_per_io_unbuffered(const struct TCP_Server_Info *server) >> +{ >> + return server && server->dialect >= SMB302_PROT_ID; >> +} >> + > > Unfortunately this check is unsufficient to tell whether the server will > actually support SMB2_WRITEFLAG_WRITE_UNBUFFERED and > SMB2_READFLAG_READ_UNBUFFERED flags. > > Have you tested this against Azure server? IIRC, the server would > reject the write and read requests with STATUS_INVALID_PARAMETER if any > of those flags was set. > > I had a patch to use those flags and improve reuse of open handles but > then gave up as Azure server seems to be the only one broken regarding > those flags.