Re: [PATCH v4] proc/mounts: add cursor

[Al Viro <viro@zeniv.linux.org.uk>]

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.