On Fri, 2 Jul 2004 12:30:22 -0700
Andrew Morton <akpm@xxxxxxxx> wrote:
> Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> > Andrew Morton <akpm@xxxxxxxx> wrote:
> > >
> > > Could someone please take a look at the locking in
> > > net/ipv4/ipmr.c:ipmr_mfc_seq_next? It seems rather broken.
> >
> > Obfuscated yes, broken no.
>
> Really? Even if that goto
>
> if (it->cache == &mfc_unres_queue)
> goto end_of_list;
>
> is taken?
The problem is that the seq_file interface is trying to create an iterator
over a set of data. The interface expects that the usage will always fit
a given pattern.
The implied semantic is:
if returned handle is NULL, then nothing is locked and it->cache = NULL
if returned handle is in the mfc_cache_table then
it->cache points to mfc_cache_array and mrt_lock is held
if returned handle is in the mfc_unres_queue then
it->cache points to mfc_unres_queue and mfc_unres_lock is held
the seq_next is only valid to mean give me the next entry after the passed
handle
therefore if there are no more entries after the handle and the handle was on
the unresolved queue, then the end is reached.
When seq_stop is called, the it->cache pointer will be NULL so no unlock is
done.
|