2010/12/14 Michał Mirosław [off-list ref]:

2010/12/14 Mike Waychison [off-list ref]:

quoted

Representing the internal state within netconsole isn't really a boolean
value, but rather a state machine with transitions.

[...]

quoted

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 6e16888..288a025 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c

[...]

quoted

@@ -350,9 +362,9 @@ static ssize_t store_enabled(struct netconsole_target *nt,

               err = netpoll_setup(&nt->np);
               spin_lock_irqsave(&target_list_lock, flags);
               if (err)
-                       nt->enabled = 0;
+                       nt->np_state = NETPOLL_DISABLED;
               else
-                       nt->enabled = 1;
+                       nt->np_state = NETPOLL_ENABLED;
               spin_unlock_irqrestore(&target_list_lock, flags);
               if (err)
                       return err;

[...]

Since the spinlock protects only nt->np_state setting, you might be
able to remove it altogether and use cmpxchg() where nt->np_state
transitions from enabled or disabled state.

Maybe the locking scheme just needs more thought altogether?

The target_list_lock protects the list, as well as the state
transitions.  This makes iterating through the list and getting a
consistent view of the targets a lot easier when it comes time to
transmitting packets we are guaranteed that nobody is changing the
target state underneath us if nt->np_state == NETPOLL_ENABLED while we
hold the lock.