lkml.org 
[lkml]   [2012]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 2/2] block: Convert BDI proportion calculations to flexible proportions
    On Tue 29-05-12 14:54:52, Jan Kara wrote:
    > On Tue 29-05-12 14:38:31, Peter Zijlstra wrote:
    > > On Tue, 2012-05-29 at 14:34 +0200, Jan Kara wrote:
    > >
    > > > The only safe solution seems to be to create a variant of percpu counters
    > > > that can be used from an interrupt. Or do you have other idea Peter?
    > >
    > > > > [ 20.680186] [<ffffffff8325ac9b>] _raw_spin_lock+0x3b/0x70
    > > > > [ 20.680186] [<ffffffff81993527>] ? __percpu_counter_sum+0x17/0xc0
    > > > > [ 20.680186] [<ffffffff81993527>] __percpu_counter_sum+0x17/0xc0
    > > > > [ 20.680186] [<ffffffff810ebf90>] ? init_timer_deferrable_key+0x20/0x20
    > > > > [ 20.680186] [<ffffffff8195b5c2>] fprop_new_period+0x12/0x60
    > > > > [ 20.680186] [<ffffffff811d929d>] writeout_period+0x3d/0xa0
    > > > > [ 20.680186] [<ffffffff810ec0bf>] call_timer_fn+0x12f/0x260
    > > > > [ 20.680186] [<ffffffff810ebf90>] ? init_timer_deferrable_key+0x20/0x20
    > >
    > > Yeah, just make sure IRQs are disabled around doing that ;-)
    > Evil ;) But we'd need to have IRQs disabled also in each
    > fprop_fraction_percpu() call, and generally, if we want things clean, we'd
    > need to disable them in all entry points to proportion code (or at least
    > around all percpu calls)...
    OK, after some thought I was wrong and fixing fprop_new_period() is
    enough. Attached patch should fix the warning (and possible deadlock).
    Fengguang should I resend you fixed patch implementing flexible proportions
    or do you prefer incremental patch against your tree?

    Honza
    --
    Jan Kara <jack@suse.cz>
    SUSE Labs, CR
    From: Jan Kara <jack@suse.cz>
    Subject: lib: Fix possible deadlock in flexible proportion code

    When percpu counter function in fprop_new_period() is interrupted by an
    interrupt while holding counter lock, it can cause deadlock when the
    interrupt wants to take the lock as well. Fix the problem by disabling
    interrupts when calling percpu counter functions.

    Signed-off-by: Jan Kara <jack@suse.cz>

    diff --git a/lib/flex_proportions.c b/lib/flex_proportions.c
    index 530dbc2..fbf6b11 100644
    --- a/lib/flex_proportions.c
    +++ b/lib/flex_proportions.c
    @@ -62,8 +62,12 @@ void fprop_global_destroy(struct fprop_global *p)
    */
    bool fprop_new_period(struct fprop_global *p, int periods)
    {
    - u64 events = percpu_counter_sum(&p->events);
    + u64 events;
    + unsigned long flags;

    + local_irq_save(flags);
    + events = percpu_counter_sum(&p->events);
    + local_irq_restore(flags);
    /*
    * Don't do anything if there are no events.
    */
    @@ -73,7 +77,9 @@ bool fprop_new_period(struct fprop_global *p, int periods)
    if (periods < 64)
    events -= events >> periods;
    /* Use addition to avoid losing events happening between sum and set */
    + local_irq_save(flags);
    percpu_counter_add(&p->events, -events);
    + local_irq_restore(flags);
    p->period += periods;
    write_seqcount_end(&p->sequence);
    \
     
     \ /
      Last update: 2012-06-01 00:21    [W:4.256 / U:1.220 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site