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

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

On Thu, Apr 09, 2020 at 11:22:14PM +0200, Miklos Szeredi wrote:

> @@ -1249,42 +1277,50 @@ struct vfsmount *mnt_clone_internal(cons
>  static void *m_start(struct seq_file *m, loff_t *pos)
>  {
>  	struct proc_mounts *p = m->private;
> +	struct mount *mnt = NULL;
>  
>  	down_read(&namespace_sem);
> -	if (p->cached_event == p->ns->event) {
> -		void *v = p->cached_mount;
> -		if (*pos == p->cached_index)
> -			return v;
> -		if (*pos == p->cached_index + 1) {
> -			v = seq_list_next(v, &p->ns->list, &p->cached_index);
> -			return p->cached_mount = v;
> -		}
> -	}
> +	lock_ns_list(p->ns);
> +	if (!*pos)
> +		list_move(&p->cursor.mnt_list, &p->ns->list);
> +	if (!list_empty(&p->cursor.mnt_list))
> +		mnt = mnt_skip_cursors(p->ns, &p->cursor);
> +	unlock_ns_list(p->ns);

Huh?  What's that if (!list_empty()) about?  The case where we have reached
the end of list, then did a read() with an lseek() in between?

If so, then this is out of place under your spinlock; "is on the list"
state changes only synchronously (seq_file ->lock serializes all of
that).  *If* this is what you've meant, I'd suggest
	/* read after we'd reached the end? */
	if (*pos && list_empty(...))
		return NULL;
	lock_ns_list(p->ns);
	if (!*pos)
		list_move(...); /* rewind on lseek or initial read */
	mnt = mnt_skip_cursors(...);
	unlock_ns_list(p->ns);

Or am I misreading your intent there?  Confused...