Re: [PATCH v6 1/5] smb/client: refresh allocation size after duplicate extents

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

Hi Steve,

Yes, patch 1 adds one extra roundtrip after each successful 
duplicate-extents request.

I think that query is needed if we want i_blocks to be correct 
immediately after the server-side clone. The client only knows the byte 
range that was cloned. It does not know how many blocks the server 
actually allocated for that range, especially when the cloned range 
contains holes.

I agree this may affect workloads that use cp/reflink heavily, but it is
still one metadata query per successful server-side clone, not extra 
work in the SMB2_READ/SMB2_WRITE path. So I think the cost is acceptable 
for the correctness fix.

I found that the v6 version could cause duplicate revalidation after
the AllocationSize refresh. I updated patch 1 in v7 to keep the 
QueryInfo result valid in the inode cache, so a following stat does not 
immediately query the same attributes again.

I also updated patch 3 based on Shashiko's comment. Small fallocate 
ranges at or beyond EOF are now zero-written directly instead of first 
querying
FSCTL_QUERY_ALLOCATED_RANGES.

Thanks,
Huiwen



在 2026/7/2 05:47, Steve French 写道:
> This does lead to an extra roundtrip (common operations like cp will
> use duplicate extents in some Samba and Windows server
> configurations).  Any worries about performance hit?
> 
> On Wed, Jul 1, 2026 at 10:22 AM Huiwen He <huiwen.he@linux.dev> wrote:
>>
>> From: Huiwen He <hehuiwen@kylinos.cn>
>>
>> FSCTL_DUPLICATE_EXTENTS_TO_FILE changes the target file extents on the
>> server, but the client does not refresh the target AllocationSize/i_blocks.
>> Callers can observe or use the wrong st_blocks value immediately after the
>> clone, before a later attribute revalidation corrects it.
>>
>> For example, create a reflinked file with a leading hole:
>>
>>          xfs_io -f -c "pwrite -S 0x61 0 64k" src
>>          touch dst
>>          chmod 600 dst
>>          xfs_io -c "reflink src 0 1m 64k" dst
>>          mkswap dst
>>          swapon dst
>>
>> The file still has a hole after mkswap:
>>
>>          /mnt/scratch/dst:
>>            [0..7]:       allocated
>>            [8..2047]:    hole
>>            [2048..2175]: allocated
>>
>> The server also reports only the allocated ranges:
>>
>>          server dst size=1114112 blocks=144
>>
>> but the client reported EOF-derived blocks:
>>
>>          client dst size=1114112 blocks=2176
>>
>> and swapon succeeded:
>>
>>          swapon_result=success
>>          /mnt/scratch/dst 1.1M 0B -1
>>
>> So EOF-derived i_blocks can let a sparse reflinked file pass the CIFS
>> swapfile hole check.
>>
>> Fix this by querying FILE_ALL_INFORMATION on the target handle after a
>> successful duplicate extents request and updating i_blocks from the
>> returned AllocationSize. If the query fails, invalidate the cached
>> inode attributes so a later getattr can refresh them.
>>
>> This also fixes the xfstests generic/370 regression introduced by the
>> i_blocks accounting change, as tested on a Samba "vfs objects = btrfs"
>> share.
>>
>> Fixes: 99cd0a6eeb6c ("smb/client: do not account EOF extension as allocation")
>> Signed-off-by: Huiwen He <hehuiwen@kylinos.cn>
>> Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
>> ---
>>   fs/smb/client/smb2ops.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
>> index 06e9322a762a..cc8e0595e504 100644
>> --- a/fs/smb/client/smb2ops.c
>> +++ b/fs/smb/client/smb2ops.c
>> @@ -2193,10 +2193,13 @@ smb2_duplicate_extents(const unsigned int xid,
>>                          u64 len, u64 dest_off)
>>   {
>>          int rc;
>> +       int qrc;
>>          unsigned int ret_data_len;
>>          struct inode *inode;
>> +       struct smb2_file_all_info file_inf;
>>          struct duplicate_extents_to_file dup_ext_buf;
>>          struct cifs_tcon *tcon = tlink_tcon(trgtfile->tlink);
>> +       u64 asize;
>>
>>          /* server fileays advertise duplicate extent support with this flag */
>>          if ((le32_to_cpu(tcon->fsAttrInfo.Attributes) &
>> @@ -2232,6 +2235,19 @@ smb2_duplicate_extents(const unsigned int xid,
>>          if (ret_data_len > 0)
>>                  cifs_dbg(FYI, "Non-zero response length in duplicate extents\n");
>>
>> +       if (rc == 0) {
>> +               qrc = SMB2_query_info(xid, tcon, trgtfile->fid.persistent_fid,
>> +                                     trgtfile->fid.volatile_fid, &file_inf);
>> +               spin_lock(&inode->i_lock);
>> +               if (qrc == 0) {
>> +                       asize = le64_to_cpu(file_inf.AllocationSize);
>> +                       inode->i_blocks = CIFS_INO_BLOCKS(asize);
>> +               } else {
>> +                       CIFS_I(inode)->time = 0; /* force reval */
>> +               }
>> +               spin_unlock(&inode->i_lock);
>> +       }
>> +
>>   duplicate_extents_out:
>>          if (rc)
>>                  trace_smb3_clone_err(xid, srcfile->fid.volatile_fid,
>> --
>> 2.43.0
>>
> 
>