| Message ID | 0dc67994b6b2478caa3d96a9e24d2bfb@AcuMS.aculab.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/9,next] mm:process_vm_access Call import_iovec() instead of rw_copy_check_uvector() | expand |
On Tue, Sep 15, 2020 at 02:55:20PM +0000, David Laight wrote: > > This is the only code that relies on import_iovec() returning > iter.count on success. > This allows a better interface to import_iovec(). This looks generall sane, but a comment below: > @@ -3123,7 +3123,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock, > if (ret < 0) > return ret; > iov_count = iov_iter_count(iter); > - io_size = ret; > + io_size = iov_count; > req->result = io_size; > ret = 0; > > @@ -3246,7 +3246,7 @@ static int io_write(struct io_kiocb *req, bool force_nonblock, > if (ret < 0) > return ret; > iov_count = iov_iter_count(iter); > - io_size = ret; > + io_size = iov_count; > req->result = io_size; I tink the local iov_count variable can go away in both functions, as io_size only changes after the last use of iov_count (io_read) or not at all (io_write).
From: Christoph Hellwig > Sent: 21 September 2020 15:15 > > On Tue, Sep 15, 2020 at 02:55:20PM +0000, David Laight wrote: > > > > This is the only code that relies on import_iovec() returning > > iter.count on success. > > This allows a better interface to import_iovec(). > > This looks generall sane, but a comment below: > > > @@ -3123,7 +3123,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock, > > if (ret < 0) > > return ret; > > iov_count = iov_iter_count(iter); > > - io_size = ret; > > + io_size = iov_count; > > req->result = io_size; > > ret = 0; > > > > @@ -3246,7 +3246,7 @@ static int io_write(struct io_kiocb *req, bool force_nonblock, > > if (ret < 0) > > return ret; > > iov_count = iov_iter_count(iter); > > - io_size = ret; > > + io_size = iov_count; > > req->result = io_size; > > I tink the local iov_count variable can go away in both functions, > as io_size only changes after the last use of iov_count (io_read) or > not at all (io_write). Yes, the compiler will probably make that optimisation. I did a minimal change because my head hurts whenever I look at io_uring.c. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On 21/09/2020 17:38, David Laight wrote: > From: Christoph Hellwig >> Sent: 21 September 2020 15:15 >> >> On Tue, Sep 15, 2020 at 02:55:20PM +0000, David Laight wrote: >>> >>> This is the only code that relies on import_iovec() returning >>> iter.count on success. >>> This allows a better interface to import_iovec(). Reviewed-by: Pavel Begunkov <asml.silence@gmail.com> >> >> This looks generall sane, but a comment below: >> >>> @@ -3123,7 +3123,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock, >>> if (ret < 0) >>> return ret; >>> iov_count = iov_iter_count(iter); >>> - io_size = ret; >>> + io_size = iov_count; >>> req->result = io_size; >>> ret = 0; >>> >>> @@ -3246,7 +3246,7 @@ static int io_write(struct io_kiocb *req, bool force_nonblock, >>> if (ret < 0) >>> return ret; >>> iov_count = iov_iter_count(iter); >>> - io_size = ret; >>> + io_size = iov_count; >>> req->result = io_size; >> >> I tink the local iov_count variable can go away in both functions, >> as io_size only changes after the last use of iov_count (io_read) or >> not at all (io_write). Yes, iov_count should be killed, now or later. > > Yes, the compiler will probably make that optimisation. > I did a minimal change because my head hurts whenever I look at io_uring.c. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) > -- Pavel Begunkov
On 15/09/2020 15:55, David Laight wrote: > > This is the only code that relies on import_iovec() returning > iter.count on success. > This allows a better interface to import_iovec(). Seems this got nowhere. I'll pick it and send with some other patches to Jens. > Signed-off-by: David Laight <david.laight@aculab.com> > --- > fs/io_uring.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/io_uring.c b/fs/io_uring.c > index 3790c7fe9fee..0df43882e4b3 100644 > --- a/fs/io_uring.c > +++ b/fs/io_uring.c > @@ -2824,7 +2824,7 @@ static ssize_t __io_import_iovec(int rw, struct io_kiocb *req, > > ret = import_single_range(rw, buf, sqe_len, *iovec, iter); > *iovec = NULL; > - return ret < 0 ? ret : sqe_len; > + return ret; > } > > if (req->flags & REQ_F_BUFFER_SELECT) { > @@ -2853,7 +2853,7 @@ static ssize_t io_import_iovec(int rw, struct io_kiocb *req, > if (!req->io) > return __io_import_iovec(rw, req, iovec, iter, needs_lock); > *iovec = NULL; > - return iov_iter_count(&req->io->rw.iter); > + return 0; > } > > static inline loff_t *io_kiocb_ppos(struct kiocb *kiocb) > @@ -3123,7 +3123,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock, > if (ret < 0) > return ret; > iov_count = iov_iter_count(iter); > - io_size = ret; > + io_size = iov_count; > req->result = io_size; > ret = 0; > > @@ -3246,7 +3246,7 @@ static int io_write(struct io_kiocb *req, bool force_nonblock, > if (ret < 0) > return ret; > iov_count = iov_iter_count(iter); > - io_size = ret; > + io_size = iov_count; > req->result = io_size; > > /* Ensure we clear previously set non-block flag */ > -- Pavel Begunkov
diff --git a/fs/io_uring.c b/fs/io_uring.c index 3790c7fe9fee..0df43882e4b3 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -2824,7 +2824,7 @@ static ssize_t __io_import_iovec(int rw, struct io_kiocb *req, ret = import_single_range(rw, buf, sqe_len, *iovec, iter); *iovec = NULL; - return ret < 0 ? ret : sqe_len; + return ret; } if (req->flags & REQ_F_BUFFER_SELECT) { @@ -2853,7 +2853,7 @@ static ssize_t io_import_iovec(int rw, struct io_kiocb *req, if (!req->io) return __io_import_iovec(rw, req, iovec, iter, needs_lock); *iovec = NULL; - return iov_iter_count(&req->io->rw.iter); + return 0; } static inline loff_t *io_kiocb_ppos(struct kiocb *kiocb) @@ -3123,7 +3123,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock, if (ret < 0) return ret; iov_count = iov_iter_count(iter); - io_size = ret; + io_size = iov_count; req->result = io_size; ret = 0; @@ -3246,7 +3246,7 @@ static int io_write(struct io_kiocb *req, bool force_nonblock, if (ret < 0) return ret; iov_count = iov_iter_count(iter); - io_size = ret; + io_size = iov_count; req->result = io_size; /* Ensure we clear previously set non-block flag */
This is the only code that relies on import_iovec() returning iter.count on success. This allows a better interface to import_iovec(). Signed-off-by: David Laight <david.laight@aculab.com> --- fs/io_uring.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)