Re: [PATCH] proc: show locks in /proc/pid/fdinfo/X

[Andrey Wagin <avagin@gmail.com>]

2015-03-12 22:23 GMT+03:00 Andrew Morton <akpm@linux-foundation.org>:
> On Thu, 12 Mar 2015 18:54:42 +0300 Andrew Vagin <avagin@parallels.com> wrote:
>
>> v2: use seq_has_overflowed() properly
>
> --- a/fs/proc/fd.c~proc-show-locks-in-proc-pid-fdinfo-x-v2
> +++ a/fs/proc/fd.c
> @@ -57,17 +57,15 @@ static int seq_show(struct seq_file *m,
>                    real_mount(file->f_path.mnt)->mnt_id);
>
>         show_fd_locks(m, file, files);
> -       ret = seq_has_overflowed(m);
> -       if (ret)
> +       if (seq_has_overflowed(m))
>                 goto out;
>
>         if (file->f_op->show_fdinfo)
>                 file->f_op->show_fdinfo(m, file);
> -       ret = seq_has_overflowed(m);
>
>  out:
>         fput(file);
> -       return ret;
> +       return 0;
>  }
>
>  static int seq_fdinfo_open(struct inode *inode, struct file *file)
>
>
> So it returns "success" when the output has overflowed?  Why this,
> rather than returning an error?

I have read fs/seq_file.c and looks like it's the right way.
seq_has_overflowed() is used to avoid useless work. If we call it or
don't call it, the result must be the same in both cases. So from this
point of view it looks logically correct too.

There are two interesting places from seq_file.c
                error = m->op->show(m, p);
                if (error < 0)
                        break;
                if (unlikely(error)) {
                        error = 0;
                        m->count = 0;
                }
                if (seq_has_overflowed(m))
                        goto Eoverflow;

...
                err = m->op->show(m, p);
                if (err < 0)
                        break;
                if (unlikely(err))
                        m->count = 0;
                if (unlikely(!m->count)) {
                        p = m->op->next(m, p, &pos);
                        m->index = pos;
                        continue;
                }
                if (m->count < m->size)
                        goto Fill;
                m->op->stop(m, p);
                kvfree(m->buf);
                m->count = 0;
                m->buf = seq_buf_alloc(m->size <<= 1);

Thanks,
Andrey