Re: REQ_HIPRI and SCSI mid-level

On 2021-05-21 5:56 p.m., Douglas Gilbert wrote:

The REQ_HIPRI flag on requests is associated with blk_poll() (aka iopoll)
and assumes the user space (or some higher level) will be calling
blk_poll() on requests marked with REQ_HIPRI and that will lead to their
completion.

In lk 5.13-rc1 the megaraid and scsi_debug LLDs support blk_poll() [seen
by searching for 'mq_poll'] with more to follow, I assume. I have tested
blk_poll() on the scsi_debug driver using both fio and the new sg driver.
It works well with one caveat: as long as there isn't an error.
After fighting with that error processing from the ULD side (i.e. the
new sg driver) and the LLD side I am concluding that the glue that
holds them together, that is, the mid-level is not as REQ_HIPRI aware
as it should be.

Yes REQ_HIPRI is there in scsi_lib.c but it is missing from scsi_error.c
How can scsi_error.c re-issue requests _without_ taking into account
that the original was issued with REQ_HIPRI ? Well I don't know but I'm
pretty sure that is close to the area that I see causing problems
(mainly lockups).

As an example the scsi_debug driver has an in-use bitmap that when a new
request arrives the code looks for an empty slot. Due to (incorrect)
parameter setup that may fail. If the driver returns:
     device_qfull_result = (DID_OK << 16) | SAM_STAT_TASK_SET_FULL;
then I see lock-ups if the request in question has REQ_HIPRI set.

If that is changed to:
     device_qfull_result = (DID_ABORT << 16) | SAM_STAT_TASK_SET_FULL;
then my user space test program sees that error and aborts showing the
TASK SET FULL SCSI status. That is much better than a lockup ...

Having played around with variants of the above for a few weeks, I'd
like to throw this problem into the open :-)


Suggestion: perhaps the eh could give up immediately on any request
with REQ_HIPRI set (i.e. make it a higher level layer's problem).

Hmmm, no response. Any LLD that decides to support blk_poll() and
relies on the mid-level eh handling will get burnt by this. And those
LLDs that do their own eh handling needs to take care not to replicate
the mid-level's eh handling in this case.

An even simpler approach for the mid-level eh handling to address
(at least partially) this problem, is to do:
    rqq->cmd_flags &= ~REQ_HIPRI;

on retried requests. Clearing the REQ_HIPRI flag, if present, should
make the LLD behave the way the mid-level eh processing expects.

Doug Gilbert