lkml.org 
[lkml]   [2009]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH] [RFC] make hd_struct->in_flight atomic to avoid diskstat corruption
    Date
    On Thursday 16 April 2009 13:05:57 Jens Axboe wrote:
    > On Thu, Apr 16 2009, Nikanth Karthikesan wrote:
    > > The disk statistics exported to userspace through proc and sysfs are
    > > not protected by locks to avoid performance overhead. Since most of
    > > the statistics are maintained in the per_cpu struct disk_stats, the
    > > chances of them getting corrupted is negligible. But the in_flight
    > > counter, that records the no of requests currently in progress is not
    > > per-cpu. This increases the chance of it getting corrupted. And
    > > corruption of this value would result in visibly distorted statistics
    > > such as negative in_flight. This can be avoided by making this field
    > > atomic.
    >
    > Hmm. Did you observe this behaviour?

    Sorry, not on current kernels. But on a very old 2.6.5 kernel.

    Reading Documentation/iostats.txt and the changelog of commit
    e71bf0d0ee89e51b92776391c5634938236977d5 made me assume that this could be a
    problem even today.

    > A quick glance at the code reveals
    > that the callers of part_inc_in_flight() and part_dec_in_flight() in the
    > block layer are always done under the queue lock. Ditto
    > part_round_stats(), which calls part_round_stats_single() and also needs
    > protection for in_flight.
    >
    > That basically just leaves the code reading this out and reporting, and
    > driver calls to part_round_stats(). I'd suggest looking there instead,
    > we're not going to make ->in_flight an atomic just because of some
    > silliness there that could be fixed.

    Isn't this also true for the stats protected by the part_stat_lock()? Only
    places where we are only reading seems to be called without the queue lock.

    Thanks
    Nikanth


    \
     
     \ /
      Last update: 2009-04-16 11:19    [W:5.045 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site