Re: hotplug e1000 failed after 32 times

[Andrew Morton]

Li Shaohua <shaohua.li@xxxxxxxxx> wrote:
>
> I'm testing a hotplug driver. In my test, I will hot add/remove an e1000
>  NIC frequently. The result is my hot add failed after 32 times hotadd.
>  After looking at the code of e1000 driver, I found
>  e1000_adapter->bd_number has maxium limitation of 32, and it increased
>  one every hot add. Looks like the remove driver routine didn't free the
>  'bd_number', so hot add failed after 32 times. Below patch fixes this
>  issue.

Yeah.  I think you'll find that damn near every net driver in the kernel
has this problem.  I think it would be better to create a little suite of
library functions in net/core/dev.c to handle this situation.

Maybe something like

struct net_boards {
	struct idr idr;
	int max_boards;
}

void net_boards_init(struct net_boards *net_boards, int max_boards);
int net_board_alloc(struct net_boards *net_boards);
int net_boards_free(struct net_boards *net_boards, int board_no);

(I wonder where the locking should be performed?)

This is a pretty thin wrapper around the idr code and actually is quite
generic and has nothing to do with networking so you might end up deciding
to rename things and to move the code into idr.c

> -	adapter->bd_number = cards_found;
> +	adapter->bd_number = e1000_alloc_bd_number();;

Extra semicolon.

> +	TxDescriptors[bd_number] = 
> +	RxDescriptors[bd_number] = 
> +	Speed[bd_number] =
> +	Duplex[bd_number] =
> +	AutoNeg[bd_number] =
> +	FlowControl[bd_number] =
> +	XsumRX[bd_number] =
> +	TxIntDelay[bd_number] =
> +	TxAbsIntDelay[bd_number] =
> +	RxIntDelay[bd_number] =
> +	RxAbsIntDelay[bd_number] =
> +	InterruptThrottleRate[bd_number] = OPTION_UNSET;

Unpopular coding style.  Please just do

	RxAbsIntDelay[bd_number] = OPTION_UNSET;
	InterruptThrottleRate[bd_number] = OPTION_UNSET;

etc.
-
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/