From cde2e9a651b76d8db36ae94cd0febc82b637e5dd Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Fri, 27 Apr 2012 10:11:48 +0000 Subject: drop_monitor: fix sleeping in invalid context warning Eric Dumazet pointed out this warning in the drop_monitor protocol to me: [ 38.352571] BUG: sleeping function called from invalid context at kernel/mutex.c:85 [ 38.352576] in_atomic(): 1, irqs_disabled(): 0, pid: 4415, name: dropwatch [ 38.352580] Pid: 4415, comm: dropwatch Not tainted 3.4.0-rc2+ #71 [ 38.352582] Call Trace: [ 38.352592] [] ? trace_napi_poll_hit+0xd0/0xd0 [ 38.352599] [] __might_sleep+0xca/0xf0 [ 38.352606] [] mutex_lock+0x26/0x50 [ 38.352610] [] ? trace_napi_poll_hit+0xd0/0xd0 [ 38.352616] [] tracepoint_probe_register+0x29/0x90 [ 38.352621] [] set_all_monitor_traces+0x105/0x170 [ 38.352625] [] net_dm_cmd_trace+0x2a/0x40 [ 38.352630] [] genl_rcv_msg+0x21a/0x2b0 [ 38.352636] [] ? zone_statistics+0x99/0xc0 [ 38.352640] [] ? genl_rcv+0x30/0x30 [ 38.352645] [] netlink_rcv_skb+0xa9/0xd0 [ 38.352649] [] genl_rcv+0x20/0x30 [ 38.352653] [] netlink_unicast+0x1ae/0x1f0 [ 38.352658] [] netlink_sendmsg+0x2b6/0x310 [ 38.352663] [] sock_sendmsg+0x10f/0x130 [ 38.352668] [] ? move_addr_to_kernel+0x60/0xb0 [ 38.352673] [] ? verify_iovec+0x64/0xe0 [ 38.352677] [] __sys_sendmsg+0x386/0x390 [ 38.352682] [] ? handle_mm_fault+0x139/0x210 [ 38.352687] [] ? do_page_fault+0x1ec/0x4f0 [ 38.352693] [] ? set_next_entity+0x9d/0xb0 [ 38.352699] [] ? tty_ldisc_deref+0x9/0x10 [ 38.352703] [] ? pick_next_task_fair+0x63/0x140 [ 38.352708] [] sys_sendmsg+0x44/0x80 [ 38.352713] [] system_call_fastpath+0x16/0x1b It stems from holding a spinlock (trace_state_lock) while attempting to register or unregister tracepoint hooks, making in_atomic() true in this context, leading to the warning when the tracepoint calls might_sleep() while its taking a mutex. Since we only use the trace_state_lock to prevent trace protocol state races, as well as hardware stat list updates on an rcu write side, we can just convert the spinlock to a mutex to avoid this problem. Signed-off-by: Neil Horman Reported-by: Eric Dumazet CC: David Miller Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- net/core/drop_monitor.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'net/core/drop_monitor.c') diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c index 5c3c81a609e5..a221a5bbecf7 100644 --- a/net/core/drop_monitor.c +++ b/net/core/drop_monitor.c @@ -42,7 +42,7 @@ static void send_dm_alert(struct work_struct *unused); * netlink alerts */ static int trace_state = TRACE_OFF; -static DEFINE_SPINLOCK(trace_state_lock); +static DEFINE_MUTEX(trace_state_mutex); struct per_cpu_dm_data { struct work_struct dm_alert_work; @@ -214,7 +214,7 @@ static int set_all_monitor_traces(int state) struct dm_hw_stat_delta *new_stat = NULL; struct dm_hw_stat_delta *temp; - spin_lock(&trace_state_lock); + mutex_lock(&trace_state_mutex); if (state == trace_state) { rc = -EAGAIN; @@ -253,7 +253,7 @@ static int set_all_monitor_traces(int state) rc = -EINPROGRESS; out_unlock: - spin_unlock(&trace_state_lock); + mutex_unlock(&trace_state_mutex); return rc; } @@ -296,12 +296,12 @@ static int dropmon_net_event(struct notifier_block *ev_block, new_stat->dev = dev; new_stat->last_rx = jiffies; - spin_lock(&trace_state_lock); + mutex_lock(&trace_state_mutex); list_add_rcu(&new_stat->list, &hw_stats_list); - spin_unlock(&trace_state_lock); + mutex_unlock(&trace_state_mutex); break; case NETDEV_UNREGISTER: - spin_lock(&trace_state_lock); + mutex_lock(&trace_state_mutex); list_for_each_entry_safe(new_stat, tmp, &hw_stats_list, list) { if (new_stat->dev == dev) { new_stat->dev = NULL; @@ -312,7 +312,7 @@ static int dropmon_net_event(struct notifier_block *ev_block, } } } - spin_unlock(&trace_state_lock); + mutex_unlock(&trace_state_mutex); break; } out: -- cgit v1.2.3 From 3885ca785a3618593226687ced84f3f336dc3860 Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Fri, 27 Apr 2012 10:11:49 +0000 Subject: drop_monitor: Make updating data->skb smp safe Eric Dumazet pointed out to me that the drop_monitor protocol has some holes in its smp protections. Specifically, its possible to replace data->skb while its being written. This patch corrects that by making data->skb an rcu protected variable. That will prevent it from being overwritten while a tracepoint is modifying it. Signed-off-by: Neil Horman Reported-by: Eric Dumazet CC: David Miller Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- net/core/drop_monitor.c | 70 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 54 insertions(+), 16 deletions(-) (limited to 'net/core/drop_monitor.c') diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c index a221a5bbecf7..7592943513e3 100644 --- a/net/core/drop_monitor.c +++ b/net/core/drop_monitor.c @@ -46,7 +46,7 @@ static DEFINE_MUTEX(trace_state_mutex); struct per_cpu_dm_data { struct work_struct dm_alert_work; - struct sk_buff *skb; + struct sk_buff __rcu *skb; atomic_t dm_hit_count; struct timer_list send_timer; }; @@ -73,35 +73,58 @@ static int dm_hit_limit = 64; static int dm_delay = 1; static unsigned long dm_hw_check_delta = 2*HZ; static LIST_HEAD(hw_stats_list); +static int initialized = 0; static void reset_per_cpu_data(struct per_cpu_dm_data *data) { size_t al; struct net_dm_alert_msg *msg; struct nlattr *nla; + struct sk_buff *skb; + struct sk_buff *oskb = rcu_dereference_protected(data->skb, 1); al = sizeof(struct net_dm_alert_msg); al += dm_hit_limit * sizeof(struct net_dm_drop_point); al += sizeof(struct nlattr); - data->skb = genlmsg_new(al, GFP_KERNEL); - genlmsg_put(data->skb, 0, 0, &net_drop_monitor_family, - 0, NET_DM_CMD_ALERT); - nla = nla_reserve(data->skb, NLA_UNSPEC, sizeof(struct net_dm_alert_msg)); - msg = nla_data(nla); - memset(msg, 0, al); - atomic_set(&data->dm_hit_count, dm_hit_limit); + skb = genlmsg_new(al, GFP_KERNEL); + + if (skb) { + genlmsg_put(skb, 0, 0, &net_drop_monitor_family, + 0, NET_DM_CMD_ALERT); + nla = nla_reserve(skb, NLA_UNSPEC, + sizeof(struct net_dm_alert_msg)); + msg = nla_data(nla); + memset(msg, 0, al); + } else if (initialized) + schedule_work_on(smp_processor_id(), &data->dm_alert_work); + + /* + * Don't need to lock this, since we are guaranteed to only + * run this on a single cpu at a time. + * Note also that we only update data->skb if the old and new skb + * pointers don't match. This ensures that we don't continually call + * synchornize_rcu if we repeatedly fail to alloc a new netlink message. + */ + if (skb != oskb) { + rcu_assign_pointer(data->skb, skb); + + synchronize_rcu(); + + atomic_set(&data->dm_hit_count, dm_hit_limit); + } + } static void send_dm_alert(struct work_struct *unused) { struct sk_buff *skb; - struct per_cpu_dm_data *data = &__get_cpu_var(dm_cpu_data); + struct per_cpu_dm_data *data = &get_cpu_var(dm_cpu_data); /* * Grab the skb we're about to send */ - skb = data->skb; + skb = rcu_dereference_protected(data->skb, 1); /* * Replace it with a new one @@ -111,8 +134,10 @@ static void send_dm_alert(struct work_struct *unused) /* * Ship it! */ - genlmsg_multicast(skb, 0, NET_DM_GRP_ALERT, GFP_KERNEL); + if (skb) + genlmsg_multicast(skb, 0, NET_DM_GRP_ALERT, GFP_KERNEL); + put_cpu_var(dm_cpu_data); } /* @@ -123,9 +148,11 @@ static void send_dm_alert(struct work_struct *unused) */ static void sched_send_work(unsigned long unused) { - struct per_cpu_dm_data *data = &__get_cpu_var(dm_cpu_data); + struct per_cpu_dm_data *data = &get_cpu_var(dm_cpu_data); + + schedule_work_on(smp_processor_id(), &data->dm_alert_work); - schedule_work(&data->dm_alert_work); + put_cpu_var(dm_cpu_data); } static void trace_drop_common(struct sk_buff *skb, void *location) @@ -134,9 +161,16 @@ static void trace_drop_common(struct sk_buff *skb, void *location) struct nlmsghdr *nlh; struct nlattr *nla; int i; - struct per_cpu_dm_data *data = &__get_cpu_var(dm_cpu_data); + struct sk_buff *dskb; + struct per_cpu_dm_data *data = &get_cpu_var(dm_cpu_data); + rcu_read_lock(); + dskb = rcu_dereference(data->skb); + + if (!dskb) + goto out; + if (!atomic_add_unless(&data->dm_hit_count, -1, 0)) { /* * we're already at zero, discard this hit @@ -144,7 +178,7 @@ static void trace_drop_common(struct sk_buff *skb, void *location) goto out; } - nlh = (struct nlmsghdr *)data->skb->data; + nlh = (struct nlmsghdr *)dskb->data; nla = genlmsg_data(nlmsg_data(nlh)); msg = nla_data(nla); for (i = 0; i < msg->entries; i++) { @@ -158,7 +192,7 @@ static void trace_drop_common(struct sk_buff *skb, void *location) /* * We need to create a new entry */ - __nla_reserve_nohdr(data->skb, sizeof(struct net_dm_drop_point)); + __nla_reserve_nohdr(dskb, sizeof(struct net_dm_drop_point)); nla->nla_len += NLA_ALIGN(sizeof(struct net_dm_drop_point)); memcpy(msg->points[msg->entries].pc, &location, sizeof(void *)); msg->points[msg->entries].count = 1; @@ -170,6 +204,8 @@ static void trace_drop_common(struct sk_buff *skb, void *location) } out: + rcu_read_unlock(); + put_cpu_var(dm_cpu_data); return; } @@ -375,6 +411,8 @@ static int __init init_net_drop_monitor(void) data->send_timer.function = sched_send_work; } + initialized = 1; + goto out; out_unreg: -- cgit v1.2.3 From 4fdcfa12843bca38d0c9deff70c8720e4e8f515f Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Tue, 1 May 2012 08:18:02 +0000 Subject: drop_monitor: prevent init path from scheduling on the wrong cpu I just noticed after some recent updates, that the init path for the drop monitor protocol has a minor error. drop monitor maintains a per cpu structure, that gets initalized from a single cpu. Normally this is fine, as the protocol isn't in use yet, but I recently made a change that causes a failed skb allocation to reschedule itself . Given the current code, the implication is that this workqueue reschedule will take place on the wrong cpu. If drop monitor is used early during the boot process, its possible that two cpus will access a single per-cpu structure in parallel, possibly leading to data corruption. This patch fixes the situation, by storing the cpu number that a given instance of this per-cpu data should be accessed from. In the case of a need for a reschedule, the cpu stored in the struct is assigned the rescheule, rather than the currently executing cpu Tested successfully by myself. Signed-off-by: Neil Horman CC: David Miller Signed-off-by: David S. Miller --- net/core/drop_monitor.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'net/core/drop_monitor.c') diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c index 7592943513e3..a7cad741df01 100644 --- a/net/core/drop_monitor.c +++ b/net/core/drop_monitor.c @@ -49,6 +49,7 @@ struct per_cpu_dm_data { struct sk_buff __rcu *skb; atomic_t dm_hit_count; struct timer_list send_timer; + int cpu; }; struct dm_hw_stat_delta { @@ -73,7 +74,6 @@ static int dm_hit_limit = 64; static int dm_delay = 1; static unsigned long dm_hw_check_delta = 2*HZ; static LIST_HEAD(hw_stats_list); -static int initialized = 0; static void reset_per_cpu_data(struct per_cpu_dm_data *data) { @@ -96,8 +96,8 @@ static void reset_per_cpu_data(struct per_cpu_dm_data *data) sizeof(struct net_dm_alert_msg)); msg = nla_data(nla); memset(msg, 0, al); - } else if (initialized) - schedule_work_on(smp_processor_id(), &data->dm_alert_work); + } else + schedule_work_on(data->cpu, &data->dm_alert_work); /* * Don't need to lock this, since we are guaranteed to only @@ -121,6 +121,8 @@ static void send_dm_alert(struct work_struct *unused) struct sk_buff *skb; struct per_cpu_dm_data *data = &get_cpu_var(dm_cpu_data); + WARN_ON_ONCE(data->cpu != smp_processor_id()); + /* * Grab the skb we're about to send */ @@ -404,14 +406,14 @@ static int __init init_net_drop_monitor(void) for_each_present_cpu(cpu) { data = &per_cpu(dm_cpu_data, cpu); - reset_per_cpu_data(data); + data->cpu = cpu; INIT_WORK(&data->dm_alert_work, send_dm_alert); init_timer(&data->send_timer); data->send_timer.data = cpu; data->send_timer.function = sched_send_work; + reset_per_cpu_data(data); } - initialized = 1; goto out; -- cgit v1.2.3 From e005d193d55ee5f757b13306112d8c23aac27a88 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 16 May 2012 19:58:40 +0000 Subject: net: core: Use pr_ Use the current logging style. This enables use of dynamic debugging as well. Convert printk(KERN_ to pr_. Add pr_fmt. Remove embedded prefixes, use %s, __func__ instead. Signed-off-by: Joe Perches Signed-off-by: David S. Miller --- net/core/drop_monitor.c | 10 ++++++---- net/core/neighbour.c | 13 +++++++------ net/core/net_namespace.c | 6 ++++-- net/core/netprio_cgroup.c | 6 ++++-- net/core/skbuff.c | 20 ++++++++++---------- net/core/sock.c | 25 +++++++++++++------------ 6 files changed, 44 insertions(+), 36 deletions(-) (limited to 'net/core/drop_monitor.c') diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c index a7cad741df01..eca00a96bcf3 100644 --- a/net/core/drop_monitor.c +++ b/net/core/drop_monitor.c @@ -4,6 +4,8 @@ * Copyright (C) 2009 Neil Horman */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -381,10 +383,10 @@ static int __init init_net_drop_monitor(void) struct per_cpu_dm_data *data; int cpu, rc; - printk(KERN_INFO "Initializing network drop monitor service\n"); + pr_info("Initializing network drop monitor service\n"); if (sizeof(void *) > 8) { - printk(KERN_ERR "Unable to store program counters on this arch, Drop monitor failed\n"); + pr_err("Unable to store program counters on this arch, Drop monitor failed\n"); return -ENOSPC; } @@ -392,13 +394,13 @@ static int __init init_net_drop_monitor(void) dropmon_ops, ARRAY_SIZE(dropmon_ops)); if (rc) { - printk(KERN_ERR "Could not create drop monitor netlink family\n"); + pr_err("Could not create drop monitor netlink family\n"); return rc; } rc = register_netdevice_notifier(&dropmon_net_notifier); if (rc < 0) { - printk(KERN_CRIT "Failed to register netdevice notifier\n"); + pr_crit("Failed to register netdevice notifier\n"); goto out_unreg; } diff --git a/net/core/neighbour.c b/net/core/neighbour.c index fadaa819b854..eb09f8bbbf07 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -15,6 +15,8 @@ * Harald Welte Add neighbour cache statistics like rtstat */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -712,14 +714,13 @@ void neigh_destroy(struct neighbour *neigh) NEIGH_CACHE_STAT_INC(neigh->tbl, destroys); if (!neigh->dead) { - printk(KERN_WARNING - "Destroying alive neighbour %p\n", neigh); + pr_warn("Destroying alive neighbour %p\n", neigh); dump_stack(); return; } if (neigh_del_timer(neigh)) - printk(KERN_WARNING "Impossible event.\n"); + pr_warn("Impossible event\n"); skb_queue_purge(&neigh->arp_queue); neigh->arp_queue_len_bytes = 0; @@ -1554,8 +1555,8 @@ void neigh_table_init(struct neigh_table *tbl) write_unlock(&neigh_tbl_lock); if (unlikely(tmp)) { - printk(KERN_ERR "NEIGH: Registering multiple tables for " - "family %d\n", tbl->family); + pr_err("Registering multiple tables for family %d\n", + tbl->family); dump_stack(); } } @@ -1571,7 +1572,7 @@ int neigh_table_clear(struct neigh_table *tbl) pneigh_queue_purge(&tbl->proxy_queue); neigh_ifdown(tbl, NULL); if (atomic_read(&tbl->entries)) - printk(KERN_CRIT "neighbour leakage\n"); + pr_crit("neighbour leakage\n"); write_lock(&neigh_tbl_lock); for (tp = &neigh_tables; *tp; tp = &(*tp)->next) { if (*tp == tbl) { diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index 31a5ae51a45c..dddbacb8f28c 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -1,3 +1,5 @@ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -212,8 +214,8 @@ static void net_free(struct net *net) { #ifdef NETNS_REFCNT_DEBUG if (unlikely(atomic_read(&net->use_count) != 0)) { - printk(KERN_EMERG "network namespace not free! Usage: %d\n", - atomic_read(&net->use_count)); + pr_emerg("network namespace not free! Usage: %d\n", + atomic_read(&net->use_count)); return; } #endif diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c index ba6900f73900..09eda68b6763 100644 --- a/net/core/netprio_cgroup.c +++ b/net/core/netprio_cgroup.c @@ -9,6 +9,8 @@ * Authors: Neil Horman */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -88,7 +90,7 @@ static void extend_netdev_table(struct net_device *dev, u32 new_len) old_priomap = rtnl_dereference(dev->priomap); if (!new_priomap) { - printk(KERN_WARNING "Unable to alloc new priomap!\n"); + pr_warn("Unable to alloc new priomap!\n"); return; } @@ -136,7 +138,7 @@ static struct cgroup_subsys_state *cgrp_create(struct cgroup *cgrp) ret = get_prioidx(&cs->prioidx); if (ret != 0) { - printk(KERN_WARNING "No space in priority index array\n"); + pr_warn("No space in priority index array\n"); kfree(cs); return ERR_PTR(ret); } diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 2a1871942317..7a10f0894152 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -36,6 +36,8 @@ * The functions in this file will not compile correctly with gcc 2.4.x */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -118,11 +120,10 @@ static const struct pipe_buf_operations sock_pipe_buf_ops = { */ static void skb_over_panic(struct sk_buff *skb, int sz, void *here) { - printk(KERN_EMERG "skb_over_panic: text:%p len:%d put:%d head:%p " - "data:%p tail:%#lx end:%#lx dev:%s\n", - here, skb->len, sz, skb->head, skb->data, - (unsigned long)skb->tail, (unsigned long)skb->end, - skb->dev ? skb->dev->name : ""); + pr_emerg("%s: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx dev:%s\n", + __func__, here, skb->len, sz, skb->head, skb->data, + (unsigned long)skb->tail, (unsigned long)skb->end, + skb->dev ? skb->dev->name : ""); BUG(); } @@ -137,11 +138,10 @@ static void skb_over_panic(struct sk_buff *skb, int sz, void *here) static void skb_under_panic(struct sk_buff *skb, int sz, void *here) { - printk(KERN_EMERG "skb_under_panic: text:%p len:%d put:%d head:%p " - "data:%p tail:%#lx end:%#lx dev:%s\n", - here, skb->len, sz, skb->head, skb->data, - (unsigned long)skb->tail, (unsigned long)skb->end, - skb->dev ? skb->dev->name : ""); + pr_emerg("%s: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx dev:%s\n", + __func__, here, skb->len, sz, skb->head, skb->data, + (unsigned long)skb->tail, (unsigned long)skb->end, + skb->dev ? skb->dev->name : ""); BUG(); } diff --git a/net/core/sock.c b/net/core/sock.c index 9d144ee7e379..5efcd6307fa7 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -89,6 +89,8 @@ * 2 of the License, or (at your option) any later version. */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -297,9 +299,8 @@ static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen) *timeo_p = 0; if (warned < 10 && net_ratelimit()) { warned++; - printk(KERN_INFO "sock_set_timeout: `%s' (pid %d) " - "tries to set negative timeout\n", - current->comm, task_pid_nr(current)); + pr_info("%s: `%s' (pid %d) tries to set negative timeout\n", + __func__, current->comm, task_pid_nr(current)); } return 0; } @@ -317,8 +318,8 @@ static void sock_warn_obsolete_bsdism(const char *name) static char warncomm[TASK_COMM_LEN]; if (strcmp(warncomm, current->comm) && warned < 5) { strcpy(warncomm, current->comm); - printk(KERN_WARNING "process `%s' is using obsolete " - "%s SO_BSDCOMPAT\n", warncomm, name); + pr_warn("process `%s' is using obsolete %s SO_BSDCOMPAT\n", + warncomm, name); warned++; } } @@ -1238,8 +1239,8 @@ static void __sk_free(struct sock *sk) sock_disable_timestamp(sk, SK_FLAGS_TIMESTAMP); if (atomic_read(&sk->sk_omem_alloc)) - printk(KERN_DEBUG "%s: optmem leakage (%d bytes) detected.\n", - __func__, atomic_read(&sk->sk_omem_alloc)); + pr_debug("%s: optmem leakage (%d bytes) detected\n", + __func__, atomic_read(&sk->sk_omem_alloc)); if (sk->sk_peer_cred) put_cred(sk->sk_peer_cred); @@ -2424,7 +2425,7 @@ static void assign_proto_idx(struct proto *prot) prot->inuse_idx = find_first_zero_bit(proto_inuse_idx, PROTO_INUSE_NR); if (unlikely(prot->inuse_idx == PROTO_INUSE_NR - 1)) { - printk(KERN_ERR "PROTO_INUSE_NR exhausted\n"); + pr_err("PROTO_INUSE_NR exhausted\n"); return; } @@ -2454,8 +2455,8 @@ int proto_register(struct proto *prot, int alloc_slab) NULL); if (prot->slab == NULL) { - printk(KERN_CRIT "%s: Can't create sock SLAB cache!\n", - prot->name); + pr_crit("%s: Can't create sock SLAB cache!\n", + prot->name); goto out; } @@ -2469,8 +2470,8 @@ int proto_register(struct proto *prot, int alloc_slab) SLAB_HWCACHE_ALIGN, NULL); if (prot->rsk_prot->slab == NULL) { - printk(KERN_CRIT "%s: Can't create request sock SLAB cache!\n", - prot->name); + pr_crit("%s: Can't create request sock SLAB cache!\n", + prot->name); goto out_free_request_sock_slab_name; } } -- cgit v1.2.3 From cad456d5abbb6307be7a658d701bc44ca689e906 Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Thu, 17 May 2012 10:04:00 +0000 Subject: drop_monitor: convert to modular building When I first wrote drop monitor I wrote it to just build monolithically. There is no reason it can't be built modularly as well, so lets give it that flexibiity. I've tested this by building it as both a module and monolithically, and it seems to work quite well Change notes: v2) * fixed for_each_present_cpu loops to be more correct as per Eric D. * Converted exit path failures to BUG_ON as per Ben H. v3) * Converted del_timer to del_timer_sync to close race noted by Ben H. Signed-off-by: Neil Horman CC: "David S. Miller" CC: Eric Dumazet CC: Ben Hutchings Reviewed-by: Ben Hutchings Signed-off-by: David S. Miller --- net/Kconfig | 2 +- net/core/drop_monitor.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 3 deletions(-) (limited to 'net/core/drop_monitor.c') diff --git a/net/Kconfig b/net/Kconfig index 4c06c7c513a4..da1282359303 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -296,7 +296,7 @@ config NET_TCPPROBE module will be called tcp_probe. config NET_DROP_MONITOR - boolean "Network packet drop alerting service" + tristate "Network packet drop alerting service" depends on INET && EXPERIMENTAL && TRACEPOINTS ---help--- This feature provides an alerting service to userspace in the diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c index eca00a96bcf3..3252e7e0a005 100644 --- a/net/core/drop_monitor.c +++ b/net/core/drop_monitor.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -263,9 +264,15 @@ static int set_all_monitor_traces(int state) switch (state) { case TRACE_ON: + if (!try_module_get(THIS_MODULE)) { + rc = -ENODEV; + break; + } + rc |= register_trace_kfree_skb(trace_kfree_skb_hit, NULL); rc |= register_trace_napi_poll(trace_napi_poll_hit, NULL); break; + case TRACE_OFF: rc |= unregister_trace_kfree_skb(trace_kfree_skb_hit, NULL); rc |= unregister_trace_napi_poll(trace_napi_poll_hit, NULL); @@ -281,6 +288,9 @@ static int set_all_monitor_traces(int state) kfree_rcu(new_stat, rcu); } } + + module_put(THIS_MODULE); + break; default: rc = 1; @@ -406,7 +416,7 @@ static int __init init_net_drop_monitor(void) rc = 0; - for_each_present_cpu(cpu) { + for_each_possible_cpu(cpu) { data = &per_cpu(dm_cpu_data, cpu); data->cpu = cpu; INIT_WORK(&data->dm_alert_work, send_dm_alert); @@ -425,4 +435,36 @@ out: return rc; } -late_initcall(init_net_drop_monitor); +static void exit_net_drop_monitor(void) +{ + struct per_cpu_dm_data *data; + int cpu; + + BUG_ON(unregister_netdevice_notifier(&dropmon_net_notifier)); + + /* + * Because of the module_get/put we do in the trace state change path + * we are guarnateed not to have any current users when we get here + * all we need to do is make sure that we don't have any running timers + * or pending schedule calls + */ + + for_each_possible_cpu(cpu) { + data = &per_cpu(dm_cpu_data, cpu); + del_timer_sync(&data->send_timer); + cancel_work_sync(&data->dm_alert_work); + /* + * At this point, we should have exclusive access + * to this struct and can free the skb inside it + */ + kfree_skb(data->skb); + } + + BUG_ON(genl_unregister_family(&net_drop_monitor_family)); +} + +module_init(init_net_drop_monitor); +module_exit(exit_net_drop_monitor); + +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Neil Horman "); -- cgit v1.2.3 From 3fdcbd453152329002f12dfda0be90b714458164 Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Tue, 29 May 2012 09:30:42 +0000 Subject: drop_monitor: Add module alias to enable automatic module loading Now that we have module alias macros for generic netlink families, lets use those to mark modules with the appropriate family names for loading Signed-off-by: Neil Horman CC: Eric Dumazet CC: David Miller Signed-off-by: David S. Miller --- net/core/drop_monitor.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net/core/drop_monitor.c') diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c index 3252e7e0a005..ea5fb9fcc3f5 100644 --- a/net/core/drop_monitor.c +++ b/net/core/drop_monitor.c @@ -468,3 +468,4 @@ module_exit(exit_net_drop_monitor); MODULE_LICENSE("GPL v2"); MODULE_AUTHOR("Neil Horman "); +MODULE_ALIAS_GENL_FAMILY("NET_DM"); -- cgit v1.2.3