On Fri, Nov 24, 2023 at 6:21 AM Jan Kara <jack@xxxxxxx> wrote:
>
> On Wed 22-11-23 14:27:11, Amir Goldstein wrote:
> > We recently moved fsnotify hook, rw_verify_area() and other checks from
> > do_iter_write() out to its two callers.
> >
> > for consistency, do the same thing for do_iter_read() - move the
> ^^^ For
>
> > rw_verify_area() checks and fsnotify hook to the callers vfs_iter_read()
> > and vfs_readv().
> >
> > This aligns those vfs helpers with the pattern used in vfs_read() and
> > vfs_iocb_iter_read() and the vfs write helpers, where all the checks are
> > in the vfs helpers and the do_* or call_* helpers do the work.
> >
> > This is needed for fanotify "pre content" events.
> >
> > Suggested-by: Jan Kara <jack@xxxxxxx>
> > Reviewed-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>
> Also one nit below. Otherwise feel free to add:
>
> Reviewed-by: Jan Kara <jack@xxxxxxx>
>
> > +/*
> > + * Low-level helpers don't perform rw sanity checks.
> > + * The caller is responsible for that.
> > + */
> > static ssize_t do_iter_read(struct file *file, struct iov_iter *iter,
> > - loff_t *pos, rwf_t flags)
> > + loff_t *pos, rwf_t flags)
> > +{
> > + if (file->f_op->read_iter)
> > + return do_iter_readv_writev(file, iter, pos, READ, flags);
> > +
> > + return do_loop_readv_writev(file, iter, pos, READ, flags);
> > +}
>
> Similarly as with do_iter_write() I don't think there's much point in this
> helper when there's actually only one real user, the other one can just
> call do_iter_readv_writev().
Yeh, nice.
Christian,
Can you please fold this patch:
BTW, both patches need a very mild edit of commit message
to mention removal of do_iter_{read/write}() and the minor
spelling mistake fixes pointed out by Jan.
Thanks,
Amir.
diff --git a/fs/read_write.c b/fs/read_write.c
index 8358ace9282e..2953fea9b65b 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -784,19 +784,6 @@ static ssize_t do_loop_readv_writev(struct file
*filp, struct iov_iter *iter,
return ret;
}
-/*
- * Low-level helpers don't perform rw sanity checks.
- * The caller is responsible for that.
- */
-static ssize_t do_iter_read(struct file *file, struct iov_iter *iter,
- loff_t *pos, rwf_t flags)
-{
- if (file->f_op->read_iter)
- return do_iter_readv_writev(file, iter, pos, READ, flags);
-
- return do_loop_readv_writev(file, iter, pos, READ, flags);
-}
-
ssize_t vfs_iocb_iter_read(struct file *file, struct kiocb *iocb,
struct iov_iter *iter)
{
@@ -845,7 +832,7 @@ ssize_t vfs_iter_read(struct file *file, struct
iov_iter *iter, loff_t *ppos,
if (ret < 0)
return ret;
- ret = do_iter_read(file, iter, ppos, flags);
+ ret = do_iter_readv_writev(file, iter, ppos, READ, flags);
out:
if (ret >= 0)
fsnotify_access(file);
@@ -939,7 +926,10 @@ static ssize_t vfs_readv(struct file *file, const
struct iovec __user *vec,
if (ret < 0)
goto out;
- ret = do_iter_read(file, &iter, pos, flags);
+ if (file->f_op->read_iter)
+ ret = do_iter_readv_writev(file, &iter, pos, READ, flags);
+ else
+ ret = do_loop_readv_writev(file, &iter, pos, READ, flags);
out:
if (ret >= 0)
fsnotify_access(file);
--
