All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <bvanassche@acm.org>
To: Ming Lei <ming.lei@redhat.com>,
	linux-nvme@lists.infradead.org, linux-scsi@vger.kernel.org,
	Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	Christoph Hellwig <hch@lst.de>
Cc: Khazhy Kumykov <khazhy@google.com>,
	Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>,
	Hannes Reinecke <hare@suse.de>,
	John Garry <john.garry@huawei.com>,
	David Jeffery <djeffery@redhat.com>
Subject: Re: [PATCH 6/8] block: drivers: complete request locally from blk_mq_tagset_busy_iter
Date: Sun, 25 Apr 2021 20:02:10 -0700	[thread overview]
Message-ID: <bcdfc3a0-f5fa-dc06-8067-4349a133e531@acm.org> (raw)
In-Reply-To: <20210425085753.2617424-7-ming.lei@redhat.com>

On 4/25/21 1:57 AM, Ming Lei wrote:
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index c289991ffaed..7cbaee282b6d 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1568,7 +1568,11 @@ static void scsi_mq_done(struct scsi_cmnd *cmd)
>  	if (unlikely(test_and_set_bit(SCMD_STATE_COMPLETE, &cmd->state)))
>  		return;
>  	trace_scsi_dispatch_cmd_done(cmd);
> -	blk_mq_complete_request(cmd->request);
> +
> +	if (unlikely(host_byte(cmd->result) != DID_OK))
> +		blk_mq_complete_request_locally(cmd->request);
> +	else
> +		blk_mq_complete_request(cmd->request);
>  }

This change is so tricky that it deserves a comment.

An even better approach would be *not* to export
blk_mq_complete_request_locally() from the block layer to block drivers
and instead modify the block layer such that it completes a request on
the same CPU if request completion happens from inside the context of a
tag iteration function. That would save driver writers the trouble of
learning yet another block layer API.

Bart.

WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <bvanassche@acm.org>
To: Ming Lei <ming.lei@redhat.com>,
	linux-nvme@lists.infradead.org, linux-scsi@vger.kernel.org,
	Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	Christoph Hellwig <hch@lst.de>
Cc: Khazhy Kumykov <khazhy@google.com>,
	Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>,
	Hannes Reinecke <hare@suse.de>,
	John Garry <john.garry@huawei.com>,
	David Jeffery <djeffery@redhat.com>
Subject: Re: [PATCH 6/8] block: drivers: complete request locally from blk_mq_tagset_busy_iter
Date: Sun, 25 Apr 2021 20:02:10 -0700	[thread overview]
Message-ID: <bcdfc3a0-f5fa-dc06-8067-4349a133e531@acm.org> (raw)
In-Reply-To: <20210425085753.2617424-7-ming.lei@redhat.com>

On 4/25/21 1:57 AM, Ming Lei wrote:
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index c289991ffaed..7cbaee282b6d 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1568,7 +1568,11 @@ static void scsi_mq_done(struct scsi_cmnd *cmd)
>  	if (unlikely(test_and_set_bit(SCMD_STATE_COMPLETE, &cmd->state)))
>  		return;
>  	trace_scsi_dispatch_cmd_done(cmd);
> -	blk_mq_complete_request(cmd->request);
> +
> +	if (unlikely(host_byte(cmd->result) != DID_OK))
> +		blk_mq_complete_request_locally(cmd->request);
> +	else
> +		blk_mq_complete_request(cmd->request);
>  }

This change is so tricky that it deserves a comment.

An even better approach would be *not* to export
blk_mq_complete_request_locally() from the block layer to block drivers
and instead modify the block layer such that it completes a request on
the same CPU if request completion happens from inside the context of a
tag iteration function. That would save driver writers the trouble of
learning yet another block layer API.

