Re: [PATCH] smb/client: fix unaligned fallocate emulation with O_DIRECT

[hehuiwen <huiwen.he@linux.dev>]

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 <huiwen.he@linux.dev> writes:
> 
>> From: Huiwen He <hehuiwen@kylinos.cn>
>>
>> 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 <hehuiwen@kylinos.cn>
>> Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
>> ---
>>   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.