On Fri, Apr 10, 2020 at 07:23:47AM +0200, Miklos Szeredi wrote:
> > Hmm... I wonder if it would be better to do something like
> > if (!*pos)
> > prev = &p->ns->list.next;
> > else
> > prev = &p->mnt.mnt_list.next;
> > mnt = mnt_skip_cursors(p->ns, prev);
> >
> > > static void *m_next(struct seq_file *m, void *v, loff_t *pos)
> > > {
> > > struct proc_mounts *p = m->private;
> > > + struct mount *mnt = v;
> > > +
> > > + lock_ns_list(p->ns);
> > > + mnt = mnt_skip_cursors(p->ns, list_next_entry(mnt, mnt_list));
> >
> > ... and mnt = mnt_skip_cursors(p->ns, &mnt->mnt_list.next);
>
> If you prefer that, yes. Functionally it's equivalent.
Sure, it's just that I suspect that result will be somewhat more
readable that way.
Incidentally, there might be another benefit - both &p->ns->list.next
and &p->mnt.mnt_list.next are not going to change. So calculation of
prev can be lifter out of lock_ns_list() and _that_ promises something
more tasty - all callers of mnt_skip_cursors() are immediately
surrounded by lock_ns_list()/unlock_ns_list() and those can be moved
inside the damn thing.
