Re: [PATCH] dev: use name hash for dev_seq_ops.

[Stephen Hemminger]

On Mon, 10 Oct 2011 11:43:20 +0300
Mihai Maruseac <mihai.maruseac@xxxxxxxxx> wrote:

> On Fri, Oct 7, 2011 at 7:24 PM, Stephen Hemminger <shemminger@xxxxxxxxxx> wrote:
> > On Fri,  7 Oct 2011 18:20:49 +0300
> > Mihai Maruseac <mihai.maruseac@xxxxxxxxx> wrote:
> >
> >> Instead of using the dev->next chain and trying to resync at each call to
> >> dev_seq_start, use this hash and store bucket number and bucket offset in
> >> seq->private field.
> >>
> >> As one can notice the improvement is of 1 order of magnitude.
> >
> > Good idea,
> > This will change the ordering of entries in /proc which may upset
> > some program, not a critical flaw but worth noting.
> >
> > Rather than recording the bucket and offset of last entry, another
> > alternative would be to just record the ifindex.
> >
> 
> I tried to record the ifindex but I think that using it and
> dev_get_by_index can result in an infinite loop or a NULL
> dereferrence. If a device is removed and ifindex points to it we'll
> get a NULL from dev_get_by_index. Checking for NULL and calling again
> dev_get_by_index will end in an infinite loop at the end of the hlist.
> 
> Augmenting the structure to also contain the number of indexes when
> the seq_file is opened returns to the current situation with two ints.
> Also, it is more prone to bugs caused by device removal while
> printing.

If ifindex has been deleted, the code should fall back to delivering
the next offset. That means for the rare case it would have the old
behavior of linear searching.  There is similar code already to
deal with /proc/net/route; it continues from the last address.
--
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/