| Message ID | 20230711114027.59945-2-hao.xu@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | io_uring getdents | expand |
On Tue, Jul 11, 2023 at 07:40:25PM +0800, Hao Xu wrote: > This splits off the vfs_getdents function from the getdents64 system > call. > This will allow io_uring to call the vfs_getdents function. > > Co-developed-by: Stefan Roesch <shr@fb.com> > Signed-off-by: Stefan Roesch <shr@fb.com> > Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> > --- Since you took this, it needs your Signed-off-by.
On Tue, Jul 11, 2023 at 07:40:25PM +0800, Hao Xu wrote: > From: Dominique Martinet <asmadeus@codewreck.org> > > This splits off the vfs_getdents function from the getdents64 system > call. > This will allow io_uring to call the vfs_getdents function. > > Co-developed-by: Stefan Roesch <shr@fb.com> > Signed-off-by: Stefan Roesch <shr@fb.com> > Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> > --- > fs/internal.h | 8 ++++++++ > fs/readdir.c | 34 ++++++++++++++++++++++++++-------- > 2 files changed, 34 insertions(+), 8 deletions(-) > > diff --git a/fs/internal.h b/fs/internal.h > index f7a3dc111026..b1f66e52d61b 100644 > --- a/fs/internal.h > +++ b/fs/internal.h > @@ -304,3 +304,11 @@ ssize_t __kernel_write_iter(struct file *file, struct iov_iter *from, loff_t *po > struct mnt_idmap *alloc_mnt_idmap(struct user_namespace *mnt_userns); > struct mnt_idmap *mnt_idmap_get(struct mnt_idmap *idmap); > void mnt_idmap_put(struct mnt_idmap *idmap); > + > +/* > + * fs/readdir.c > + */ > +struct linux_dirent64; > + > +int vfs_getdents(struct file *file, struct linux_dirent64 __user *dirent, > + unsigned int count); Uh... Since when have we allowed code outside fs/ to use fs/internal.h? -Dave.
On 7/11/23 5:41?PM, Dave Chinner wrote: > On Tue, Jul 11, 2023 at 07:40:25PM +0800, Hao Xu wrote: >> From: Dominique Martinet <asmadeus@codewreck.org> >> >> This splits off the vfs_getdents function from the getdents64 system >> call. >> This will allow io_uring to call the vfs_getdents function. >> >> Co-developed-by: Stefan Roesch <shr@fb.com> >> Signed-off-by: Stefan Roesch <shr@fb.com> >> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> >> --- >> fs/internal.h | 8 ++++++++ >> fs/readdir.c | 34 ++++++++++++++++++++++++++-------- >> 2 files changed, 34 insertions(+), 8 deletions(-) >> >> diff --git a/fs/internal.h b/fs/internal.h >> index f7a3dc111026..b1f66e52d61b 100644 >> --- a/fs/internal.h >> +++ b/fs/internal.h >> @@ -304,3 +304,11 @@ ssize_t __kernel_write_iter(struct file *file, struct iov_iter *from, loff_t *po >> struct mnt_idmap *alloc_mnt_idmap(struct user_namespace *mnt_userns); >> struct mnt_idmap *mnt_idmap_get(struct mnt_idmap *idmap); >> void mnt_idmap_put(struct mnt_idmap *idmap); >> + >> +/* >> + * fs/readdir.c >> + */ >> +struct linux_dirent64; >> + >> +int vfs_getdents(struct file *file, struct linux_dirent64 __user *dirent, >> + unsigned int count); > > Uh... > > Since when have we allowed code outside fs/ to use fs/internal.h? io_uring does use for things like open/close, statx, and xattr already.
On 7/11/23 21:02, Ammar Faizi wrote: > On Tue, Jul 11, 2023 at 07:40:25PM +0800, Hao Xu wrote: >> This splits off the vfs_getdents function from the getdents64 system >> call. >> This will allow io_uring to call the vfs_getdents function. >> >> Co-developed-by: Stefan Roesch <shr@fb.com> >> Signed-off-by: Stefan Roesch <shr@fb.com> >> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> >> --- > > Since you took this, it needs your Signed-off-by. > Hi Ammar, I just add this signed-off-by of Stefan to resolve the checkpatch complain, no code change.
On Tue, Jul 11, 2023 at 05:50:27PM -0600, Jens Axboe wrote: > On 7/11/23 5:41?PM, Dave Chinner wrote: > > On Tue, Jul 11, 2023 at 07:40:25PM +0800, Hao Xu wrote: > >> From: Dominique Martinet <asmadeus@codewreck.org> > >> > >> This splits off the vfs_getdents function from the getdents64 system > >> call. > >> This will allow io_uring to call the vfs_getdents function. > >> > >> Co-developed-by: Stefan Roesch <shr@fb.com> > >> Signed-off-by: Stefan Roesch <shr@fb.com> > >> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> > >> --- > >> fs/internal.h | 8 ++++++++ > >> fs/readdir.c | 34 ++++++++++++++++++++++++++-------- > >> 2 files changed, 34 insertions(+), 8 deletions(-) > >> > >> diff --git a/fs/internal.h b/fs/internal.h > >> index f7a3dc111026..b1f66e52d61b 100644 > >> --- a/fs/internal.h > >> +++ b/fs/internal.h > >> @@ -304,3 +304,11 @@ ssize_t __kernel_write_iter(struct file *file, struct iov_iter *from, loff_t *po > >> struct mnt_idmap *alloc_mnt_idmap(struct user_namespace *mnt_userns); > >> struct mnt_idmap *mnt_idmap_get(struct mnt_idmap *idmap); > >> void mnt_idmap_put(struct mnt_idmap *idmap); > >> + > >> +/* > >> + * fs/readdir.c > >> + */ > >> +struct linux_dirent64; > >> + > >> +int vfs_getdents(struct file *file, struct linux_dirent64 __user *dirent, > >> + unsigned int count); > > > > Uh... > > > > Since when have we allowed code outside fs/ to use fs/internal.h? > > io_uring does use for things like open/close, statx, and xattr already. Arguably though because you io_uring once used to be located under fs/. In general though, we don't support anyone outside of fs/ to use that header.
On Wed, Jul 12, 2023 at 04:03:41PM +0800, Hao Xu wrote: > On 7/11/23 21:02, Ammar Faizi wrote: > > On Tue, Jul 11, 2023 at 07:40:25PM +0800, Hao Xu wrote: > > > Co-developed-by: Stefan Roesch <shr@fb.com> > > > Signed-off-by: Stefan Roesch <shr@fb.com> > > > Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> > > > --- > > > > Since you took this, it needs your Signed-off-by. > > > > Hi Ammar, > I just add this signed-off-by of Stefan to resolve the checkpatch complain, > no code change. Both, you and Stefan are required to sign-off. The submitter is also required to sign-off even if the submitter makes no code change. See https://www.kernel.org/doc/html/latest/process/submitting-patches.html: """ Any further SoBs (Signed-off-by:'s) following the author's SoB are from people handling and transporting the patch, but were not involved in its development. SoB chains should reflect the real route a patch took as it was propagated to the maintainers and ultimately to Linus, with the first SoB entry signalling primary authorship of a single author. """ It also applies to the maintainer when they apply your patches.
On 7/12/23 21:55, Ammar Faizi wrote: > On Wed, Jul 12, 2023 at 04:03:41PM +0800, Hao Xu wrote: >> On 7/11/23 21:02, Ammar Faizi wrote: >>> On Tue, Jul 11, 2023 at 07:40:25PM +0800, Hao Xu wrote: >>>> Co-developed-by: Stefan Roesch <shr@fb.com> >>>> Signed-off-by: Stefan Roesch <shr@fb.com> >>>> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> >>>> --- >>> Since you took this, it needs your Signed-off-by. >>> >> Hi Ammar, >> I just add this signed-off-by of Stefan to resolve the checkpatch complain, >> no code change. > Both, you and Stefan are required to sign-off. The submitter is also > required to sign-off even if the submitter makes no code change. > > See https://www.kernel.org/doc/html/latest/process/submitting-patches.html: > """ > Any further SoBs (Signed-off-by:'s) following the author's SoB are from > people handling and transporting the patch, but were not involved in its > development. SoB chains should reflect the real route a patch took as it > was propagated to the maintainers and ultimately to Linus, with the > first SoB entry signalling primary authorship of a single author. > """ > > It also applies to the maintainer when they apply your patches. Hi Ammar, I see, this information is really helpful, I'll fix it in next version to make it more standardized. Thanks, Hao
diff --git a/fs/internal.h b/fs/internal.h index f7a3dc111026..b1f66e52d61b 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -304,3 +304,11 @@ ssize_t __kernel_write_iter(struct file *file, struct iov_iter *from, loff_t *po struct mnt_idmap *alloc_mnt_idmap(struct user_namespace *mnt_userns); struct mnt_idmap *mnt_idmap_get(struct mnt_idmap *idmap); void mnt_idmap_put(struct mnt_idmap *idmap); + +/* + * fs/readdir.c + */ +struct linux_dirent64; + +int vfs_getdents(struct file *file, struct linux_dirent64 __user *dirent, + unsigned int count); diff --git a/fs/readdir.c b/fs/readdir.c index b264ce60114d..9592259b7e7f 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> @@ -351,10 +352,16 @@ static bool filldir64(struct dir_context *ctx, const char *name, int namlen, return false; } -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 + */ +int vfs_getdents(struct file *file, struct linux_dirent64 __user *dirent, + unsigned int count) { - struct fd f; struct getdents_callback64 buf = { .ctx.actor = filldir64, .count = count, @@ -362,11 +369,7 @@ SYSCALL_DEFINE3(getdents64, unsigned int, fd, }; int error; - f = fdget_pos(fd); - if (!f.file) - return -EBADF; - - error = iterate_dir(f.file, &buf.ctx); + error = iterate_dir(file, &buf.ctx); if (error >= 0) error = buf.error; if (buf.prev_reclen) { @@ -379,6 +382,21 @@ 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); + fdput_pos(f); return error; }