Re: [PATCH 8/8] blk-mq: clear stale request in tags->rq[] before freeing one request pool

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 8/8] blk-mq: clear stale request in tags->rq[] before freeing one request pool
Date: Sun, 25 Apr 2021 13:42:59 -0700	[thread overview]
Message-ID: <d3493eef-ff45-23a8-f12a-b7246ba9f3a2@acm.org> (raw)
In-Reply-To: <20210425085753.2617424-9-ming.lei@redhat.com>

On 4/25/21 1:57 AM, Ming Lei wrote:
> +	spin_lock_irqsave(&drv_tags->lock, flags);
> +	list_for_each_entry(page, &tags->page_list, lru) {
> +		unsigned long start = (unsigned long)page_address(page);
> +		unsigned long end = start + order_to_size(page->private);
> +		int i;
> +
> +		for (i = 0; i < set->queue_depth; i++) {
> +			struct request *rq = drv_tags->rqs[i];
> +			unsigned long rq_addr = (unsigned long)rq;
> +
> +			if (rq_addr >= start && rq_addr < end) {
> +				WARN_ON_ONCE(refcount_read(&rq->ref) != 0);
> +				cmpxchg(&drv_tags->rqs[i], rq, NULL);
> +			}
> +		}
> +	}
> +	spin_unlock_irqrestore(&drv_tags->lock, flags);

Using cmpxchg() on set->tags[] is only safe if all other set->tags[]
accesses are changed into WRITE_ONCE() or READ_ONCE().

Bart.