[PATCH v1 0/3] io_uring: add getdents64 support

[PATCH v1 0/3] io_uring: add getdents64 support

[Stefan Roesch]

This series adds support for getdents64 in liburing. The intent is to
provide a more complete I/O interface for io_uring.. 

Patch 1: fs: add parameter use_fpos to iterate_dir()
  This adds a new parameter to the function iterate_dir() so the
  caller can specify if the position is the file position or the
  position stored in the buffer context.

Patch 2: fs: split off vfs_getdents function from getdents64 system call
  This splits of the iterate_dir part of the syscall in its own
  dedicated function. This allows to call the function directly from
  liburing.

Patch 3: io_uring: add support for getdents64
  Adds the functions to io_uring to support getdents64.

There is also a patch series for the changes to liburing. This includes
a new test. The patch series is called "liburing: add getdents support."

The following tests have been performed:
- new liburing getdents test program has been run
- xfstests have been run
- both tests have been repeated with the kernel memory leak checker
  and no leaks have been reported.


Stefan Roesch (3):
  fs: add parameter use_fpos to iterate_dir function
  fs: split off vfs_getdents function of getdents64 syscall
  io_uring: add support for getdents64

 fs/exportfs/expfs.c           |  2 +-
 fs/internal.h                 |  8 +++++
 fs/io_uring.c                 | 52 ++++++++++++++++++++++++++++
 fs/nfsd/nfs4recover.c         |  2 +-
 fs/nfsd/vfs.c                 |  2 +-
 fs/overlayfs/readdir.c        |  6 ++--
 fs/readdir.c                  | 64 ++++++++++++++++++++++++++---------
 include/linux/fs.h            |  2 +-
 include/uapi/linux/io_uring.h |  1 +
 9 files changed, 116 insertions(+), 23 deletions(-)

Signed-off-by: Stefan Roesch <shr@fb.com>
-- 
2.30.2

[PATCH v1 1/3] fs: add parameter use_fpos to iterate_dir function

[Stefan Roesch]

This adds the use_fpos parameter to the iterate_dir function.
If use_fpos is true it uses the file position in the file
structure (existing behavior). If use_fpos is false, it uses
the pos in the context structure.

This change is required to support getdents in io_uring.

Signed-off-by: Stefan Roesch <shr@fb.com>
---
 fs/exportfs/expfs.c    |  2 +-
 fs/nfsd/nfs4recover.c  |  2 +-
 fs/nfsd/vfs.c          |  2 +-
 fs/overlayfs/readdir.c |  6 +++---
 fs/readdir.c           | 28 ++++++++++++++++++++--------
 include/linux/fs.h     |  2 +-
 6 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index 0106eba46d5a..0f303356f907 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -323,7 +323,7 @@ static int get_name(const struct path *path, char *name, struct dentry *child)
 	while (1) {
 		int old_seq = buffer.sequence;
 
-		error = iterate_dir(file, &buffer.ctx);
+		error = iterate_dir(file, &buffer.ctx, true);
 		if (buffer.found) {
 			error = 0;
 			break;
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 6fedc49726bf..013b1a3530c9 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -307,7 +307,7 @@ nfsd4_list_rec_dir(recdir_func *f, struct nfsd_net *nn)
 		return status;
 	}
 
-	status = iterate_dir(nn->rec_file, &ctx.ctx);
+	status = iterate_dir(nn->rec_file, &ctx.ctx, true);
 	inode_lock_nested(d_inode(dir), I_MUTEX_PARENT);
 
 	list_for_each_entry_safe(entry, tmp, &ctx.names, list) {
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index c99857689e2c..cd7a7d783fa7 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1980,7 +1980,7 @@ static __be32 nfsd_buffered_readdir(struct file *file, struct svc_fh *fhp,
 		buf.used = 0;
 		buf.full = 0;
 
-		host_err = iterate_dir(file, &buf.ctx);
+		host_err = iterate_dir(file, &buf.ctx, true);
 		if (buf.full)
 			host_err = 0;
 
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 150fdf3bc68d..089150315942 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -306,7 +306,7 @@ static inline int ovl_dir_read(struct path *realpath,
 	do {
 		rdd->count = 0;
 		rdd->err = 0;
-		err = iterate_dir(realfile, &rdd->ctx);
+		err = iterate_dir(realfile, &rdd->ctx, true);
 		if (err >= 0)
 			err = rdd->err;
 	} while (!err && rdd->count);
@@ -722,7 +722,7 @@ static int ovl_iterate_real(struct file *file, struct dir_context *ctx)
 			return PTR_ERR(rdt.cache);
 	}
 
-	err = iterate_dir(od->realfile, &rdt.ctx);
+	err = iterate_dir(od->realfile, &rdt.ctx, true);
 	ctx->pos = rdt.ctx.pos;
 
 	return err;
@@ -753,7 +753,7 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx)
 		      OVL_TYPE_MERGE(ovl_path_type(dentry->d_parent))))) {
 			err = ovl_iterate_real(file, ctx);
 		} else {
-			err = iterate_dir(od->realfile, ctx);
+			err = iterate_dir(od->realfile, ctx, true);
 		}
 		goto out;
 	}
