Re: [seqcount] INFO: trying to register non-static key.

[John Stultz <john.stultz@linaro.org>]

On 11/12/2013 07:29 AM, Peter Zijlstra wrote:
> On Tue, Nov 12, 2013 at 10:15:41AM -0500, Vivek Goyal wrote:
>> I see that we allocate per cpu stats but don't do any initializations.
>>
>> static void tg_stats_alloc_fn(struct work_struct *work)
>> {
>>         static struct tg_stats_cpu *stats_cpu;  /* this fn is non-reentrant */
>>         struct delayed_work *dwork = to_delayed_work(work);
>>         bool empty = false;
>>
>> alloc_stats:
>>         if (!stats_cpu) {
>>                 stats_cpu = alloc_percpu(struct tg_stats_cpu);
>>                 if (!stats_cpu) {
>>                         /* allocation failed, try again after some time */
>>                         schedule_delayed_work(dwork, msecs_to_jiffies(10));
>>                         return;
>>                 }
>>         }
>>
>>         spin_lock_irq(&tg_stats_alloc_lock);
> Absolutely!
>
> Something like this perhaps? Did I miss more blkg_[rw]stats? If I read
> the git grep output right, this was the last one.

Hey Peter,
    Thanks for chasing these issues down so fast! And sorry for my slow
response here (power outage for a chunk of the day :P) I finally got the
issue reproduced and gave your two patches a whirl, but unfortunately I
get the following bug:

[    0.728658] BUG: unable to handle kernel NULL pointer dereference at
00000008
[    0.729351] IP: [<78287084>] lockdep_init_map+0xd/0x544
[    0.729351] *pde = 00000000
[    0.729351] Oops: 0002 [#1] SMP
[    0.729351] CPU: 0 PID: 18 Comm: kworker/0:1 Not tainted
3.12.0-00185-g838cc7b-dirty #13
[    0.729351] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[    0.729351] Workqueue: events tg_stats_alloc_fn
[    0.729351] task: 7e9e0230 ti: 7e9e6000 task.ti: 7e9e6000
[    0.729351] EIP: 0060:[<78287084>] EFLAGS: 00010216 CPU: 0
[    0.729351] EIP is at lockdep_init_map+0xd/0x544
[    0.729351] EAX: 00000004 EBX: 79d554b0 ECX: 79d554b0 EDX: 793f682a
[    0.729351] ESI: 00000004 EDI: 79d554b8 EBP: 7e9e7e90 ESP: 7e9e7e78
[    0.729351]  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
[    0.729351] CR0: 8005003b CR2: 00000008 CR3: 01898000 CR4: 000006d0
[    0.729351] Stack:
[    0.729351]  0000014a 0000014b 7e9b7340 00000000 00000000 79d554b8
7e9e7eac 785fae1d
[    0.729351]  00000000 795a2a60 7e9b7340 00000000 00000000 7e9e7ef8
7826bae8 00000000
[    0.729351]  00000001 00000000 7826ba8f 7e806c00 7f08d700 795a2a60
7f08c380 795a2a60
[    0.729351] Call Trace:
[    0.729351]  [<785fae1d>] tg_stats_alloc_fn+0xe3/0x155
...

>
> ---
>  block/blk-throttle.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
> index 8331aba9426f..fd743d98c41d 100644
> --- a/block/blk-throttle.c
> +++ b/block/blk-throttle.c
> @@ -256,6 +256,12 @@ static struct throtl_data *sq_to_td(struct throtl_service_queue *sq)
>  	}								\
>  } while (0)
>  
> +static void tg_stats_init(struct tg_stats_cpu *tg_stats)
> +{
> +	blkg_rwstat_init(&tg_stats->service_bytes);
> +	blkg_rwstat_init(&tg_stats->serviced);
> +}
> +
>  /*
>   * Worker for allocating per cpu stat for tgs. This is scheduled on the
>   * system_wq once there are some groups on the alloc_list waiting for
> @@ -269,12 +275,16 @@ static void tg_stats_alloc_fn(struct work_struct *work)
>  
>  alloc_stats:
>  	if (!stats_cpu) {
> +		int cpu;
> +
>  		stats_cpu = alloc_percpu(struct tg_stats_cpu);
>  		if (!stats_cpu) {
>  			/* allocation failed, try again after some time */
>  			schedule_delayed_work(dwork, msecs_to_jiffies(10));
>  			return;
>  		}
> +		for_each_possible_cpu(cpu)
> +			tg_stats_init(per_cpu(stats_cpu, cpu));
It looks like this line should be per_cpu_*ptr*

With that line changed I don't trigger either the BUG or the warning
Fengguang found (btw, great work again Fengguang!).

I'll send out a net patch here that includes all the fixes in a second.

thanks
-john