Re: [PATCH] scsi: core: Simplify the code for waking up the error handler

[Benjamin Block <bblock@linux.ibm.com>]

On Thu, Mar 09, 2023 at 09:20:45AM -0800, Bart Van Assche wrote:
> On 3/9/23 04:13, Benjamin Block wrote:
> > On Tue, Mar 07, 2023 at 01:51:51PM -0800, Bart Van Assche wrote:
> >> scsi_dec_host_busy() is called from the hot path and hence must not
> >> obtain the host lock if no commands have failed. scsi_dec_host_busy()
> >> tests three different variables of which at least two are set if a
> >> command failed. Commit 3bd6f43f5cb3 ("scsi: core: Ensure that the
> >> SCSI error handler gets woken up") introduced a call_rcu() call to
> >> ensure that all tasks observe the host state change before the
> >> host_failed change. Simplify the approach for guaranteeing that the host
> >> state and host_failed/host_eh_scheduled changes are observed in order by using
> >> smp_store_release() to update host_failed or host_eh_scheduled after
> >> having update the host state and smp_load_acquire() before reading the
> >> host state.
> > 
> > It's probably just me, but "simplify" is a bit of a misnomer when you
> > replace RCU by plain memory barriers. And I'm kind of wondering what we
> > improve here? It seems to me that at least as far as the hot path is
> > concerned, nothing really changes? The situation for
> > `scsi_eh_scmd_add()` seems to improve, but that is already way off the
> > hot path.
> 
> Hi Benjamin,
> 
> The advantages of the approach introduced by this patch are as follows:
> * The size of struct scsi_cmnd is reduced. This may improve performance
>    by reducing the number of cache misses.
> * One call_rcu() call is eliminated. This reduces the error handler
>    wake-up latency.

Is that really a problem today? From personal experience we hardly ever
have any real SCSI timeouts in our environments, and once we are at the
point where we add commands to EH, we are already past the (default) 30s
timeout for disks IIRC, so is the RCU latency significant at that point?

I'm just wondering, because at least IMHO plain memory barrier are more
complex than RCU - at least in understanding them.

-- 
Best Regards, Benjamin Block  / Linux on IBM Z Kernel Development / IBM Systems
IBM Deutschland Research & Development GmbH    /    https://www.ibm.com/privacy
Vorsitz. AufsR.: Gregor Pillen         /         Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294