Re: [PATCH v2] cxl/mbox: Add background cmd handling machinery

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Mon, 15 May 2023 08:40:30 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> On Mon, 15 May 2023, Jonathan Cameron wrote:
> 
> >On Wed, 3 May 2023 07:57:56 -0700
> >Davidlohr Bueso <dave@stgolabs.net> wrote:
> >  
> >> This adds support for handling background operations, as defined in
> >> the CXL 3.0 spec. Commands that can take too long (over ~2 seconds)
> >> can run in the background asynchronously (to the hardware).
> >>
> >> The driver will deal with such commands synchronously, blocking all
> >> other incoming commands for a specified period of time, allowing
> >> time-slicing the command such that the caller can send incremental
> >> requests to avoid monopolizing the driver/device. This approach
> >> makes the code simpler, where any out of sync (timeout) between the
> >> driver and hardware is just disregarded as an invalid state until
> >> the next successful submission.
> >>
> >> On devices where mbox interrupts are supported, this will still use
> >> a poller that will wakeup in the specified wait intervals. The irq
> >> handler will simply awake the blocked cmd, which is also safe vs a
> >> task that is either waking (timing out) or already awoken. Similarly
> >> any irq setup error during the probing falls back to polling, thus
> >> avoids unnecessarily erroring out.  
> >
> >This raises the question of why we don't support Doorbell Interrupts.
> >2 seconds is rather a long time to poll for.  I can't really remember the
> >reasoning but maybe it's that we don't expect anyone to every produce
> >hardware that takes that long.
> >
> >Ah well, job for another day perhaps.
> >
> >I'm not following why the rcuwait is needed versus other options.
> >Perhaps some minimal text here for those of us not familiar with that
> >particular mechanism vs a completion of similar.  
> 
> As mentioned before, rcuwait gives us the single wait/wake semantics we
> need (we are serialized by the mbox_mutex) - completions use queued wait,
> which is an overkill for this case.
Fair enough I guess. New toy that I'd not come across before.

Jonathan

> 
> >> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>  
> >Otherwise just a comment inline on dev_warn for unexpected interrupt
> >on an interrupt that you've marked as IRQF_SHARED which pretty much
> >guarantees you need to handle unexpected interrupts as 'normal'.  
> 
> Per your feedback in the the previous series, I will remove this dev_warn
> altogether.
> 
> Thanks,
> Davidlohr