RE: +AFs-PATCH+AF0- blk-mq: Set request mapping to NULL in blk+AF8-mq+AF8-put+AF8-driver+AF8-tag

From: Kashyap Desai <kashyap.desai@broadcom.com>
To: Jens Axboe <axboe@kernel.dk>, Ming Lei <ming.lei@redhat.com>
Cc: Bart Van Assche <bvanassche@acm.org>,
	linux-block <linux-block@vger.kernel.org>,
	linux-scsi <linux-scsi@vger.kernel.org>,
	Suganath Prabu Subramani  <suganath-prabu.subramani@broadcom.com>,
	Sreekanth Reddy <sreekanth.reddy@broadcom.com>,
	Sathya Prakash Veerichetty <sathya.prakash@broadcom.com>
Subject: RE: +AFs-PATCH+AF0- blk-mq: Set request mapping to NULL in blk+AF8-mq+AF8-put+AF8-driver+AF8-tag
Date: Fri, 7 Dec 2018 12:46:44 +0530	[thread overview]
Message-ID: <cf0b27a5c09afb21aeef0ea8392161dc@mail.gmail.com> (raw)
In-Reply-To: <b7a44ebf-8996-aee3-3aaf-2ca18622d161@kernel.dk>

> On 12/5/18 10:45 PM, Kashyap Desai wrote:
> >>
> >> If the 'tag' passed to scsi_host_find_tag() is valid, I think there
> >> shouldn't have such issue.
> >>
> >> If you want to find outstanding IOs, maybe you can try
> >> blk_mq_queue_tag_busy_iter()
> >> or blk_mq_tagset_busy_iter(), because you may not know if the passed
> > 'tag'
> >> to
> >> scsi_host_find_tag() is valid or not.
> >
> > We tried quick change in mpt3sas driver using blk_mq_tagset_busy_iter
> > and
> > it returns/callback for valid requests (no stale entries are returned).
> > Expected.
> > Above two APIs are only for blk-mq.  What about non-mq case ? Driver
> > should use scsi_host_find_tag for non-mq and blk_mq_tagset_busy_iter for
> > blk-mq case ?
> > I don't see that will be good interface. Also, blk_mq_tagset_busy_iter
> > API
> > does not provide control if driver wants to quit in-between or do some
> > retry logic etc.
> >
> > Why can't we add single API which provides the correct output.
>
> From 4.21 and forward, there will only be blk/scsi-mq. This is exactly
> the problem with having to maintain two stacks, it's a huge pain.

Hi Jens, Fix for this issue also required to be back ported to stable
kernels (which still use non-mq + mq stack).
We have multiple choices to fix this.

1. Use " blk_mq_tagset_busy_iter" in *all* the affected drivers. This API
has certain limitation as explained and also fix only blk-mq part. Using
this API may need more code in low level drivers to handle non-mq and mq
separately.
2. Driver can use internal memory for scsiio_track (driver private) and
track all the outstanding IO within a driver. This is mostly a scsi mid
layer interface. All the affected driver require a changes.
3. Fix blk-mq code around  blk_mq_put_tag and driver can still use "
scsi_host_find_tag". No driver changes are required.
This is smooth to back port stable kernel and Linux Distribution which
normally pick critical fixes from stable can pick the fix. This is better
fix and did not change any design.
In fact, I mimic the same flow as non-mq code is doing.

I don't see any design or functional issue with #3 (PATCH provided in this
thread.) What is your feedback for this patch ?

Kashyap

>
> --
> Jens Axboe