On 2020-11-02 18:12:38 [+0000], Christoph Hellwig wrote:
> > to not break that assumption you just mentioned and provide
> > |static inline void blk_mq_complete_request_local(struct request *rq)
> > |{
> > | rq->q->mq_ops->complete(rq);
> > |}
> >
> > so that completion issued from from process context (like those from
> > usb-storage) don't end up waking `ksoftird' (running at SCHED_OTHER)
> > completing the requests but rather performing it right away. The softirq
> > dance makes no sense here.
>
> Agreed. But I don't think your above blk_mq_complete_request_local
> is all that useful either as ->complete is defined by the caller,
> so we could just do a direct call.
In usb-storage case it is hidden somewhere in the SCSI stack but this
can probably be changed later on.
> Basically we should just
> return false from blk_mq_complete_request_remote after updating
> the state when called from process context. But given that IIRC
> we are not supposed to check what state we are called from
> we'll need a helper just for updating the state instead and
> ensure the driver uses the right helper. Now of course we might
> have process context callers that still want to bounce to the
> submitting CPU, but in that case we should go directly to a
> workqueue or similar.
So instead blk_mq_complete_request_local() you want a helper to set the
state in which the completion function is invoked. Sounds more like an
argument :)
> Either way doing this properly will probabl involve an audit of all
> drivers, but I think that is worth it.
I'm lost. Should I repost the three patches with a preempt_disable()
section (as suggested) to not break preemptible callers? And then move
from there to provide callers from preemtible context an alternative?
Sebastian
