On (06/01/17 22:34), Julian Anastasov wrote:
> > + np = &nht->hash_buckets[hash_val];
> > + while ((n = rcu_dereference_protected(*np,
> > + lockdep_is_held(&tbl->lock))) != NULL) {
>
> checkpatch shows some warnings:
>
> scripts/checkpatch.pl --strict /tmp/file.patch
Yes, checkpatch complained about
"CHECK: Alignment should match open parenthesis"
but trying to meet that requirement (without exceeding the 80 char limit)
would need additional variables, and I noticed that there are
other places in the code (e.g., neigh_forced_gc()) where the alignment
prescription is not observed, so I let things follow existing style..
[ In neigh_remove_one()]
> In case there is another patch version,
> the retval can be removed:
Let me see if there are additional review comments, and I can update
with the retval removed.
Thanks much for the review!
> Looks like we can also call neigh_remove_one only when !err.
> But this is some corner case when n->dead is set by GC and entry
> was unlinked, neigh_remove_one simply will not find it in the list,
> so it is not fatal to call neigh_remove_one unconditionally.
> > @@ -1113,13 +1113,17 @@ static int arp_invalidate(struct net_device *dev,
> > __be32 ip)
:
> Here the same race with GC already assigned
> neigh->dead to 1 is possible but it is more tricky to catch
> that exactly neigh_update() has failed. So, may be better to
> call neigh_remove_one like now.
