[PATCH v2 1/2] fs: btrfs: implement opendir(), readdir() and closedir()

[Qu Wenruo]

在 2026/6/27 15:29, Alexey Charkov 写道:
> On Sat, Jun 27, 2026 at 2:27 AM Qu Wenruo <quwenruo.btrfs at gmx.com> wrote:
[...]
>>> +     key.objectid = dirs->subvolid;
>>> +     key.type = BTRFS_ROOT_ITEM_KEY;
>>> +     key.offset = (u64)-1;
>>> +     root = btrfs_read_fs_root(fs_info, &key);
>>
>> You can save the root pointer into btrfs_dir_stream, so we can avoid
>> subvolume root lookup for each dentry.
> 
> That was in fact the first thing I tried, but turns out that it
> doesn't work because a root pointer created with one fs_info structure
> stops working once fs_close at the end of the generic fs opendir
> caller closes the fs and deallocates the fs_info structure, due to it
> referencing trees allocated for that instance of fs_info. The result
> is a synchronous abort.
> 
> Hence the comment above the btrfs_dir_stream definition and this
> second-best solution.
> 
> Please let me know if I've missed a more elegant option!
> 
> Closing and reopening the fs at every call which the FS layer does
> sounds like a huge overhead, but I must assume it was unavoidable when
> the interface was designed.

Oh, I wasn't aware that u-boot closes the fs after opening the dir, 
that's sounds very weird.
But if that's the case, the current one is indeed our best solution.

Thanks for point this out.

With that said, the series looks good to me now.

Reviewed-by: Qu Wenruo <wqu at suse.com>

Thanks,
Qu
> 
>> You do not need to bother the lifespan of subvolume roots either, they
>> are properly released during the close of the fs.
>>
>> Otherwise looks good to me, except an unrelated question just lines below.
>>
>>> +     if (IS_ERR(root))
>>> +             return PTR_ERR(root);
>>> +
>>> +     memset(dent, 0, sizeof(*dent));
>>> +     ret = btrfs_next_dir_entry(root, dirs->ino, &dirs->offset, dent->name,
>>> +                                sizeof(dent->name), &type);
>>> +     if (ret < 0)
>>> +             return ret;
>>> +     if (ret > 0)
>>> +             return -ENOENT;
>>
>> I'm not sure what is the proper/preferred/sane handling of end of directory.
>>
>> Ext4/fat returns -ENOENT, squashfs returned -1, erofs return 1, exfat
>> returns 0 but without populating @dentp.
>>
>> Personally I found the exfat behavior more sane, but considering the
>> caller fs_ls_generic() doesn't really bother the return value but only
>> cares if @dent is populated, it should be fine either way.
>>
>> But still, returning -ENOENT will populate errno, which may be confusing
>> for debugging.
>>
>> Anyway it's an unrelated nitpick.
> 
> Happy to switch to exfat-like behavior. I was only looking at ext4
> implementation as that is likely the most widely used and tested in
> various corner cases, but it sounds like either way should work.
> 
> Thanks a lot,
> Alexey