On Thu, Apr 19, 2012 at 07:16:21PM +0200, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@xxxxxxxxxx> > > It seems there is a logic error in trace_drop_common(), since we store > only 64 drops, even if they are from same location. > > This fix is a one liner, but we probably need more work to avoid useless > atomic dec/inc > > Now I can watch 1 Mpps drops through dropwatch... > > Signed-off-by: Eric Dumazet <edumazet@xxxxxxxxxx> > Cc: Neil Horman <nhorman@xxxxxxxxxxxxx> > --- > Neil, it seems this code is not SMP/preempt safe. Worker can free our > data under us, and genlmsg_new() can return NULL under stress. > > net/core/drop_monitor.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c > index 7f36b38..5c3c81a 100644 > --- a/net/core/drop_monitor.c > +++ b/net/core/drop_monitor.c > @@ -150,6 +150,7 @@ static void trace_drop_common(struct sk_buff *skb, void *location) > for (i = 0; i < msg->entries; i++) { > if (!memcmp(&location, msg->points[i].pc, sizeof(void *))) { > msg->points[i].count++; > + atomic_inc(&data->dm_hit_count); > goto out; > } > } > > > I spent a good deal of time going through it to make sure it was preempt safe, but its certainly possible that I missed something. I'll look at it shortly. Thanks for this update though Acked-by: Neil Horman <nhorman@xxxxxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html