diff --git a/fs/readdir.c b/fs/readdir.c
index 09e8ed7d4161..8ea5b5f45a78 100644
--- a/fs/readdir.c
+++ b/fs/readdir.c
@@ -21,6 +21,7 @@
 #include <linux/unistd.h>
 #include <linux/compat.h>
 #include <linux/uaccess.h>
+#include "internal.h"
 
 #include <asm/unaligned.h>
 
@@ -36,8 +37,14 @@
 	unsafe_copy_to_user(dst, src, len, label);		\
 } while (0)
 
-
-int iterate_dir(struct file *file, struct dir_context *ctx)
+/**
+ * iterate_dir - iterate over directory
+ * @file    : pointer to file struct of directory
+ * @ctx     : pointer to directory ctx structure
+ * @use_fpos: true : use file offset
+ *            false: use pos in ctx structure
+ */
+int iterate_dir(struct file *file, struct dir_context *ctx, bool use_fpos)
 {
 	struct inode *inode = file_inode(file);
 	bool shared = false;
@@ -60,12 +67,17 @@ int iterate_dir(struct file *file, struct dir_context *ctx)
 
 	res = -ENOENT;
 	if (!IS_DEADDIR(inode)) {
-		ctx->pos = file->f_pos;
+		if (use_fpos)
+			ctx->pos = file->f_pos;
+
 		if (shared)
 			res = file->f_op->iterate_shared(file, ctx);
 		else
 			res = file->f_op->iterate(file, ctx);
-		file->f_pos = ctx->pos;
+
+		if (use_fpos)
+			file->f_pos = ctx->pos;
+
 		fsnotify_access(file);
 		file_accessed(file);
 	}
@@ -190,7 +202,7 @@ SYSCALL_DEFINE3(old_readdir, unsigned int, fd,
 	if (!f.file)
 		return -EBADF;
 
-	error = iterate_dir(f.file, &buf.ctx);
+	error = iterate_dir(f.file, &buf.ctx, true);
 	if (buf.result)
 		error = buf.result;
 
@@ -283,7 +295,7 @@ SYSCALL_DEFINE3(getdents, unsigned int, fd,
 	if (!f.file)
 		return -EBADF;
 
-	error = iterate_dir(f.file, &buf.ctx);
+	error = iterate_dir(f.file, &buf.ctx, true);
 	if (error >= 0)
 		error = buf.error;
 	if (buf.prev_reclen) {
@@ -448,7 +460,7 @@ COMPAT_SYSCALL_DEFINE3(old_readdir, unsigned int, fd,
 	if (!f.file)
 		return -EBADF;
 
-	error = iterate_dir(f.file, &buf.ctx);
+	error = iterate_dir(f.file, &buf.ctx, true);
 	if (buf.result)
 		error = buf.result;
 
@@ -534,7 +546,7 @@ COMPAT_SYSCALL_DEFINE3(getdents, unsigned int, fd,
 	if (!f.file)
 		return -EBADF;
 
-	error = iterate_dir(f.file, &buf.ctx);
+	error = iterate_dir(f.file, &buf.ctx, true);
 	if (error >= 0)
 		error = buf.error;
 	if (buf.prev_reclen) {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1cb616fc1105..ba4f49c4ac41 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3343,7 +3343,7 @@ const char *simple_get_link(struct dentry *, struct inode *,
 			    struct delayed_call *);
 extern const struct inode_operations simple_symlink_inode_operations;
 
-extern int iterate_dir(struct file *, struct dir_context *);
+extern int iterate_dir(struct file *file, struct dir_context *ctx, bool use_fpos);
 
 int vfs_fstatat(int dfd, const char __user *filename, struct kstat *stat,
 		int flags);
-- 
2.30.2

Re: [PATCH v1 1/3] fs: add parameter use_fpos to iterate_dir function

[kernel test robot]

Hi Stefan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.16-rc2 next-20211124]
[cannot apply to mszeredi-vfs/overlayfs-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Stefan-Roesch/io_uring-add-getdents64-support/20211124-022809
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 136057256686de39cc3a07c2e39ef6bc43003ff6
config: x86_64-randconfig-r006-20211124 (https://download.01.org/0day-ci/archive/20211125/202111250356.yBAHK4KL-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 67a1c45def8a75061203461ab0060c75c864df1c)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/94fab53b56d471270b8b7b9afe6d73a8098448be
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Stefan-Roesch/io_uring-add-getdents64-support/20211124-022809
        git checkout 94fab53b56d471270b8b7b9afe6d73a8098448be
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> fs/ksmbd/vfs.c:1139:47: error: too few arguments to function call, expected 3, have 2
           err = iterate_dir(fp->filp, &readdir_data.ctx);
                 ~~~~~~~~~~~                            ^
   include/linux/fs.h:3346:12: note: 'iterate_dir' declared here
   extern int iterate_dir(struct file *file, struct dir_context *ctx, bool use_fpos);
              ^
   fs/ksmbd/vfs.c:1189:44: error: too few arguments to function call, expected 3, have 2
           ret = iterate_dir(dfilp, &readdir_data.ctx);
                 ~~~~~~~~~~~                         ^
   include/linux/fs.h:3346:12: note: 'iterate_dir' declared here
   extern int iterate_dir(struct file *file, struct dir_context *ctx, bool use_fpos);
              ^
   2 errors generated.
--
>> fs/ksmbd/smb2pdu.c:3926:58: error: too few arguments to function call, expected 3, have 2
           rc = iterate_dir(dir_fp->filp, &dir_fp->readdir_data.ctx);
                ~~~~~~~~~~~                                        ^
   include/linux/fs.h:3346:12: note: 'iterate_dir' declared here
   extern int iterate_dir(struct file *file, struct dir_context *ctx, bool use_fpos);
              ^
   1 error generated.


vim +1139 fs/ksmbd/vfs.c

f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1122  
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1123  /**
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1124   * ksmbd_vfs_empty_dir() - check for empty directory
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1125   * @fp:	ksmbd file pointer
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1126   *
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1127   * Return:	true if directory empty, otherwise false
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1128   */
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1129  int ksmbd_vfs_empty_dir(struct ksmbd_file *fp)
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1130  {
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1131  	int err;
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1132  	struct ksmbd_readdir_data readdir_data;
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1133  
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1134  	memset(&readdir_data, 0, sizeof(struct ksmbd_readdir_data));
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1135  
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1136  	set_ctx_actor(&readdir_data.ctx, __dir_empty);
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1137  	readdir_data.dirent_count = 0;
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1138  
e8c061917133dd fs/cifsd/vfs.c Namjae Jeon 2021-06-22 @1139  	err = iterate_dir(fp->filp, &readdir_data.ctx);
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1140  	if (readdir_data.dirent_count > 2)
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1141  		err = -ENOTEMPTY;
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1142  	else
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1143  		err = 0;
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1144  	return err;
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1145  }
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon 2021-03-16  1146  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH v1 1/3] fs: add parameter use_fpos to iterate_dir function

[Miklos Szeredi]

On Tue, 23 Nov 2021 at 19:10, Stefan Roesch <shr@fb.com> wrote:
>
> This adds the use_fpos parameter to the iterate_dir function.
> If use_fpos is true it uses the file position in the file
> structure (existing behavior). If use_fpos is false, it uses
> the pos in the context structure.

Is there a reason not to introduce a iterate_dir_no_fpos() variant and
call this from iterate_dir() with the necessary fpos update?

Thanks,
Miklos

Re: [PATCH v1 1/3] fs: add parameter use_fpos to iterate_dir function

[Stefan Roesch]

On 11/25/21 6:45 AM, Miklos Szeredi wrote:
> On Tue, 23 Nov 2021 at 19:10, Stefan Roesch <shr@fb.com> wrote:
>>
>> This adds the use_fpos parameter to the iterate_dir function.
>> If use_fpos is true it uses the file position in the file
>> structure (existing behavior). If use_fpos is false, it uses
>> the pos in the context structure.
> 
> Is there a reason not to introduce a iterate_dir_no_fpos() variant and
> call this from iterate_dir() with the necessary fpos update?
>

This would cause some code duplication. With the next version of the patch
I'll keep the current function of iterate_dir and introduce a new do_iterate_dir
function that has the use_fpos parameter. With that change the function
signature of iterate_dir does not change and iterate_dir will call do_iterate_dir.
 
> Thanks,
> Miklos
> 

Re: [PATCH v1 1/3] fs: add parameter use_fpos to iterate_dir function

[Pavel Begunkov]

On 11/23/21 18:10, Stefan Roesch wrote:
> This adds the use_fpos parameter to the iterate_dir function.
> If use_fpos is true it uses the file position in the file
> structure (existing behavior). If use_fpos is false, it uses
> the pos in the context structure.

Looks sane, one question below

> 
> This change is required to support getdents in io_uring.
> 
> Signed-off-by: Stefan Roesch <shr@fb.com>
> ---
>   fs/exportfs/expfs.c    |  2 +-
>   fs/nfsd/nfs4recover.c  |  2 +-
>   fs/nfsd/vfs.c          |  2 +-
>   fs/overlayfs/readdir.c |  6 +++---
>   fs/readdir.c           | 28 ++++++++++++++++++++--------
>   include/linux/fs.h     |  2 +-
>   6 files changed, 27 insertions(+), 15 deletions(-)
> 
[...]
> diff --git a/fs/readdir.c b/fs/readdir.c
> index 09e8ed7d4161..8ea5b5f45a78 100644
> --- a/fs/readdir.c
> +++ b/fs/readdir.c
> @@ -21,6 +21,7 @@
>   #include <linux/unistd.h>
>   #include <linux/compat.h>
>   #include <linux/uaccess.h>
> +#include "internal.h"

Don't see this header is used in this patch. Just to be clear,
it is here only for 2/3, right?

[...]

-- 
Pavel Begunkov

Re: [PATCH v1 1/3] fs: add parameter use_fpos to iterate_dir function

[Stefan Roesch]

On 11/25/21 7:47 AM, Pavel Begunkov wrote:
> On 11/23/21 18:10, Stefan Roesch wrote:
>> This adds the use_fpos parameter to the iterate_dir function.
>> If use_fpos is true it uses the file position in the file
>> structure (existing behavior). If use_fpos is false, it uses
>> the pos in the context structure.
> 
> Looks sane, one question below
> 
>>
>> This change is required to support getdents in io_uring.
>>
>> Signed-off-by: Stefan Roesch <shr@fb.com>
>> ---
>>   fs/exportfs/expfs.c    |  2 +-
>>   fs/nfsd/nfs4recover.c  |  2 +-
>>   fs/nfsd/vfs.c          |  2 +-
>>   fs/overlayfs/readdir.c |  6 +++---
>>   fs/readdir.c           | 28 ++++++++++++++++++++--------
>>   include/linux/fs.h     |  2 +-
>>   6 files changed, 27 insertions(+), 15 deletions(-)
>>
> [...]
>> diff --git a/fs/readdir.c b/fs/readdir.c
>> index 09e8ed7d4161..8ea5b5f45a78 100644
>> --- a/fs/readdir.c
>> +++ b/fs/readdir.c
>> @@ -21,6 +21,7 @@
>>   #include <linux/unistd.h>
>>   #include <linux/compat.h>
>>   #include <linux/uaccess.h>
>> +#include "internal.h"
> 
> Don't see this header is used in this patch. Just to be clear,
> it is here only for 2/3, right?
>

This is not needed. It will be removed with the next version (v3)
of the patch.
 
> [...]
> 

Re: [PATCH v1 1/3] fs: add parameter use_fpos to iterate_dir function

[Pavel Begunkov]

On 11/23/21 18:10, Stefan Roesch wrote:
> This adds the use_fpos parameter to the iterate_dir function.
> If use_fpos is true it uses the file position in the file
> structure (existing behavior). If use_fpos is false, it uses
> the pos in the context structure.
> 
> This change is required to support getdents in io_uring.
> 
> Signed-off-by: Stefan Roesch <shr@fb.com>
> ---
>   fs/exportfs/expfs.c    |  2 +-
>   fs/nfsd/nfs4recover.c  |  2 +-
>   fs/nfsd/vfs.c          |  2 +-
>   fs/overlayfs/readdir.c |  6 +++---
>   fs/readdir.c           | 28 ++++++++++++++++++++--------
>   include/linux/fs.h     |  2 +-
>   6 files changed, 27 insertions(+), 15 deletions(-)
> 
[...]
> diff --git a/fs/readdir.c b/fs/readdir.c
> index 09e8ed7d4161..8ea5b5f45a78 100644
> --- a/fs/readdir.c
> +++ b/fs/readdir.c
> @@ -21,6 +21,7 @@
>   #include <linux/unistd.h>
>   #include <linux/compat.h>
>   #include <linux/uaccess.h>
> +#include "internal.h"
>   
>   #include <asm/unaligned.h>
>   
> @@ -36,8 +37,14 @@
>   	unsafe_copy_to_user(dst, src, len, label);		\
>   } while (0)
>   
> -
> -int iterate_dir(struct file *file, struct dir_context *ctx)
> +/**
> + * iterate_dir - iterate over directory
> + * @file    : pointer to file struct of directory
> + * @ctx     : pointer to directory ctx structure
> + * @use_fpos: true : use file offset
> + *            false: use pos in ctx structure
> + */
> +int iterate_dir(struct file *file, struct dir_context *ctx, bool use_fpos)
>   {
>   	struct inode *inode = file_inode(file);
>   	bool shared = false;
> @@ -60,12 +67,17 @@ int iterate_dir(struct file *file, struct dir_context *ctx)
>   
>   	res = -ENOENT;
>   	if (!IS_DEADDIR(inode)) {
> -		ctx->pos = file->f_pos;
> +		if (use_fpos)
> +			ctx->pos = file->f_pos;

One more thing I haven't noticed before, should pos be sanitised
somehow if passed from the userspace? Do filesystems handle it
well?


> +
>   		if (shared)
>   			res = file->f_op->iterate_shared(file, ctx);
>   		else
>   			res = file->f_op->iterate(file, ctx);
> -		file->f_pos = ctx->pos;
> +
> +		if (use_fpos)
> +			file->f_pos = ctx->pos;
> +
>   		fsnotify_access(file);
>   		file_accessed(file);
>   	}


-- 
Pavel Begunkov

Re: [PATCH v1 1/3] fs: add parameter use_fpos to iterate_dir function

[Stefan Roesch]

On 11/25/21 7:55 AM, Pavel Begunkov wrote:
> On 11/23/21 18:10, Stefan Roesch wrote:
>> This adds the use_fpos parameter to the iterate_dir function.
>> If use_fpos is true it uses the file position in the file
>> structure (existing behavior). If use_fpos is false, it uses
>> the pos in the context structure.
>>
>> This change is required to support getdents in io_uring.
>>
>> Signed-off-by: Stefan Roesch <shr@fb.com>
>> ---
>>   fs/exportfs/expfs.c    |  2 +-
>>   fs/nfsd/nfs4recover.c  |  2 +-
>>   fs/nfsd/vfs.c          |  2 +-
>>   fs/overlayfs/readdir.c |  6 +++---
>>   fs/readdir.c           | 28 ++++++++++++++++++++--------
>>   include/linux/fs.h     |  2 +-
>>   6 files changed, 27 insertions(+), 15 deletions(-)
>>
> [...]
>> diff --git a/fs/readdir.c b/fs/readdir.c
>> index 09e8ed7d4161..8ea5b5f45a78 100644
>> --- a/fs/readdir.c
>> +++ b/fs/readdir.c
>> @@ -21,6 +21,7 @@
>>   #include <linux/unistd.h>
>>   #include <linux/compat.h>
>>   #include <linux/uaccess.h>
>> +#include "internal.h"
>>     #include <asm/unaligned.h>
>>   @@ -36,8 +37,14 @@
>>       unsafe_copy_to_user(dst, src, len, label);        \
>>   } while (0)
>>   -
>> -int iterate_dir(struct file *file, struct dir_context *ctx)
>> +/**
>> + * iterate_dir - iterate over directory
>> + * @file    : pointer to file struct of directory
>> + * @ctx     : pointer to directory ctx structure
>> + * @use_fpos: true : use file offset
>> + *            false: use pos in ctx structure
>> + */
>> +int iterate_dir(struct file *file, struct dir_context *ctx, bool use_fpos)
>>   {
>>       struct inode *inode = file_inode(file);
>>       bool shared = false;
>> @@ -60,12 +67,17 @@ int iterate_dir(struct file *file, struct dir_context *ctx)
>>         res = -ENOENT;
>>       if (!IS_DEADDIR(inode)) {
>> -        ctx->pos = file->f_pos;
>> +        if (use_fpos)
>> +            ctx->pos = file->f_pos;
> 
> One more thing I haven't noticed before, should pos be sanitised
> somehow if passed from the userspace? Do filesystems handle it
> well?
> 

I checked a couple of filesystems and they all check that the pos value is reasonable.

> 
>> +
>>           if (shared)
>>               res = file->f_op->iterate_shared(file, ctx);
>>           else
>>               res = file->f_op->iterate(file, ctx);
>> -        file->f_pos = ctx->pos;
>> +
>> +        if (use_fpos)
>> +            file->f_pos = ctx->pos;
>> +
>>           fsnotify_access(file);
>>           file_accessed(file);
>>       }
> 
> 

[PATCH v1 2/3] fs: split off vfs_getdents function of getdents64 syscall

[Stefan Roesch]

This splits off the vfs_getdents function from the getdents64 system
call. This allows io_uring to call the function.

Signed-off-by: Stefan Roesch <shr@fb.com>
---
 fs/internal.h |  8 ++++++++
 fs/readdir.c  | 36 ++++++++++++++++++++++++++++--------
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 7979ff8d168c..355be993b9f1 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -194,3 +194,11 @@ long splice_file_to_pipe(struct file *in,
 			 struct pipe_inode_info *opipe,
 			 loff_t *offset,
 			 size_t len, unsigned int flags);
+
+/*
+ * fs/readdir.c
+ */
+struct linux_dirent64;
+
+int vfs_getdents(struct file *file, struct linux_dirent64 __user *dirent,
+		 unsigned int count, s64 pos);
diff --git a/fs/readdir.c b/fs/readdir.c
index 8ea5b5f45a78..fc5b50fb160b 100644
--- a/fs/readdir.c
+++ b/fs/readdir.c
@@ -363,22 +363,26 @@ static int filldir64(struct dir_context *ctx, const char *name, int namlen,
 	return -EFAULT;
 }
 
-SYSCALL_DEFINE3(getdents64, unsigned int, fd,
-		struct linux_dirent64 __user *, dirent, unsigned int, count)
+/**
+ * vfs_getdents - getdents without fdget
+ * @file    : pointer to file struct of directory
+ * @dirent  : pointer to user directory structure
+ * @count   : size of buffer
+ * @ctx_pos : if file pos is used, pass -1,
+ *            if ctx pos is used, pass ctx pos
+ */
+int vfs_getdents(struct file *file, struct linux_dirent64 __user *dirent,
+		 unsigned int count, s64 ctx_pos)
 {
-	struct fd f;
 	struct getdents_callback64 buf = {
 		.ctx.actor = filldir64,
+		.ctx.pos = ctx_pos,
 		.count = count,
 		.current_dir = dirent
 	};
 	int error;
 
-	f = fdget_pos(fd);
-	if (!f.file)
-		return -EBADF;
-
-	error = iterate_dir(f.file, &buf.ctx);
+	error = iterate_dir(file, &buf.ctx, ctx_pos < 0);
 	if (error >= 0)
 		error = buf.error;
 	if (buf.prev_reclen) {
@@ -391,6 +395,22 @@ SYSCALL_DEFINE3(getdents64, unsigned int, fd,
 		else
 			error = count - buf.count;
 	}
+
+	return error;
+}
+
+SYSCALL_DEFINE3(getdents64, unsigned int, fd,
+		struct linux_dirent64 __user *, dirent, unsigned int, count)
+{
+	struct fd f;
+	int error;
+
+	f = fdget_pos(fd);
+	if (!f.file)
+		return -EBADF;
+
+	error = vfs_getdents(f.file, dirent, count, -1);
+
 	fdput_pos(f);
 	return error;
 }
-- 
2.30.2

Re: [PATCH v1 2/3] fs: split off vfs_getdents function of getdents64 syscall

[Christian Brauner]

On Tue, Nov 23, 2021 at 10:10:09AM -0800, Stefan Roesch wrote:
> This splits off the vfs_getdents function from the getdents64 system
> call. This allows io_uring to call the function.
> 
> Signed-off-by: Stefan Roesch <shr@fb.com>
> ---

Looks good.
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>

[PATCH v1 3/3] io_uring: add support for getdents64

[Stefan Roesch]

This adds support for getdents64 to io_uring.

Signed-off-by: Stefan Roesch <shr@fb.com>
---
 fs/io_uring.c                 | 52 +++++++++++++++++++++++++++++++++++
 include/uapi/linux/io_uring.h |  1 +
 2 files changed, 53 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index b07196b4511c..b19fa94bcd95 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -688,6 +688,13 @@ struct io_hardlink {
 	int				flags;
 };
 
+struct io_getdents {
+	struct file			*file;
+	struct linux_dirent64 __user	*dirent;
+	unsigned int			count;
+	loff_t				pos;
+};
+
 struct io_async_connect {
 	struct sockaddr_storage		address;
 };
@@ -847,6 +854,7 @@ struct io_kiocb {
 		struct io_mkdir		mkdir;
 		struct io_symlink	symlink;
 		struct io_hardlink	hardlink;
+		struct io_getdents	getdents;
 	};
 
 	u8				opcode;
@@ -1096,6 +1104,9 @@ static const struct io_op_def io_op_defs[] = {
 	[IORING_OP_MKDIRAT] = {},
 	[IORING_OP_SYMLINKAT] = {},
 	[IORING_OP_LINKAT] = {},
+	[IORING_OP_GETDENTS] = {
+		.needs_file		= 1,
+	},
 };
 
 /* requests with any of those set should undergo io_disarm_next() */
@@ -3940,6 +3951,42 @@ static int io_linkat(struct io_kiocb *req, unsigned int issue_flags)
 	return 0;
 }
 
+static int io_getdents_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+	struct io_getdents *getdents = &req->getdents;
+
+	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
+		return -EINVAL;
+	if (sqe->ioprio || sqe->rw_flags || sqe->buf_index)
+		return -EINVAL;
+
+	getdents->pos = READ_ONCE(sqe->off);
+	getdents->dirent = u64_to_user_ptr(READ_ONCE(sqe->addr));
+	getdents->count = READ_ONCE(sqe->len);
+
+	return 0;
+}
+
+static int io_getdents(struct io_kiocb *req, unsigned int issue_flags)
+{
+	struct io_getdents *getdents = &req->getdents;
+	int ret;
+
+	if (issue_flags & IO_URING_F_NONBLOCK)
+		return -EAGAIN;
+
+	ret = vfs_getdents(req->file, getdents->dirent, getdents->count, getdents->pos);
+	if (ret < 0) {
+		if (ret == -ERESTARTSYS)
+			ret = -EINTR;
+
+		req_set_fail(req);
+	}
+
+	io_req_complete(req, ret);
+	return 0;
+}
+
 static int io_shutdown_prep(struct io_kiocb *req,
 			    const struct io_uring_sqe *sqe)
 {
@@ -6446,6 +6493,8 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		return io_symlinkat_prep(req, sqe);
 	case IORING_OP_LINKAT:
 		return io_linkat_prep(req, sqe);
+	case IORING_OP_GETDENTS:
+		return io_getdents_prep(req, sqe);
 	}
 
 	printk_once(KERN_WARNING "io_uring: unhandled opcode %d\n",
@@ -6728,6 +6777,9 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
 	case IORING_OP_LINKAT:
 		ret = io_linkat(req, issue_flags);
 		break;
+	case IORING_OP_GETDENTS:
+		ret = io_getdents(req, issue_flags);
+		break;
 	default:
 		ret = -EINVAL;
 		break;
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index c45b5e9a9387..792875075a2f 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -140,6 +140,7 @@ enum {
 	IORING_OP_MKDIRAT,
 	IORING_OP_SYMLINKAT,
 	IORING_OP_LINKAT,
+	IORING_OP_GETDENTS,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
-- 
2.30.2

Re: [PATCH v1 3/3] io_uring: add support for getdents64

[Pavel Begunkov]

On 11/23/21 18:10, Stefan Roesch wrote:
> This adds support for getdents64 to io_uring.

Looks good,
Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>

> 
> Signed-off-by: Stefan Roesch <shr@fb.com>
> ---
>   fs/io_uring.c                 | 52 +++++++++++++++++++++++++++++++++++
>   include/uapi/linux/io_uring.h |  1 +
>   2 files changed, 53 insertions(+)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index b07196b4511c..b19fa94bcd95 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -688,6 +688,13 @@ struct io_hardlink {
>   	int				flags;
>   };
>   
> +struct io_getdents {
> +	struct file			*file;
> +	struct linux_dirent64 __user	*dirent;
> +	unsigned int			count;
> +	loff_t				pos;
> +};
> +
>   struct io_async_connect {
>   	struct sockaddr_storage		address;
>   };
> @@ -847,6 +854,7 @@ struct io_kiocb {
>   		struct io_mkdir		mkdir;
>   		struct io_symlink	symlink;
>   		struct io_hardlink	hardlink;
> +		struct io_getdents	getdents;
>   	};
>   
>   	u8				opcode;
> @@ -1096,6 +1104,9 @@ static const struct io_op_def io_op_defs[] = {
>   	[IORING_OP_MKDIRAT] = {},
>   	[IORING_OP_SYMLINKAT] = {},
>   	[IORING_OP_LINKAT] = {},
> +	[IORING_OP_GETDENTS] = {
> +		.needs_file		= 1,
> +	},
>   };
>   
>   /* requests with any of those set should undergo io_disarm_next() */
> @@ -3940,6 +3951,42 @@ static int io_linkat(struct io_kiocb *req, unsigned int issue_flags)
>   	return 0;
>   }
>   
> +static int io_getdents_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> +{
> +	struct io_getdents *getdents = &req->getdents;
> +
> +	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
> +		return -EINVAL;
> +	if (sqe->ioprio || sqe->rw_flags || sqe->buf_index)
> +		return -EINVAL;
> +
> +	getdents->pos = READ_ONCE(sqe->off);
> +	getdents->dirent = u64_to_user_ptr(READ_ONCE(sqe->addr));
> +	getdents->count = READ_ONCE(sqe->len);
> +
> +	return 0;
> +}
> +
> +static int io_getdents(struct io_kiocb *req, unsigned int issue_flags)
> +{
> +	struct io_getdents *getdents = &req->getdents;
> +	int ret;
> +
> +	if (issue_flags & IO_URING_F_NONBLOCK)
> +		return -EAGAIN;
> +
> +	ret = vfs_getdents(req->file, getdents->dirent, getdents->count, getdents->pos);
> +	if (ret < 0) {
> +		if (ret == -ERESTARTSYS)
> +			ret = -EINTR;
> +
> +		req_set_fail(req);
> +	}
> +
> +	io_req_complete(req, ret);
> +	return 0;
> +}
> +
>   static int io_shutdown_prep(struct io_kiocb *req,
>   			    const struct io_uring_sqe *sqe)
>   {
> @@ -6446,6 +6493,8 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>   		return io_symlinkat_prep(req, sqe);
>   	case IORING_OP_LINKAT:
>   		return io_linkat_prep(req, sqe);
> +	case IORING_OP_GETDENTS:
> +		return io_getdents_prep(req, sqe);
>   	}
>   
>   	printk_once(KERN_WARNING "io_uring: unhandled opcode %d\n",
> @@ -6728,6 +6777,9 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
>   	case IORING_OP_LINKAT:
>   		ret = io_linkat(req, issue_flags);
>   		break;
> +	case IORING_OP_GETDENTS:
> +		ret = io_getdents(req, issue_flags);
> +		break;
>   	default:
>   		ret = -EINVAL;
>   		break;
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index c45b5e9a9387..792875075a2f 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -140,6 +140,7 @@ enum {
>   	IORING_OP_MKDIRAT,
>   	IORING_OP_SYMLINKAT,
>   	IORING_OP_LINKAT,
> +	IORING_OP_GETDENTS,
>   
>   	/* this goes last, obviously */
>   	IORING_OP_LAST,
> 

-- 
Pavel Begunkov