Re: [PATCH] vfs: use helpers for calling f_op->{read,write}_iter() in read_write.c

Re: [PATCH] vfs: use helpers for calling f_op->{read,write}_iter() in read_write.c

[Christoph Hellwig]

On Mon, Aug 28, 2023 at 11:50:56PM +0800, Kemeng Shi wrote:
> use helpers for calling f_op->{read,write}_iter() in read_write.c
> 

Why?  We really should just remove the completely pointless wrappers
instead.

Re: [PATCH] vfs: use helpers for calling f_op->{read,write}_iter() in read_write.c

[Kemeng Shi]

on 8/28/2023 8:00 PM, Christoph Hellwig wrote:
> On Mon, Aug 28, 2023 at 11:50:56PM +0800, Kemeng Shi wrote:
>> use helpers for calling f_op->{read,write}_iter() in read_write.c
>>
> 
> Why?  We really should just remove the completely pointless wrappers
> instead.
> 
This patch is intended to uniform how we call {read,write}_iter()...
Would it be good to opencode all call_{read,write}_iter. But I'm not sure
if these wrappers are really needless...

Re: [PATCH] vfs: use helpers for calling f_op->{read,write}_iter() in read_write.c

[Matthew Wilcox]

On Mon, Aug 28, 2023 at 05:00:36AM -0700, Christoph Hellwig wrote:
> On Mon, Aug 28, 2023 at 11:50:56PM +0800, Kemeng Shi wrote:
> > use helpers for calling f_op->{read,write}_iter() in read_write.c
> > 
> 
> Why?  We really should just remove the completely pointless wrappers
> instead.

Agreed.

Re: [PATCH] vfs: use helpers for calling f_op->{read,write}_iter() in read_write.c

[Christian Brauner]

On Mon, Aug 28, 2023 at 05:00:36AM -0700, Christoph Hellwig wrote:
> On Mon, Aug 28, 2023 at 11:50:56PM +0800, Kemeng Shi wrote:
> > use helpers for calling f_op->{read,write}_iter() in read_write.c
> > 
> 
> Why?  We really should just remove the completely pointless wrappers
> instead.

Especially because it means you chase this helper to figure out what's
actually going on. If there was more to it then it would make sense but
not just as a pointless wrapper.

Re: [PATCH] vfs: use helpers for calling f_op->{read,write}_iter() in read_write.c

[Matthew Wilcox]

On Mon, Aug 28, 2023 at 08:17:52PM +0800, Kemeng Shi wrote:
> on 8/28/2023 8:00 PM, Christoph Hellwig wrote:
> > On Mon, Aug 28, 2023 at 11:50:56PM +0800, Kemeng Shi wrote:
> >> use helpers for calling f_op->{read,write}_iter() in read_write.c
> >>
> > 
> > Why?  We really should just remove the completely pointless wrappers
> > instead.
> > 
> This patch is intended to uniform how we call {read,write}_iter()...
> Would it be good to opencode all call_{read,write}_iter. But I'm not sure
> if these wrappers are really needless...

All three, call_read_iter(), call_write_iter(), call_mmap()
should go.  Miklos dumped them into the tree without any discussion
that I noticed.

Re: [PATCH] vfs: use helpers for calling f_op->{read,write}_iter() in read_write.c

[Al Viro]

On Mon, Aug 28, 2023 at 02:30:42PM +0200, Christian Brauner wrote:
> On Mon, Aug 28, 2023 at 05:00:36AM -0700, Christoph Hellwig wrote:
> > On Mon, Aug 28, 2023 at 11:50:56PM +0800, Kemeng Shi wrote:
> > > use helpers for calling f_op->{read,write}_iter() in read_write.c
> > > 
> > 
> > Why?  We really should just remove the completely pointless wrappers
> > instead.
> 
> Especially because it means you chase this helper to figure out what's
> actually going on. If there was more to it then it would make sense but
> not just as a pointless wrapper.

It's borderline easier to grep for, but not dramatically so.  call_mmap()
has a stronger argument in favour - there are several methods called
->mmap and telling one from another is hard without looking into context.

For ->{read,write}_iter() I'd prefer to get rid of the wrappers.

RE: [PATCH] vfs: use helpers for calling f_op->{read,write}_iter() in read_write.c

[David Laight]

From: Al Viro
> Sent: 28 August 2023 15:10
> 
> On Mon, Aug 28, 2023 at 02:30:42PM +0200, Christian Brauner wrote:
> > On Mon, Aug 28, 2023 at 05:00:36AM -0700, Christoph Hellwig wrote:
> > > On Mon, Aug 28, 2023 at 11:50:56PM +0800, Kemeng Shi wrote:
> > > > use helpers for calling f_op->{read,write}_iter() in read_write.c
> > > >
> > >
> > > Why?  We really should just remove the completely pointless wrappers
> > > instead.
> >
> > Especially because it means you chase this helper to figure out what's
> > actually going on. If there was more to it then it would make sense but
> > not just as a pointless wrapper.
> 
> It's borderline easier to grep for, but not dramatically so.  call_mmap()
> has a stronger argument in favour - there are several methods called
> ->mmap and telling one from another is hard without looking into context.

Which might be a justification for renaming some/all of the ->mmap()
do make them easier to find.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

[PATCH] vfs: use helpers for calling f_op->{read,write}_iter() in read_write.c

[Kemeng Shi]

use helpers for calling f_op->{read,write}_iter() in read_write.c

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/read_write.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index a21ba3be7dbe..2f816f1212f4 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -425,7 +425,7 @@ ssize_t __kernel_read(struct file *file, void *buf, size_t count, loff_t *pos)
 	init_sync_kiocb(&kiocb, file);
 	kiocb.ki_pos = pos ? *pos : 0;
 	iov_iter_kvec(&iter, ITER_DEST, &iov, 1, iov.iov_len);
-	ret = file->f_op->read_iter(&kiocb, &iter);
+	ret = call_read_iter(file, &kiocb, &iter);
 	if (ret > 0) {
 		if (pos)
 			*pos = kiocb.ki_pos;
@@ -514,7 +514,7 @@ ssize_t __kernel_write_iter(struct file *file, struct iov_iter *from, loff_t *po
 
 	init_sync_kiocb(&kiocb, file);
 	kiocb.ki_pos = pos ? *pos : 0;
-	ret = file->f_op->write_iter(&kiocb, from);
+	ret = call_write_iter(file, &kiocb, from);
 	if (ret > 0) {
 		if (pos)
 			*pos = kiocb.ki_pos;
-- 
2.30.0