Re: [PATCH] fs: btrfs: report file sizes from readdir

在 2026/8/2 15:32, Alexey Charkov 写道:
> Hi Cole,
> 
> On Sun, Aug 2, 2026 at 9:42 AM Cole Munz <[email protected]> wrote:
>>
>> btrfs_readdir() zeroes the dirent and fills in only the name and the
>> type, so dent->size stays 0 and every file is listed as zero bytes:
>>
>>    => ls host 0 /
>>            0   f_192k.bin
>>            0   small_3k.bin
>>
>> Reads themselves are fine, since btrfs_read() takes the size from
>> btrfs_size(), which does its own inode item lookup. It affects EFI
>> too: dir_read() in lib/efi_loader/efi_file.c copies dent->size into
>> both file_size and physical_size, so an EFI application enumerating a
>> directory on btrfs sees every file as empty, which is the generic-code
>> path Alexey's readdir series moves btrfs onto.
>>
>> The custom listing that fs_ls_generic() replaced looked the inode item
>> up and printed the real size, and every other filesystem in the tree
>> fills dent->size in its own readdir: ext4fs.c:327, exfat io.c:805,
>> erofs fs.c:186, squashfs sqfs.c:1095 and fat.c:1555.
>>
>> btrfs_next_dir_entry() already has the dir item mapped, so read the
>> key it points at while we are there and hand it back to the caller,
>> and use that to reach the inode item. A subvolume entry points at a
>> root item instead and has no size of its own, so leave that one at 0.
>>
>>    => ls host 0 /
>>       196608   f_192k.bin
>>         3000   small_3k.bin
>>
>> Fixes: 31cf3f177823 ("fs: btrfs: use fs_ls_generic() and drop custom implementation")
>> Signed-off-by: Cole Munz <[email protected]>
>> ---
>> The Fixes: commit is in Tom's tree, applied 2026-07-10 (b4-ty
>> [email protected]); it has not
>> reached master yet, so that SHA will not resolve on a public clone today.
>>
>> Found by a btrfs test suite I am preparing for test/py; I will send that
>> separately once the pending zstd inline extent fix is applied, since it
>> depends on it.
>>   fs/btrfs/btrfs.c    | 24 +++++++++++++++++++++++-
>>   fs/btrfs/ctree.h    |  3 ++-
>>   fs/btrfs/dir-item.c |  7 ++++++-
>>   3 files changed, 31 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/btrfs.c b/fs/btrfs/btrfs.c
>> index e663dda12e80..b2856be0662f 100644
>> --- a/fs/btrfs/btrfs.c
>> +++ b/fs/btrfs/btrfs.c
>> @@ -93,7 +93,10 @@ int btrfs_readdir(struct fs_dir_stream *fs_dirs, struct fs_dirent **dentp)
>>          struct btrfs_dir_stream *dirs = container_of(fs_dirs, struct btrfs_dir_stream, parent);
>>          struct btrfs_fs_info *fs_info = current_fs_info;
>>          struct fs_dirent *dent = &dirs->dirent;
>> +       struct btrfs_inode_item *ii;
>>          struct btrfs_root *root;
>> +       struct btrfs_path path;
>> +       struct btrfs_key location;
>>          struct btrfs_key key;
>>          u8 type;
>>          int ret;
>> @@ -110,13 +113,32 @@ int btrfs_readdir(struct fs_dir_stream *fs_dirs, struct fs_dirent **dentp)
>>
>>          memset(dent, 0, sizeof(*dent));
>>          ret = btrfs_next_dir_entry(root, dirs->ino, &dirs->offset, dent->name,
>> -                                  sizeof(dent->name), &type);
>> +                                  sizeof(dent->name), &type, &location);
>>          if (ret < 0)
>>                  return ret;
>>          if (ret > 0)
>>                  return -ENOENT;
>>
>>          dent->type = btrfs_dirent_type_to_fs_type(type);
>> +
>> +       /*
>> +        * A subvolume entry points at a root item rather than an inode, and
>> +        * has no size of its own.  Everything else carries one, and the fs
>> +        * layer prints it, so look it up.
>> +        */
>> +       if (location.type == BTRFS_INODE_ITEM_KEY) {
>> +               btrfs_init_path(&path);
>> +               ret = btrfs_search_slot(NULL, root, &location, &path, 0, 0);
>> +               if (ret == 0) {
>> +                       ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
>> +                                           struct btrfs_inode_item);
>> +                       dent->size = btrfs_inode_size(path.nodes[0], ii);
>> +               }
>> +               btrfs_release_path(&path);
> 
> The above repeats much of the calls that have just been done for this
> exact entry several lines above inside btrfs_read_next_dir_enrty()
> 
> Can you perhaps refactor to avoid this repetition?

Sorry I didn't see the point.

The function btrfs_read_next_dirty_entry() is search the key inside the 
parent directory.

Meanwhile this one is search for the inode item of the child entry.

They are completely different and I didn't see why "refactor" can improve.

In fact, this version is much simpler:

  fs/btrfs/btrfs.c    | 24 +++++++++++++++++++++++-
  fs/btrfs/ctree.h    |  3 ++-
  fs/btrfs/dir-item.c |  7 ++++++-
  3 files changed, 31 insertions(+), 3 deletions(-)

Compared to the v2:

  fs/btrfs/btrfs.c    | 75 +++++++++++++++++++++++++++++++--------------
  fs/btrfs/ctree.h    |  3 +-
  fs/btrfs/dir-item.c |  7 ++++-
  3 files changed, 60 insertions(+), 25 deletions(-)

Thanks,
Qu
> 
> Best regards,
> Alexey