Thread (37 messages) 37 messages, 5 authors, 2010-12-17

Re: [PATCH v3 02/22] netconsole: Introduce locking over the netpoll fields

From: Michał Mirosław <hidden>
Date: 2010-12-14 23:15:26
Also in: lkml, netdev

2010/12/14 Mike Waychison [off-list ref]:
quoted hunk ↗ jump to hunk
The netconsole driver currently doesn't do any locking over its
configuration fields.  This can cause problems if we were to ever have
concurrent writing to fields while somebody is enabling the service.

For simplicity, this patch extends targets_list_lock to cover all
configuration fields within the targets.  Macros are also added here to
wrap accessors so that we check whether the target has been enabled with
locking handled.

Signed-off-by: Mike Waychison <redacted>
Acked-by: Matt Mackall <redacted>
---
 drivers/net/netconsole.c |  114 ++++++++++++++++++++++++++--------------------
 1 files changed, 64 insertions(+), 50 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index c87a49e..6e16888 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -327,6 +327,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
                            const char *buf,
                            size_t count)
 {
+       unsigned long flags;
       int err;
       long enabled;
@@ -335,6 +336,10 @@ static ssize_t store_enabled(struct netconsole_target *nt,
               return enabled;

       if (enabled) {  /* 1 */
+               spin_lock_irqsave(&target_list_lock, flags);
+               if (nt->enabled)
+                       goto busy;
+               spin_unlock_irqrestore(&target_list_lock, flags);
This looks wrong. Unless there is another lock or mutex covering this
function, at this point (after spin_unlock_irqrestore()) another
thread might set nt->enabled = 1.

Best Regards,
Michał Mirosław
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help