Re: Locking in disk(9) api

[David Laight <david%l8s.co.uk@localhost>]

On Wed, Dec 30, 2009 at 03:07:38PM +1100, matthew green wrote:
>    
>    If the system can panic because a "statistic" goes negative, that's
>    a plenty serious bug all its own.
> 
>    If we can't figure out how to choose appropriate size and signedness
>    for statistics so the kernel can update them unlocked with, at worst,
>    a risk that readers will see a slightly outdated value, we're a lot
>    dumber than I think we are.
> 
> the size and sign are irrelevant for this check (it actually checks
> against zero, not negative.)
> 
> since i don't think that completely corrupting the "total bytes
> transferred" of iostats is an OK thing to do, i don't agree here that
> we have a normal case to play fast&loose with stats.  our drivers
> should ensure they call disk_*busy() with appropriate locking.  ie,
> this isn't some transient failure, it's going to potentially corrupt
> "since boot" data every time the subsystem is pushed hard.

We seem to be talking about 2 different things here:
1) the 'disk busy' count, this isn't a statistic so needs locking.
2) cumulative counts of transfers and bytes, these are just stats so
   don't necessarily need locking. The most likely 'error' is that one
   of the addittions is lost, this is really below the noise floor.
   A slight worry might be an error when a multi-word value wraps (eg
   a 64bit counter on a 32bit machine that has no double-word memory
   accesses). This is probably statistically unlikely enough to be
   ignored as well.

In some drivers, counting the stats is already a significant part of the
workload - adding an extra mutex will be a significant cost!
Possibly best is to count the stats in per-cpu data areas and sum on demand
- unless they fit in some other area which is dirtied at the same time.

        David

-- 
David Laight: david%l8s.co.uk@localhost