Re: [PATCH v3 1/1 net] net: fec: fix kernel oops when plug/unplug cable many times

[Lucas Stach <l.stach@pengutronix.de>]

Am Montag, den 06.05.2013, 17:56 +0800 schrieb Frank Li:
> 2013/5/6 Lucas Stach <l.stach@pengutronix.de>:
> > Hi Frank,
> >
> > Am Donnerstag, den 02.05.2013, 08:55 +0800 schrieb Frank Li:
[...]
> >> diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
> >> index eb43729..a367b21 100644
> >> --- a/drivers/net/ethernet/freescale/fec.h
> >> +++ b/drivers/net/ethernet/freescale/fec.h
> >> @@ -260,7 +260,8 @@ struct fec_enet_private {
> >>       int hwts_rx_en;
> >>       int hwts_tx_en;
> >>       struct timer_list time_keep;
> >> -
> >> +     struct delayed_work delay_work;
> >> +     int timeout;
> > I suspect you are going to add more variables like this in the future if
> > you are using the workqueue more extensively. This is a massive
> > pollution of the fec_enet_private struct. Please make this a bitfield
> > with a proper define for the timeout, so we can reuse one variable for
> > different tasks, or even split this out into it's own struct for the
> > delayed tasks.
> 
> Bit | & is not atomic.
> So I prefer use two variable. the real work in delay work is very
> simple for future workaround.

Ah yes, that's right. Could you then please change the type of the
variable to bool to make it clear that nothing other than true/false
should go into this?

I would still prefer to group the delayed work related things together
in one struct to make the driver more readable overall. Like this:
struct fec_delayed_work {
	struct delayed_work delay_work;
	bool timeout;
	bool future_workaround;
	bool more_delayed_things;
};
And just include this struct in fec_enet_private.

-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |