On Tue, Apr 08, 2025 at 01:11:52PM -0600, Uday Shankar wrote:
> On Tue, Apr 08, 2025 at 03:24:38PM +0800, Ming Lei wrote:
> > ubq->canceling is set with request queue quiesced when io_uring context is
> > exiting. Recovery & reissue(UBLK_F_USER_RECOVERY_REISSUE) requires
> > request to be re-queued and re-dispatch after device is recovered.
> >
> > However commit d796cea7b9f3 ("ublk: implement ->queue_rqs()") still may
> > fail any request in case of ubq->canceling, this way breaks
> > UBLK_F_USER_RECOVERY_REISSUE.
>
> This change actually affects UBLK_F_USER_RECOVERY as long as FAIL_IO
> isn't set (regardless of RECOVERY_REISSUE). RECOVERY_REISSUE only
> changes behavior for requests that are already in the ublk server when
> the ublk server dies, but the code change only affects requests that
> come in after ublk server death is already detected. At that point, both
> plain USER_RECOVERY and USER_RECOVERY|USER_RECOVERY_REISSUE should see
> requests queue instead of completing with error. See below code
> snippets:
>
> static inline void __ublk_abort_rq(struct ublk_queue *ubq,
> struct request *rq)
> {
> /* We cannot process this rq so just requeue it. */
> if (ublk_nosrv_dev_should_queue_io(ubq->dev))
> blk_mq_requeue_request(rq, false);
> else
> blk_mq_end_request(rq, BLK_STS_IOERR);
> }
>
> /*
> * Should I/O issued while there is no ublk server queue? If not, I/O
> * issued while there is no ublk server will get errors.
> */
> static inline bool ublk_nosrv_dev_should_queue_io(struct ublk_device *ub)
> {
> return (ub->dev_info.flags & UBLK_F_USER_RECOVERY) &&
> !(ub->dev_info.flags & UBLK_F_USER_RECOVERY_FAIL_IO);
> }
Indeed, will update commit log.
>
> >
> > Fix it by calling __ublk_abort_rq() in case of ubq->canceling.
> >
> > Reported-by: Uday Shankar <ushankar@xxxxxxxxxxxxxxx>
> > Closes: https://lore.kernel.org/linux-block/Z%2FQkkTRHfRxtN%2FmB@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
> > Fixes: commit d796cea7b9f3 ("ublk: implement ->queue_rqs()")
>
> Will this upset anything parsing these tags? The syntax I've usually
> seen doesn't have "commit," i.e.
>
> Fixes: d796cea7b9f3 ("ublk: implement ->queue_rqs()")
Will fix it in V2.
Thanks
Ming
