| Message ID | 20230711114027.59945-1-hao.xu@linux.dev (mailing list archive) |
|---|---|
| Headers | show |
| Series | io_uring getdents | expand |
On Tue, Jul 11, 2023 at 07:40:24PM +0800, Hao Xu wrote: > From: Hao Xu <howeyxu@tencent.com> > > This series introduce getdents64 to io_uring, the code logic is similar > with the snychronized version's. It first try nowait issue, and offload > it to io-wq threads if the first try fails. > > > v2->v3: > - removed the kernfs patches > - add f_pos_lock logic > - remove the "reduce last EOF getdents try" optimization since > Dominique reports that doesn't make difference > - remove the rewind logic, I think the right way is to introduce lseek > to io_uring not to patch this logic to getdents. > - add Singed-off-by of Stefan Roesch for patch 1 since checkpatch > complained that Co-developed-by someone should be accompanied with > Signed-off-by same person, I can remove them if Stefan thinks that's > not proper. > > > Dominique Martinet (1): > fs: split off vfs_getdents function of getdents64 syscall > > Hao Xu (2): > vfs_getdents/struct dir_context: add flags field > io_uring: add support for getdents So what filesystem actually uses this new NOWAIT functionality? Unless I'm blind (quite possibly) I don't see any filesystem implementation of this functionality in the patch series. I know I posted a prototype for XFS to use it, and I expected that it would become part of this patch series to avoid the "we don't add unused code to the kernel" problem. i.e. the authors would take the XFS prototype, make it work, add support into for the new io_uring operation to fsstress in fstests and then use that to stress test the new infrastructure before it gets merged.... But I don't see any of this? -Dave.
On 7/11/23 5:47?PM, Dave Chinner wrote: > On Tue, Jul 11, 2023 at 07:40:24PM +0800, Hao Xu wrote: >> From: Hao Xu <howeyxu@tencent.com> >> >> This series introduce getdents64 to io_uring, the code logic is similar >> with the snychronized version's. It first try nowait issue, and offload >> it to io-wq threads if the first try fails. >> >> >> v2->v3: >> - removed the kernfs patches >> - add f_pos_lock logic >> - remove the "reduce last EOF getdents try" optimization since >> Dominique reports that doesn't make difference >> - remove the rewind logic, I think the right way is to introduce lseek >> to io_uring not to patch this logic to getdents. >> - add Singed-off-by of Stefan Roesch for patch 1 since checkpatch >> complained that Co-developed-by someone should be accompanied with >> Signed-off-by same person, I can remove them if Stefan thinks that's >> not proper. >> >> >> Dominique Martinet (1): >> fs: split off vfs_getdents function of getdents64 syscall >> >> Hao Xu (2): >> vfs_getdents/struct dir_context: add flags field >> io_uring: add support for getdents > > So what filesystem actually uses this new NOWAIT functionality? > Unless I'm blind (quite possibly) I don't see any filesystem > implementation of this functionality in the patch series. > > I know I posted a prototype for XFS to use it, and I expected that > it would become part of this patch series to avoid the "we don't add > unused code to the kernel" problem. i.e. the authors would take the > XFS prototype, make it work, add support into for the new io_uring > operation to fsstress in fstests and then use that to stress test > the new infrastructure before it gets merged.... > > But I don't see any of this? That would indeed be great if we could get NOWAIT, that might finally convince me that it's worth plumbing up! Do you have a link to that prototype? That seems like what should be the base for this, and be an inspiration for other file systems to get efficient getdents via this (rather than io-wq punt, which I'm not a huge fan of...).
Jens Axboe wrote on Tue, Jul 11, 2023 at 05:51:46PM -0600: > > So what filesystem actually uses this new NOWAIT functionality? > > Unless I'm blind (quite possibly) I don't see any filesystem > > implementation of this functionality in the patch series. I had implemented this for kernfs and libfs (so sysfs, debugfs, possibly tmpfs/proc?) in v2 The patch as of v2's mail has a bug, but my branch has it fixed as of https://github.com/martinetd/linux/commits/io_uring_getdents (I guess these aren't "real" enough though) > > I know I posted a prototype for XFS to use it, and I expected that > > it would become part of this patch series to avoid the "we don't add > > unused code to the kernel" problem. i.e. the authors would take the > > XFS prototype, make it work, add support into for the new io_uring > > operation to fsstress in fstests and then use that to stress test > > the new infrastructure before it gets merged.... > > > > But I don't see any of this? > > That would indeed be great if we could get NOWAIT, that might finally > convince me that it's worth plumbing up! Do you have a link to that > prototype? That seems like what should be the base for this, and be an > inspiration for other file systems to get efficient getdents via this > (rather than io-wq punt, which I'm not a huge fan of...). the xfs poc was in this mail: https://lore.kernel.org/all/20230501071603.GE2155823@dread.disaster.area/ I never spent time debugging it, but it should definitely be workable
On 7/11/23 6:53 PM, Dominique Martinet wrote: > Jens Axboe wrote on Tue, Jul 11, 2023 at 05:51:46PM -0600: >>> So what filesystem actually uses this new NOWAIT functionality? >>> Unless I'm blind (quite possibly) I don't see any filesystem >>> implementation of this functionality in the patch series. > > I had implemented this for kernfs and libfs (so sysfs, debugfs, possibly > tmpfs/proc?) in v2 > > The patch as of v2's mail has a bug, but my branch has it fixed as of > https://github.com/martinetd/linux/commits/io_uring_getdents > > (I guess these aren't "real" enough though) No, I definitely think those are real and valid. But would be nice with a "real" file system as well. >>> I know I posted a prototype for XFS to use it, and I expected that >>> it would become part of this patch series to avoid the "we don't add >>> unused code to the kernel" problem. i.e. the authors would take the >>> XFS prototype, make it work, add support into for the new io_uring >>> operation to fsstress in fstests and then use that to stress test >>> the new infrastructure before it gets merged.... >>> >>> But I don't see any of this? >> >> That would indeed be great if we could get NOWAIT, that might finally >> convince me that it's worth plumbing up! Do you have a link to that >> prototype? That seems like what should be the base for this, and be an >> inspiration for other file systems to get efficient getdents via this >> (rather than io-wq punt, which I'm not a huge fan of...). > > the xfs poc was in this mail: > https://lore.kernel.org/all/20230501071603.GE2155823@dread.disaster.area/ > > I never spent time debugging it, but it should definitely be workable If either you or Hao wants to take a stab at it and see how it goes, I think that would be hugely beneficial for this patchset.
On 7/12/23 08:53, Dominique Martinet wrote: > Jens Axboe wrote on Tue, Jul 11, 2023 at 05:51:46PM -0600: >>> So what filesystem actually uses this new NOWAIT functionality? >>> Unless I'm blind (quite possibly) I don't see any filesystem >>> implementation of this functionality in the patch series. > > I had implemented this for kernfs and libfs (so sysfs, debugfs, possibly > tmpfs/proc?) in v2 > > The patch as of v2's mail has a bug, but my branch has it fixed as of > https://github.com/martinetd/linux/commits/io_uring_getdents I see, I'll try this, those in v2 causes issues when I boot my VM with them. > > (I guess these aren't "real" enough though) > >>> I know I posted a prototype for XFS to use it, and I expected that >>> it would become part of this patch series to avoid the "we don't add >>> unused code to the kernel" problem. i.e. the authors would take the >>> XFS prototype, make it work, add support into for the new io_uring >>> operation to fsstress in fstests and then use that to stress test >>> the new infrastructure before it gets merged.... >>> >>> But I don't see any of this? >> >> That would indeed be great if we could get NOWAIT, that might finally >> convince me that it's worth plumbing up! Do you have a link to that >> prototype? That seems like what should be the base for this, and be an >> inspiration for other file systems to get efficient getdents via this >> (rather than io-wq punt, which I'm not a huge fan of...). > > the xfs poc was in this mail: > https://lore.kernel.org/all/20230501071603.GE2155823@dread.disaster.area/ > > I never spent time debugging it, but it should definitely be workable >
Hi, On 7/12/23 08:56, Jens Axboe wrote: > On 7/11/23 6:53 PM, Dominique Martinet wrote: >> Jens Axboe wrote on Tue, Jul 11, 2023 at 05:51:46PM -0600: >>>> So what filesystem actually uses this new NOWAIT functionality? >>>> Unless I'm blind (quite possibly) I don't see any filesystem >>>> implementation of this functionality in the patch series. >> >> I had implemented this for kernfs and libfs (so sysfs, debugfs, possibly >> tmpfs/proc?) in v2 >> >> The patch as of v2's mail has a bug, but my branch has it fixed as of >> https://github.com/martinetd/linux/commits/io_uring_getdents >> >> (I guess these aren't "real" enough though) > > No, I definitely think those are real and valid. But would be nice with > a "real" file system as well. > >>>> I know I posted a prototype for XFS to use it, and I expected that >>>> it would become part of this patch series to avoid the "we don't add >>>> unused code to the kernel" problem. i.e. the authors would take the >>>> XFS prototype, make it work, add support into for the new io_uring >>>> operation to fsstress in fstests and then use that to stress test >>>> the new infrastructure before it gets merged.... >>>> >>>> But I don't see any of this? >>> >>> That would indeed be great if we could get NOWAIT, that might finally >>> convince me that it's worth plumbing up! Do you have a link to that >>> prototype? That seems like what should be the base for this, and be an >>> inspiration for other file systems to get efficient getdents via this >>> (rather than io-wq punt, which I'm not a huge fan of...). >> >> the xfs poc was in this mail: >> https://lore.kernel.org/all/20230501071603.GE2155823@dread.disaster.area/ >> >> I never spent time debugging it, but it should definitely be workable > > If either you or Hao wants to take a stab at it and see how it goes, > I think that would be hugely beneficial for this patchset. > I can take the xfs and kernfs part if Dominique doesn't mind. Regards, Hao
On 7/12/23 07:47, Dave Chinner wrote: > On Tue, Jul 11, 2023 at 07:40:24PM +0800, Hao Xu wrote: >> From: Hao Xu <howeyxu@tencent.com> >> >> This series introduce getdents64 to io_uring, the code logic is similar >> with the snychronized version's. It first try nowait issue, and offload >> it to io-wq threads if the first try fails. >> >> >> v2->v3: >> - removed the kernfs patches >> - add f_pos_lock logic >> - remove the "reduce last EOF getdents try" optimization since >> Dominique reports that doesn't make difference >> - remove the rewind logic, I think the right way is to introduce lseek >> to io_uring not to patch this logic to getdents. >> - add Singed-off-by of Stefan Roesch for patch 1 since checkpatch >> complained that Co-developed-by someone should be accompanied with >> Signed-off-by same person, I can remove them if Stefan thinks that's >> not proper. >> >> >> Dominique Martinet (1): >> fs: split off vfs_getdents function of getdents64 syscall >> >> Hao Xu (2): >> vfs_getdents/struct dir_context: add flags field >> io_uring: add support for getdents > > So what filesystem actually uses this new NOWAIT functionality? > Unless I'm blind (quite possibly) I don't see any filesystem > implementation of this functionality in the patch series. > > I know I posted a prototype for XFS to use it, and I expected that > it would become part of this patch series to avoid the "we don't add > unused code to the kernel" problem. i.e. the authors would take the > XFS prototype, make it work, add support into for the new io_uring > operation to fsstress in fstests and then use that to stress test > the new infrastructure before it gets merged.... > > But I don't see any of this? > > -Dave. Hi Dave, You are right, currently no real filesystem supports that from my investigation, I saw the xfs prototype, I'd like to make it work first. That may cause some time. Thanks, Hao
From: Hao Xu <howeyxu@tencent.com> This series introduce getdents64 to io_uring, the code logic is similar with the snychronized version's. It first try nowait issue, and offload it to io-wq threads if the first try fails. v2->v3: - removed the kernfs patches - add f_pos_lock logic - remove the "reduce last EOF getdents try" optimization since Dominique reports that doesn't make difference - remove the rewind logic, I think the right way is to introduce lseek to io_uring not to patch this logic to getdents. - add Singed-off-by of Stefan Roesch for patch 1 since checkpatch complained that Co-developed-by someone should be accompanied with Signed-off-by same person, I can remove them if Stefan thinks that's not proper. Dominique Martinet (1): fs: split off vfs_getdents function of getdents64 syscall Hao Xu (2): vfs_getdents/struct dir_context: add flags field io_uring: add support for getdents fs/internal.h | 8 +++++ fs/readdir.c | 36 ++++++++++++++++----- include/linux/fs.h | 8 +++++ include/uapi/linux/io_uring.h | 7 ++++ io_uring/fs.c | 60 +++++++++++++++++++++++++++++++++++ io_uring/fs.h | 3 ++ io_uring/opdef.c | 8 +++++ 7 files changed, 122 insertions(+), 8 deletions(-)