Re: [PATCH 2/2] block: Convert BDI proportion calculations to flexible proportions

[Jan Kara <jack@suse.cz>]

[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]

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

[-- Attachment #2: flex-proportion-irq-save.diff --]
[-- Type: text/x-patch, Size: 1285 bytes --]

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);