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.
--Shailabh
> Cheers,
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
