[lkml]   [2006]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [Patch 6/6] per task delay accounting taskstats interface: fix clone skbs for each listener
Herbert Xu wrote:
> On Tue, Jul 11, 2006 at 03:57:31AM -0700, Andrew Morton wrote:
>>>>> down_write(&listeners->sem);
>>>>> list_for_each_entry_safe(s, tmp, &listeners->list, list) {
>>>>>- ret = genlmsg_unicast(skb, s->pid);
>>>>>+ skb_next = NULL;
>>>>>+ if (!list_islast(&s->list, &listeners->list)) {
>>>>>+ skb_next = skb_clone(skb_cur, GFP_KERNEL);
>>>>If we do a GFP_KERNEL allocation with this semaphore held, and the
>>>>oom-killer tries to kill something to satisfy the allocation, and the
>>>>killed task gets stuck on that semaphore, I wonder of the box locks up.

Hmm...doesn't look very safe does it.
There's no real need for us to skb_clone within the sem. Keeping a count of
listeners and doing the clone outside should let us avoid this problem.

I was trying to avoid doing the above because of the potential for
listeners getting added continuously to the list
(and having to repeat the allocation loop outside the down_write).

But on second thoughts, we're under no obligation to send the data to all the
listeners who add themselves in the short time between our taking a snapshot
of the listener count and when the send is done (within the down_write). So
it should be ok.

>>>We do GFP_KERNEL inside semaphores/mutexes in lots of places. So if this
>>>can deadlock with the oom-killer we probably should fix that, preferably
>>>by having GFP_KERNEL fail in that case.
>>This lock is special, in that it's taken on the exit() path (I think). So
>>it can block tasks which are trying to exit.
> Sorry, missed the context.
> If there is a deadlock then it's not just this allocation that you
> need worry about. There is also an allocation within genlmsg_uniast
> that would be GFP_KERNEL.

Thats true. The GFP_KERNEL allocation potentially called in netlink_trim() as part
of the genlmsg/netlink_unicast() is again a problem.

So perhaps we should switch to using RCU for protecting the listener list.


> Cheers,

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

 \ /
  Last update: 2006-07-11 19:47    [from the cache]
©2003-2011 Jasper Spaans