lkml.org 
[lkml]   [2006]   [Apr]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 1/2] raid6_end_write_request() spinlock fix
    On Tue, Apr 25, 2006 at 04:50:10PM +1000, Neil Brown wrote:
    > On Tuesday April 25, qiyong@fc-cn.com wrote:
    > > On Tue, Apr 25, 2006 at 03:13:49PM +1000, Neil Brown wrote:
    > > > On Tuesday April 25, qiyong@fc-cn.com wrote:
    > > > > Hello,
    > > > >
    > > > > Reduce the raid6_end_write_request() spinlock window.
    > > >
    > > > Andrew: please don't include these in -mm. This one and the
    > > > corresponding raid5 are wrong, and I'm not sure yet the unplug_device
    > > > changes.
    > >
    > > I am sure with the unplug_device. Just look follow the path...
    > >
    >
    > What path? There are probably several. If I follow the path, will I
    > see the same things as you see? Who knows, because you haven't
    > bothered to tell us what you see.

    There are only two places where handle_list is possibly re-filled:
    __release_stripe() and raidX_activate_delayed(). So raidXd should only
    wakeup after these two points.

    >
    > >
    > > Yes. Let's fix the error(). In any case, the current code is broken. (see raid5/6_end_read_request)
    >
    > What will I see in raidX_end_read_request. Surely it isn't that hard
    > to write a few more sentences?

    You should see md_error() in raidX_end_read_request isn't in any spinlocks.

    > conf->working_disks isn't atomic_t and so decrementing without a
    > spinlock isn't safe. So lets just leave it all inside a spinlock.

    test_and_set_bit(Faulty, &rdev->flags) protects it as well imho.
    It can be enter only once.

    >
    > Also I have a vague memory that clearing In_sync before Faulty is
    > important, but I'm not certain of that.

    Maybe, but seems not apply here.

    >
    > Remember: the code is there for a reason. It might not be a good
    > reason, and the code could well be wrong. But it would be worth your
    > effort trying to find out what the reason is before blithely changing
    > it (as I discovered recently with a change I suggested to
    > invalidate_mapping_pages).

    Thanks :)
    --
    Coywolf Qi Hunt
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-04-25 10:08    [W:0.022 / U:33.640 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site