| Message ID | 20230828155056.4100924-1-shikemeng@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfs: use helpers for calling f_op->{read,write}_iter() in read_write.c | expand |
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.
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...
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.
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.
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.
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.
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)
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;
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(-)