lkml.org 
[lkml]   [2006]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: + delay-accounting-taskstats-interface-send-tgid-once.patch added to -mm tree
Arjan van de Ven wrote:

>also in response to
> Subject:
>Re: 2.6.17-mm3
> Date:
>Tue, 27 Jun 2006 16:12:42 +0200
>with Message-ID:
><6bffcb0e0606270712w166f04a6u237d695e2bfa1913@mail.gmail.com>
>
>On Mon, 2006-06-26 at 12:06 -0700, akpm@osdl.org wrote:
>
>
>
>>+static inline void taskstats_tgid_alloc(struct signal_struct *sig)
>>+{
>>+ struct taskstats *stats;
>>+
>>+ stats = kmem_cache_zalloc(taskstats_cache, SLAB_KERNEL);
>>+ if (!stats)
>>+ return;
>>+
>>+ spin_lock(&sig->stats_lock);
>>+ if (!sig->stats) {
>>
>>
>
>+static inline void taskstats_tgid_free(struct signal_struct *sig)
>+{
>+ struct taskstats *stats = NULL;
>+ spin_lock(&sig->stats_lock);
>+ if (sig->stats) {
>+ stats = sig->stats;
>+ sig->stats = NULL;
>+ }
>+ spin_unlock(&sig->stats_lock);
>+ if (stats)
>+ kmem_cache_free(taskstats_cache, stats);
>+}
>
>this is buggy and deadlock prone!
>(found by lockdep)
>
>stats_lock nests within tasklist_lock, which is taken in irq context.
>(and thus needs irq_save treatment). But because of this nesting, it is
>not allowed to take stats_lock without disabling irqs, or you can have
>the following scenario
>
>CPU 0 CPU 1
>(in irq) (in the code above)
> stats_lock is taken
>tasklist_lock is taken
>stats_lock_is taken <spin>
> <interrupt happens>
> tasklist_lock is taken
>
>which now forms an AB-BA deadlock!
>
>
>this happens at least in copy_process which can call taskstats_tgid_free
>without first disabling interrupts (via cleanup_signal).
>

Arjan,

Didn't get how stats_lock is nesting within tasklist_lock in copy_process ?
The call to cleanup_signal (or any call to taskstats_tgid_alloc/free)
seems to be happening
outside of holding the tasklist lock ? Or maybe I'm missing something.

Changing to an irqsave variant isn't a problem of course...

--Shailabh

>There may be
>many other cases, I've not checked deeper yet.
>
>Solution should be to make these functions use irqsave variant... any
>comments from the authors of this patch ?
>
>
>
>

-
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-06-27 17:56    [W:0.057 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site