> It is not discarded. Seems, you did not noticed that the line is splitted.
> Right?
I just checked the usage with grep, so I didn't notice any line splits.
But as the neigh_parm pointer is passed as an argument to
neigh_sysctl_register, wouldn't it be better to install the structure
there?
> : The attached patch also corrects a race condition in
> : neigh_parms_release, where I think you want the start_bh_atomic
> : exclusion before walking the parms list.
>
> Nope. This list is never modified by any interrupts,
> so that bh protection is not necessary.
> Now only list change is "protected":
>
> : start_bh_atomic();
> : *p = parms->next;
> : end_bh_atomic();
>
> And it is overkill i.e. it is made only to mark potentially
> dangerous place. Assignment is atomic in any case.
Hmm, if the list is never modified by interrupts, you really shouldn't
have the start_bh_atomic/end_bh_atomic sequence there. First of all, it
wastes time, and as you point out the assignment is atomic.
Secondly (and more importantly) having an incorrect usage of the
xxx_bh_atomic calls makes it difficult for anyone to review the code.
All of the other usages within neighbour.c follow the correct pattern --
start_bh_atomic before the list search, end_bh_atomic on any exit path
or after completing the search.
So,I would suggest using a comment to mark potentially dangerous places,
e.g.
/*
* We don't need start/end_bh_atomic here, as
* this list is never modified by interrupts.
*/
Regards,
Bill