Bart.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

  reply	other threads:[~2021-04-26  3:02 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-25  8:57 [PATCH 0/8] blk-mq: fix request UAF related with iterating over tagset requests Ming Lei
2021-04-25  8:57 ` Ming Lei
2021-04-25  8:57 ` [PATCH 1/8] Revert "blk-mq: Fix races between blk_mq_update_nr_hw_queues() and iterating over tags" Ming Lei
2021-04-25  8:57   ` Ming Lei
2021-04-25  8:57 ` [PATCH 2/8] Revert "blk-mq: Make it safe to use RCU to iterate over blk_mq_tag_set.tag_list" Ming Lei
2021-04-25  8:57   ` Ming Lei
2021-04-25  8:57 ` [PATCH 3/8] Revert "blk-mq: Fix races between iterating over requests and freeing requests" Ming Lei
2021-04-25  8:57   ` Ming Lei
2021-04-25  8:57 ` [PATCH 4/8] Revert "blk-mq: Introduce atomic variants of blk_mq_(all_tag|tagset_busy)_iter" Ming Lei
2021-04-25  8:57   ` Ming Lei
2021-04-25  8:57 ` [PATCH 5/8] blk-mq: blk_mq_complete_request_locally Ming Lei
2021-04-25  8:57   ` Ming Lei
2021-04-25  8:57 ` [PATCH 6/8] block: drivers: complete request locally from blk_mq_tagset_busy_iter Ming Lei
2021-04-25  8:57   ` Ming Lei
2021-04-26  3:02   ` Bart Van Assche [this message]
2021-04-26  3:02     ` Bart Van Assche
2021-04-26  6:24     ` Ming Lei
2021-04-26  6:24       ` Ming Lei
2021-04-27  8:54       ` Ming Lei
2021-04-27  8:54         ` Ming Lei
2021-04-25  8:57 ` [PATCH 7/8] blk-mq: grab rq->refcount before calling ->fn in blk_mq_tagset_busy_iter Ming Lei
2021-04-25  8:57   ` Ming Lei
2021-04-25 18:55   ` Bart Van Assche
2021-04-25 18:55     ` Bart Van Assche
2021-04-26  0:41     ` Ming Lei
2021-04-26  0:41       ` Ming Lei
2021-04-25  8:57 ` [PATCH 8/8] blk-mq: clear stale request in tags->rq[] before freeing one request pool Ming Lei
2021-04-25  8:57   ` Ming Lei
2021-04-25 20:42   ` Bart Van Assche
2021-04-25 20:42     ` Bart Van Assche
2021-04-26  0:49     ` Ming Lei
2021-04-26  0:49       ` Ming Lei
2021-04-26  1:50       ` Bart Van Assche
2021-04-26  1:50         ` Bart Van Assche
2021-04-26  2:07         ` Ming Lei
2021-04-26  2:07           ` Ming Lei
2021-04-25  9:27 ` [PATCH 0/8] blk-mq: fix request UAF related with iterating over tagset requests Ming Lei
2021-04-25  9:27   ` Ming Lei
2021-04-25 20:53   ` Bart Van Assche
2021-04-25 20:53     ` Bart Van Assche
2021-04-26  1:19     ` Ming Lei
2021-04-26  1:19       ` Ming Lei
2021-04-26  1:57       ` Bart Van Assche
2021-04-26  1:57         ` Bart Van Assche
2021-04-25 16:17 ` Jens Axboe
2021-04-25 16:17   ` Jens Axboe
2021-04-25 18:39   ` Bart Van Assche
2021-04-25 18:39     ` Bart Van Assche
2021-04-25 20:18     ` Jens Axboe
2021-04-25 20:18       ` 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=bcdfc3a0-f5fa-dc06-8067-4349a133e531@acm.org \
    --to=bvanassche@acm.org \
    --cc=axboe@kernel.dk \
    --cc=djeffery@redhat.com \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=john.garry@huawei.com \
    --cc=khazhy@google.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=ming.lei@redhat.com \
    --cc=shinichiro.kawasaki@wdc.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.