Message ID | 20190310070822.11564-3-viro@ZenIV.linux.org.uk |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
Series | [1/8] pin iocb through aio. | expand |
On Sun, Mar 10, 2019 at 07:08:17AM +0000, Al Viro wrote: > From: Al Viro <viro@zeniv.linux.org.uk> > > Instead of having aio_complete() set ->ki_res.{res,res2}, do that > explicitly in its callers, drop the reference (as aio_complete() > used to do) and delay the rest until the final iocb_put(). > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- > fs/aio.c | 45 ++++++++++++++++++++------------------------- > 1 file changed, 20 insertions(+), 25 deletions(-) > > diff --git a/fs/aio.c b/fs/aio.c > index 2249a7a1d6b3..b9c4c1894020 100644 > --- a/fs/aio.c > +++ b/fs/aio.c > @@ -1077,24 +1077,10 @@ static inline void iocb_destroy(struct aio_kiocb *iocb) > kmem_cache_free(kiocb_cachep, iocb); > } > > -static inline void iocb_put(struct aio_kiocb *iocb) > -{ > - if (refcount_dec_and_test(&iocb->ki_refcnt)) > - iocb_destroy(iocb); > -} Maybe iocb_put should just have been added in the place you move it to in patch 1?
On Mon, Mar 11, 2019 at 08:44:31PM +0100, Christoph Hellwig wrote: > On Sun, Mar 10, 2019 at 07:08:17AM +0000, Al Viro wrote: > > From: Al Viro <viro@zeniv.linux.org.uk> > > > > Instead of having aio_complete() set ->ki_res.{res,res2}, do that > > explicitly in its callers, drop the reference (as aio_complete() > > used to do) and delay the rest until the final iocb_put(). > > > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > > --- > > fs/aio.c | 45 ++++++++++++++++++++------------------------- > > 1 file changed, 20 insertions(+), 25 deletions(-) > > > > diff --git a/fs/aio.c b/fs/aio.c > > index 2249a7a1d6b3..b9c4c1894020 100644 > > --- a/fs/aio.c > > +++ b/fs/aio.c > > @@ -1077,24 +1077,10 @@ static inline void iocb_destroy(struct aio_kiocb *iocb) > > kmem_cache_free(kiocb_cachep, iocb); > > } > > > > -static inline void iocb_put(struct aio_kiocb *iocb) > > -{ > > - if (refcount_dec_and_test(&iocb->ki_refcnt)) > > - iocb_destroy(iocb); > > -} > > Maybe iocb_put should just have been added in the place you move > it to in patch 1? Might as well...
On Mon, Mar 11, 2019 at 09:13:28PM +0000, Al Viro wrote: > On Mon, Mar 11, 2019 at 08:44:31PM +0100, Christoph Hellwig wrote: > > On Sun, Mar 10, 2019 at 07:08:17AM +0000, Al Viro wrote: > > > From: Al Viro <viro@zeniv.linux.org.uk> > > > > > > Instead of having aio_complete() set ->ki_res.{res,res2}, do that > > > explicitly in its callers, drop the reference (as aio_complete() > > > used to do) and delay the rest until the final iocb_put(). > > > > > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > > > --- > > > fs/aio.c | 45 ++++++++++++++++++++------------------------- > > > 1 file changed, 20 insertions(+), 25 deletions(-) > > > > > > diff --git a/fs/aio.c b/fs/aio.c > > > index 2249a7a1d6b3..b9c4c1894020 100644 > > > --- a/fs/aio.c > > > +++ b/fs/aio.c > > > @@ -1077,24 +1077,10 @@ static inline void iocb_destroy(struct aio_kiocb *iocb) > > > kmem_cache_free(kiocb_cachep, iocb); > > > } > > > > > > -static inline void iocb_put(struct aio_kiocb *iocb) > > > -{ > > > - if (refcount_dec_and_test(&iocb->ki_refcnt)) > > > - iocb_destroy(iocb); > > > -} > > > > Maybe iocb_put should just have been added in the place you move > > it to in patch 1? > > Might as well... Actually, that wouldn't be any better - it would need at least a declaration before aio_complete(), since in the original patch aio_complete() calls it. So that wouldn't be less noisy...
diff --git a/fs/aio.c b/fs/aio.c index 2249a7a1d6b3..b9c4c1894020 100644 --- a/fs/aio.c +++ b/fs/aio.c @@ -1077,24 +1077,10 @@ static inline void iocb_destroy(struct aio_kiocb *iocb) kmem_cache_free(kiocb_cachep, iocb); } -static inline void iocb_put(struct aio_kiocb *iocb) -{ - if (refcount_dec_and_test(&iocb->ki_refcnt)) - iocb_destroy(iocb); -} - -static void aio_fill_event(struct io_event *ev, struct aio_kiocb *iocb, - long res, long res2) -{ - iocb->ki_res.res = res; - iocb->ki_res.res2 = res2; - *ev = iocb->ki_res; -} - /* aio_complete * Called when the io request on the given iocb is complete. */ -static void aio_complete(struct aio_kiocb *iocb, long res, long res2) +static void aio_complete(struct aio_kiocb *iocb) { struct kioctx *ctx = iocb->ki_ctx; struct aio_ring *ring; @@ -1118,14 +1104,14 @@ static void aio_complete(struct aio_kiocb *iocb, long res, long res2) ev_page = kmap_atomic(ctx->ring_pages[pos / AIO_EVENTS_PER_PAGE]); event = ev_page + pos % AIO_EVENTS_PER_PAGE; - aio_fill_event(event, iocb, res, res2); + *event = iocb->ki_res; kunmap_atomic(ev_page); flush_dcache_page(ctx->ring_pages[pos / AIO_EVENTS_PER_PAGE]); - pr_debug("%p[%u]: %p: %p %Lx %lx %lx\n", + pr_debug("%p[%u]: %p: %p %Lx %Lx %Lx\n", ctx, tail, iocb, (void __user *)iocb->ki_res.obj, iocb->ki_res.data, - res, res2); + iocb->ki_res.res, iocb->ki_res.res2); /* after flagging the request as done, we * must never even look at it again @@ -1167,7 +1153,14 @@ static void aio_complete(struct aio_kiocb *iocb, long res, long res2) if (waitqueue_active(&ctx->wait)) wake_up(&ctx->wait); - iocb_put(iocb); +} + +static inline void iocb_put(struct aio_kiocb *iocb) +{ + if (refcount_dec_and_test(&iocb->ki_refcnt)) { + aio_complete(iocb); + iocb_destroy(iocb); + } } /* aio_read_events_ring @@ -1441,7 +1434,9 @@ static void aio_complete_rw(struct kiocb *kiocb, long res, long res2) file_end_write(kiocb->ki_filp); } - aio_complete(iocb, res, res2); + iocb->ki_res.res = res; + iocb->ki_res.res2 = res2; + iocb_put(iocb); } static int aio_prep_rw(struct kiocb *req, const struct iocb *iocb) @@ -1589,11 +1584,10 @@ static ssize_t aio_write(struct kiocb *req, const struct iocb *iocb, static void aio_fsync_work(struct work_struct *work) { - struct fsync_iocb *req = container_of(work, struct fsync_iocb, work); - int ret; + struct aio_kiocb *iocb = container_of(work, struct aio_kiocb, fsync.work); - ret = vfs_fsync(req->file, req->datasync); - aio_complete(container_of(req, struct aio_kiocb, fsync), ret, 0); + iocb->ki_res.res = vfs_fsync(iocb->fsync.file, iocb->fsync.datasync); + iocb_put(iocb); } static int aio_fsync(struct fsync_iocb *req, const struct iocb *iocb, @@ -1614,7 +1608,8 @@ static int aio_fsync(struct fsync_iocb *req, const struct iocb *iocb, static inline void aio_poll_complete(struct aio_kiocb *iocb, __poll_t mask) { - aio_complete(iocb, mangle_poll(mask), 0); + iocb->ki_res.res = mangle_poll(mask); + iocb_put(iocb); } static void aio_poll_complete_work(struct work_struct *work)