Re: [PATCH v3 03/22] netconsole: Introduce 'enabled' state-machine

[Michał Mirosław]

2010/12/14 Mike Waychison <mikew@xxxxxxxxxx>:
> Representing the internal state within netconsole isn't really a boolean
> value, but rather a state machine with transitions.
[...]
> 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
[...]
> @@ -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?

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/