From: Jeff Moyer <jmoyer@redhat.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: "linux-fsdevel\@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
"linux-block\@vger.kernel.org" <linux-block@vger.kernel.org>,
linux-aio@kvack.org
Subject: Re: [PATCH] fs: kill unused ret2 argument from iocb->ki_complete()
Date: Wed, 20 Oct 2021 15:11:03 -0400 [thread overview]
Message-ID: <x49zgr3mrzs.fsf@segfault.boston.devel.redhat.com> (raw)
In-Reply-To: <8f5fdbbf-dc66-fabe-db3b-01b2085083b0@kernel.dk> (Jens Axboe's message of "Wed, 20 Oct 2021 12:56:05 -0600")
Jens Axboe <axboe@kernel.dk> writes:
> On 10/20/21 12:41 PM, Jens Axboe wrote:
>> Working on just changing it to a 64-bit type instead, then we can pass
>> in both at once with res2 being the upper 32 bits. That'll keep the same
>> API on the aio side.
>
> Here's that as an incremental. Since we can only be passing in 32-bits
> anyway across 32/64-bit, we can just make it an explicit 64-bit instead.
> This generates the same code on 64-bit for calling ->ki_complete, and we
> can trivially ignore the usb gadget issue as we now can pass in both
> values (and fill them in on the aio side).
Yeah, I think that should work.
Cheers,
Jeff
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 92b87aa8be86..66c6e0c5d638 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -550,7 +550,7 @@ static void lo_rw_aio_do_completion(struct loop_cmd *cmd)
> blk_mq_complete_request(rq);
> }
>
> -static void lo_rw_aio_complete(struct kiocb *iocb, long ret)
> +static void lo_rw_aio_complete(struct kiocb *iocb, u64 ret)
> {
> struct loop_cmd *cmd = container_of(iocb, struct loop_cmd, iocb);
>
> diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
> index 80a0f35ae1dc..83a2f5b0a3a0 100644
> --- a/drivers/nvme/target/io-cmd-file.c
> +++ b/drivers/nvme/target/io-cmd-file.c
> @@ -123,7 +123,7 @@ static ssize_t nvmet_file_submit_bvec(struct nvmet_req *req, loff_t pos,
> return call_iter(iocb, &iter);
> }
>
> -static void nvmet_file_io_done(struct kiocb *iocb, long ret)
> +static void nvmet_file_io_done(struct kiocb *iocb, u64 ret)
> {
> struct nvmet_req *req = container_of(iocb, struct nvmet_req, f.iocb);
> u16 status = NVME_SC_SUCCESS;
> diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
> index 968ace2ddf64..c4ca7fa18e61 100644
> --- a/drivers/target/target_core_file.c
> +++ b/drivers/target/target_core_file.c
> @@ -245,7 +245,7 @@ struct target_core_file_cmd {
> struct bio_vec bvecs[];
> };
>
> -static void cmd_rw_aio_complete(struct kiocb *iocb, long ret)
> +static void cmd_rw_aio_complete(struct kiocb *iocb, u64 ret)
> {
> struct target_core_file_cmd *cmd;
>
> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> index e20c19a0f106..8536f19d3c9a 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -831,7 +831,7 @@ static void ffs_user_copy_worker(struct work_struct *work)
> kthread_unuse_mm(io_data->mm);
> }
>
> - io_data->kiocb->ki_complete(io_data->kiocb, ret);
> + io_data->kiocb->ki_complete(io_data->kiocb, ((u64) ret << 32) | ret);
>
> if (io_data->ffs->ffs_eventfd && !kiocb_has_eventfd)
> eventfd_signal(io_data->ffs->ffs_eventfd, 1);
> diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
> index ad1739dbfab9..d3deb23eb2ab 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
> @@ -469,7 +469,7 @@ static void ep_user_copy_worker(struct work_struct *work)
> ret = -EFAULT;
>
> /* completing the iocb can drop the ctx and mm, don't touch mm after */
> - iocb->ki_complete(iocb, ret);
> + iocb->ki_complete(iocb, ((u64) ret << 32) | ret);
>
> kfree(priv->buf);
> kfree(priv->to_free);
> @@ -492,14 +492,16 @@ static void ep_aio_complete(struct usb_ep *ep, struct usb_request *req)
> * complete the aio request immediately.
> */
> if (priv->to_free == NULL || unlikely(req->actual == 0)) {
> + u64 aio_ret;
> +
> kfree(req->buf);
> kfree(priv->to_free);
> kfree(priv);
> iocb->private = NULL;
> /* aio_complete() reports bytes-transferred _and_ faults */
> -
> - iocb->ki_complete(iocb,
> - req->actual ? req->actual : (long)req->status);
> + aio_ret = req->actual ? req->actual : (long)req->status;
> + aio_ret |= (u64) req->status << 32;
> + iocb->ki_complete(iocb, aio_ret);
> } else {
> /* ep_copy_to_user() won't report both; we hide some faults */
> if (unlikely(0 != req->status))
> diff --git a/fs/aio.c b/fs/aio.c
> index 836dc7e48db7..e39c61dccf37 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1417,7 +1417,7 @@ static void aio_remove_iocb(struct aio_kiocb *iocb)
> spin_unlock_irqrestore(&ctx->ctx_lock, flags);
> }
>
> -static void aio_complete_rw(struct kiocb *kiocb, long res)
> +static void aio_complete_rw(struct kiocb *kiocb, u64 res)
> {
> struct aio_kiocb *iocb = container_of(kiocb, struct aio_kiocb, rw);
>
> @@ -1436,8 +1436,8 @@ static void aio_complete_rw(struct kiocb *kiocb, long res)
> file_end_write(kiocb->ki_filp);
> }
>
> - iocb->ki_res.res = res;
> - iocb->ki_res.res2 = 0;
> + iocb->ki_res.res = res & 0xffffffff;
> + iocb->ki_res.res2 = res >> 32;
> iocb_put(iocb);
> }
>
> diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c
> index effe37ef8629..b2f44ff8eae2 100644
> --- a/fs/cachefiles/io.c
> +++ b/fs/cachefiles/io.c
> @@ -37,11 +37,11 @@ static inline void cachefiles_put_kiocb(struct cachefiles_kiocb *ki)
> /*
> * Handle completion of a read from the cache.
> */
> -static void cachefiles_read_complete(struct kiocb *iocb, long ret)
> +static void cachefiles_read_complete(struct kiocb *iocb, u64 ret)
> {
> struct cachefiles_kiocb *ki = container_of(iocb, struct cachefiles_kiocb, iocb);
>
> - _enter("%ld", ret);
> + _enter("%llu", (unsigned long long) ret);
>
> if (ki->term_func) {
> if (ret >= 0)
> @@ -159,12 +159,12 @@ static int cachefiles_read(struct netfs_cache_resources *cres,
> /*
> * Handle completion of a write to the cache.
> */
> -static void cachefiles_write_complete(struct kiocb *iocb, long ret)
> +static void cachefiles_write_complete(struct kiocb *iocb, u64 ret)
> {
> struct cachefiles_kiocb *ki = container_of(iocb, struct cachefiles_kiocb, iocb);
> struct inode *inode = file_inode(ki->iocb.ki_filp);
>
> - _enter("%ld", ret);
> + _enter("%llu", (unsigned long long) ret);
>
> /* Tell lockdep we inherited freeze protection from submission thread */
> __sb_writers_acquired(inode->i_sb, SB_FREEZE_WRITE);
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 5ad046145f29..0ed6c199f394 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -2672,7 +2672,7 @@ static void __io_complete_rw(struct io_kiocb *req, long res, long res2,
> __io_req_complete(req, issue_flags, req->result, io_put_rw_kbuf(req));
> }
>
> -static void io_complete_rw(struct kiocb *kiocb, long res)
> +static void io_complete_rw(struct kiocb *kiocb, u64 res)
> {
> struct io_kiocb *req = container_of(kiocb, struct io_kiocb, rw.kiocb);
>
> @@ -2683,7 +2683,7 @@ static void io_complete_rw(struct kiocb *kiocb, long res)
> io_req_task_work_add(req);
> }
>
> -static void io_complete_rw_iopoll(struct kiocb *kiocb, long res)
> +static void io_complete_rw_iopoll(struct kiocb *kiocb, u64 res)
> {
> struct io_kiocb *req = container_of(kiocb, struct io_kiocb, rw.kiocb);
>
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index ac461a499882..ff7db16aea2e 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -272,7 +272,7 @@ static void ovl_aio_cleanup_handler(struct ovl_aio_req *aio_req)
> kmem_cache_free(ovl_aio_request_cachep, aio_req);
> }
>
> -static void ovl_aio_rw_complete(struct kiocb *iocb, long res)
> +static void ovl_aio_rw_complete(struct kiocb *iocb, u64 res)
> {
> struct ovl_aio_req *aio_req = container_of(iocb,
> struct ovl_aio_req, iocb);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 0dcb9020a7b3..3c809ce2518c 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -330,7 +330,7 @@ struct kiocb {
> randomized_struct_fields_start
>
> loff_t ki_pos;
> - void (*ki_complete)(struct kiocb *iocb, long ret);
> + void (*ki_complete)(struct kiocb *iocb, u64 ret);
> void *private;
> int ki_flags;
> u16 ki_hint;
next prev parent reply other threads:[~2021-10-20 19:09 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-20 16:49 [PATCH] fs: kill unused ret2 argument from iocb->ki_complete() Jens Axboe
2021-10-20 17:30 ` Christoph Hellwig
2021-10-20 17:35 ` Jens Axboe
2021-10-20 17:49 ` Greg Kroah-Hartman
2021-10-21 16:40 ` John Keeping
2021-10-21 16:56 ` Jens Axboe
2021-10-22 15:44 ` Jens Axboe
2021-10-23 9:09 ` Greg Kroah-Hartman
2021-10-23 14:01 ` Jens Axboe
2021-10-20 18:16 ` Jeff Moyer
2021-10-20 18:21 ` Jens Axboe
2021-10-20 18:37 ` Jeff Moyer
2021-10-20 18:41 ` Jens Axboe
2021-10-20 18:56 ` Jens Axboe
2021-10-20 19:11 ` Jeff Moyer [this message]
2021-10-20 19:12 ` Jens Axboe
2021-10-20 19:47 ` Jeff Moyer
2021-10-20 19:54 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=x49zgr3mrzs.fsf@segfault.boston.devel.redhat.com \
--to=jmoyer@redhat.com \
--cc=axboe@kernel.dk \
--cc=linux-aio@kvack.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).