All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] io_uring: add getdents support, take 2
@ 2023-05-10 10:52 Dominique Martinet
  2023-05-10 10:52 ` [PATCH v2 1/6] fs: split off vfs_getdents function of getdents64 syscall Dominique Martinet
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Dominique Martinet @ 2023-05-10 10:52 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner, Jens Axboe, Pavel Begunkov,
	Stefan Roesch
  Cc: Clay Harris, Dave Chinner, linux-fsdevel, linux-kernel, io-uring,
	Dominique Martinet

This is an attempt to revive discussion after bringing it up as a reply
to the last attempt last week.

Since the problem with the previous attempt at adding getdents to
io_uring was that offset was problematic, we can just not add an offset
parameter: using different offsets is rarely useful in practice (maybe
for some cacheless userspace NFS server?) and more isn't worth the cost
of allowing arbitrary offsets: just allow rewind as a flag instead.
[happy to drop even that flag for what I care about, but that might be
useful enough on its own as io_uring rightfully has no seek API]

The new API does nothing that cannot be achieved with plain syscalls so
it shouldn't be introducing any new problem, the only downside is that
having the state in the file struct isn't very uring-ish and if a
better solution is found later that will probably require duplicating
some logic in a new flag... But that seems like it would likely be a
distant future, and this version should be usable right away.

To repeat the rationale for having getdents available from uring as it
has been a while, while I believe there can be real performance
improvements as suggested in [1], the real reason is to allow coherency
in code: applications using io_uring usually center their event loop
around the ring, and having the occasional synchronous getdents call in
there is cumbersome and hard to do properly when you consider e.g. a
slow or acting up network fs...
[1] https://lore.kernel.org/linux-fsdevel/20210218122640.GA334506@wantstofly.org/
(unfortunately the source is no longer available...)

liburing implementation:
https://github.com/martinetd/liburing/commits/getdents
(will submit properly after initial discussions here)

Previous discussion:
https://lore.kernel.org/all/20211221164004.119663-1-shr@fb.com/T/#m517583f23502f32b040c819d930359313b3db00c

Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
Changes in v2:
- implement NOWAIT as suggested by dchinner; I'm unable to provide
reliable benchmarks but it does indeed look positive (and makes sense)
to avoid spinning out to another thread when not required.
FWIW though, the serializing readdirs per inode argument given in v1
thread isn't valid: serialization is only done in io_prep_async_work
for regular files (REQ_F_ISREG, set from file mode through FFS_ISREG),
so dir operations aren't serialized in our case.
If I was pedantic I'd say we might want that hashing for filesystems
that don't implement interate_shared, but that info comes too late and
these filesystems should become less frequent so leaving as is.
- implement NOWAIT for kernfs and libfs to test with /sys
(the tentative patch for xfs didn't seem to work, didn't take the time
to debug)
- try to implement some EOF flag in CQE as suggested by Clay Harris,
this is less clearly cut and left as RFC.
The liburing test/getdents.t implementation has grown options to test
this flag and also try async explicitly in the latest commit:
https://github.com/martinetd/liburing/commits/getdents
- vfs_getdents split: add missing internal.h include
- Link to v1: https://lore.kernel.org/r/20230422-uring-getdents-v1-0-14c1db36e98c@codewreck.org

---
Dominique Martinet (6):
      fs: split off vfs_getdents function of getdents64 syscall
      vfs_getdents/struct dir_context: add flags field
      io_uring: add support for getdents
      kernfs: implement readdir FMODE_NOWAIT
      libfs: set FMODE_NOWAIT on dir open
      RFC: io_uring getdents: test returning an EOF flag in CQE

 fs/internal.h                 |  8 ++++++
 fs/kernfs/dir.c               | 21 ++++++++++++++-
 fs/libfs.c                    | 10 ++++---
 fs/readdir.c                  | 38 +++++++++++++++++++++------
 include/linux/fs.h            | 10 +++++++
 include/uapi/linux/io_uring.h |  9 +++++++
 io_uring/fs.c                 | 61 +++++++++++++++++++++++++++++++++++++++++++
 io_uring/fs.h                 |  3 +++
 io_uring/opdef.c              |  8 ++++++
 9 files changed, 156 insertions(+), 12 deletions(-)
---
base-commit: 58390c8ce1bddb6c623f62e7ed36383e7fa5c02f
change-id: 20230422-uring-getdents-2aab84d240aa

Best regards,
-- 
Dominique Martinet | Asmadeus


^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH v2 4/6] kernfs: implement readdir FMODE_NOWAIT
@ 2023-05-10 23:13 kernel test robot
  0 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2023-05-10 23:13 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Dan Carpenter

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230422-uring-getdents-v2-4-2db1e37dc55e@codewreck.org>
References: <20230422-uring-getdents-v2-4-2db1e37dc55e@codewreck.org>
TO: Dominique Martinet <asmadeus@codewreck.org>
TO: Alexander Viro <viro@zeniv.linux.org.uk>
TO: Christian Brauner <brauner@kernel.org>
TO: Jens Axboe <axboe@kernel.dk>
TO: Pavel Begunkov <asml.silence@gmail.com>
TO: Stefan Roesch <shr@fb.com>
CC: Clay Harris <bugs@claycon.org>
CC: Dave Chinner <david@fromorbit.com>
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: io-uring@vger.kernel.org
CC: Dominique Martinet <asmadeus@codewreck.org>

Hi Dominique,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 58390c8ce1bddb6c623f62e7ed36383e7fa5c02f]

url:    https://github.com/intel-lab-lkp/linux/commits/Dominique-Martinet/fs-split-off-vfs_getdents-function-of-getdents64-syscall/20230510-185542
base:   58390c8ce1bddb6c623f62e7ed36383e7fa5c02f
patch link:    https://lore.kernel.org/r/20230422-uring-getdents-v2-4-2db1e37dc55e%40codewreck.org
patch subject: [PATCH v2 4/6] kernfs: implement readdir FMODE_NOWAIT
:::::: branch date: 12 hours ago
:::::: commit date: 12 hours ago
config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20230511/202305110647.eSnSEulg-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202305110647.eSnSEulg-lkp@intel.com/

smatch warnings:
fs/kernfs/dir.c:1863 kernfs_fop_readdir() warn: inconsistent returns '&root->kernfs_rwsem'.

vim +1863 fs/kernfs/dir.c

fd7b9f7b9776b1 Tejun Heo          2013-11-28  1814  
c637b8acbe079e Tejun Heo          2013-12-11  1815  static int kernfs_fop_readdir(struct file *file, struct dir_context *ctx)
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1816  {
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1817  	struct dentry *dentry = file->f_path.dentry;
319ba91d352a74 Shaohua Li         2017-07-12  1818  	struct kernfs_node *parent = kernfs_dentry_node(dentry);
324a56e16e44ba Tejun Heo          2013-12-11  1819  	struct kernfs_node *pos = file->private_data;
393c3714081a53 Minchan Kim        2021-11-18  1820  	struct kernfs_root *root;
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1821  	const void *ns = NULL;
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1822  
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1823  	if (!dir_emit_dots(file, ctx))
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1824  		return 0;
393c3714081a53 Minchan Kim        2021-11-18  1825  
393c3714081a53 Minchan Kim        2021-11-18  1826  	root = kernfs_root(parent);
a551138c4b3b9f Dominique Martinet 2023-05-10  1827  	if (ctx->flags & DIR_CONTEXT_F_NOWAIT) {
a551138c4b3b9f Dominique Martinet 2023-05-10  1828  		if (!down_read_trylock(&root->kernfs_rwsem))
a551138c4b3b9f Dominique Martinet 2023-05-10  1829  			return -EAGAIN;
a551138c4b3b9f Dominique Martinet 2023-05-10  1830  	} else {
393c3714081a53 Minchan Kim        2021-11-18  1831  		down_read(&root->kernfs_rwsem);
a551138c4b3b9f Dominique Martinet 2023-05-10  1832  	}
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1833  
324a56e16e44ba Tejun Heo          2013-12-11  1834  	if (kernfs_ns_enabled(parent))
c525aaddc366df Tejun Heo          2013-12-11  1835  		ns = kernfs_info(dentry->d_sb)->ns;
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1836  
c637b8acbe079e Tejun Heo          2013-12-11  1837  	for (pos = kernfs_dir_pos(ns, parent, ctx->pos, pos);
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1838  	     pos;
c637b8acbe079e Tejun Heo          2013-12-11  1839  	     pos = kernfs_dir_next_pos(ns, parent, ctx->pos, pos)) {
adc5e8b58f4886 Tejun Heo          2013-12-11  1840  		const char *name = pos->name;
364595a6851bf6 Jeff Layton        2023-03-30  1841  		unsigned int type = fs_umode_to_dtype(pos->mode);
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1842  		int len = strlen(name);
67c0496e87d193 Tejun Heo          2019-11-04  1843  		ino_t ino = kernfs_ino(pos);
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1844  
adc5e8b58f4886 Tejun Heo          2013-12-11  1845  		ctx->pos = pos->hash;
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1846  		file->private_data = pos;
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1847  		kernfs_get(pos);
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1848  
393c3714081a53 Minchan Kim        2021-11-18  1849  		up_read(&root->kernfs_rwsem);
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1850  		if (!dir_emit(ctx, name, len, ino, type))
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1851  			return 0;
393c3714081a53 Minchan Kim        2021-11-18  1852  		down_read(&root->kernfs_rwsem);
a551138c4b3b9f Dominique Martinet 2023-05-10  1853  		if (ctx->flags & DIR_CONTEXT_F_NOWAIT) {
a551138c4b3b9f Dominique Martinet 2023-05-10  1854  			if (!down_read_trylock(&root->kernfs_rwsem))
a551138c4b3b9f Dominique Martinet 2023-05-10  1855  				return 0;
a551138c4b3b9f Dominique Martinet 2023-05-10  1856  		} else {
a551138c4b3b9f Dominique Martinet 2023-05-10  1857  			down_read(&root->kernfs_rwsem);
a551138c4b3b9f Dominique Martinet 2023-05-10  1858  		}
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1859  	}
393c3714081a53 Minchan Kim        2021-11-18  1860  	up_read(&root->kernfs_rwsem);
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1861  	file->private_data = NULL;
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1862  	ctx->pos = INT_MAX;
fd7b9f7b9776b1 Tejun Heo          2013-11-28 @1863  	return 0;
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1864  }
fd7b9f7b9776b1 Tejun Heo          2013-11-28  1865  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2023-07-11  8:24 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-10 10:52 [PATCH v2 0/6] io_uring: add getdents support, take 2 Dominique Martinet
2023-05-10 10:52 ` [PATCH v2 1/6] fs: split off vfs_getdents function of getdents64 syscall Dominique Martinet
2023-05-23 15:39   ` Christian Brauner
2023-05-23 21:13     ` Dominique Martinet
2023-05-10 10:52 ` [PATCH v2 2/6] vfs_getdents/struct dir_context: add flags field Dominique Martinet
2023-05-10 10:52 ` [PATCH v2 3/6] io_uring: add support for getdents Dominique Martinet
2023-05-10 10:52 ` [PATCH v2 4/6] kernfs: implement readdir FMODE_NOWAIT Dominique Martinet
2023-05-11 10:55   ` Dan Carpenter
2023-05-11 11:03     ` Dominique Martinet
2023-05-16  3:04   ` kernel test robot
2023-05-10 10:52 ` [PATCH v2 5/6] libfs: set FMODE_NOWAIT on dir open Dominique Martinet
2023-05-10 10:52 ` [PATCH v2 6/6] RFC: io_uring getdents: test returning an EOF flag in CQE Dominique Martinet
2023-05-23 14:30   ` Christian Brauner
2023-05-23 21:05     ` Dominique Martinet
2023-05-24 13:52       ` [PATCH v2 1/6] fs: split off vfs_getdents function of getdents64 syscall Christian Brauner
2023-05-24 21:36         ` Dominique Martinet
2023-05-25  9:22           ` Christian Brauner
2023-05-25 11:00             ` Dominique Martinet
2023-05-25 12:33               ` Christian Brauner
2023-07-11  8:17               ` Hao Xu
2023-07-11  8:24                 ` Dominique Martinet
  -- strict thread matches above, loose matches on Subject: below --
2023-05-10 23:13 [PATCH v2 4/6] kernfs: implement readdir FMODE_NOWAIT kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.