2018-11-19 11:19:54

by Yafang Shao

[permalink] [raw]
Subject: [PATCH] procfs: fix the output format in /proc/PID/wchan

Just add the missing newline.

Signed-off-by: Yafang Shao <[email protected]>
---
fs/proc/base.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index ce34654..d7f49fb 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -370,11 +370,12 @@ static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns,
wchan = get_wchan(task);
if (wchan && !lookup_symbol_name(wchan, symname)) {
seq_puts(m, symname);
+ seq_putc(m, '\n');
return 0;
}

print0:
- seq_putc(m, '0');
+ seq_puts(m, "0\n");
return 0;
}
#endif /* CONFIG_KALLSYMS */
--
1.8.3.1



2018-11-22 16:15:09

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] procfs: fix the output format in /proc/PID/wchan

On Mon, 19 Nov 2018 19:17:52 +0800 Yafang Shao <[email protected]> wrote:

> Just add the missing newline.
>
> ...
>
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -370,11 +370,12 @@ static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns,
> wchan = get_wchan(task);
> if (wchan && !lookup_symbol_name(wchan, symname)) {
> seq_puts(m, symname);
> + seq_putc(m, '\n');
> return 0;
> }
>
> print0:
> - seq_putc(m, '0');
> + seq_puts(m, "0\n");
> return 0;
> }
> #endif /* CONFIG_KALLSYMS */

What is presently wrong with the wchan output? The changelog
should explain such things, please.

Providing example output with the patch unapplied and then with the
patch applied would help us to understand the patch's effect.

Thanks.

2018-11-23 14:38:59

by Yafang Shao

[permalink] [raw]
Subject: Re: [PATCH] procfs: fix the output format in /proc/PID/wchan

On Thu, Nov 22, 2018 at 7:40 PM Alexey Dobriyan <[email protected]> wrote:
>
> On Wed, Nov 21, 2018 at 07:28:44PM -0800, Andrew Morton wrote:
> > On Mon, 19 Nov 2018 19:17:52 +0800 Yafang Shao <[email protected]> wrote:
> >
> > > Just add the missing newline.
> > >
> > > ...
> > >
> > > --- a/fs/proc/base.c
> > > +++ b/fs/proc/base.c
> > > @@ -370,11 +370,12 @@ static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns,
> > > wchan = get_wchan(task);
> > > if (wchan && !lookup_symbol_name(wchan, symname)) {
> > > seq_puts(m, symname);
> > > + seq_putc(m, '\n');
> > > return 0;
> > > }
> > >
> > > print0:
> > > - seq_putc(m, '0');
> > > + seq_puts(m, "0\n");
> > > return 0;
> > > }
> > > #endif /* CONFIG_KALLSYMS */
> >
> > What is presently wrong with the wchan output? The changelog
> > should explain such things, please.
>
> It is just newline to make "cat /proc/*/wchan" output look cool.
> But newline can break something.

Could you pls. show some examples for what the newline may break ?

Thanks
Yafang

2018-11-23 14:55:42

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] procfs: fix the output format in /proc/PID/wchan

On Wed, Nov 21, 2018 at 07:28:44PM -0800, Andrew Morton wrote:
> On Mon, 19 Nov 2018 19:17:52 +0800 Yafang Shao <[email protected]> wrote:
>
> > Just add the missing newline.
> >
> > ...
> >
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -370,11 +370,12 @@ static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns,
> > wchan = get_wchan(task);
> > if (wchan && !lookup_symbol_name(wchan, symname)) {
> > seq_puts(m, symname);
> > + seq_putc(m, '\n');
> > return 0;
> > }
> >
> > print0:
> > - seq_putc(m, '0');
> > + seq_puts(m, "0\n");
> > return 0;
> > }
> > #endif /* CONFIG_KALLSYMS */
>
> What is presently wrong with the wchan output? The changelog
> should explain such things, please.

It is just newline to make "cat /proc/*/wchan" output look cool.
But newline can break something.

2018-11-23 17:13:19

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] procfs: fix the output format in /proc/PID/wchan

On Thu, Nov 22, 2018 at 09:29:52PM +0800, Yafang Shao wrote:
> On Thu, Nov 22, 2018 at 7:40 PM Alexey Dobriyan <[email protected]> wrote:
> >
> > On Wed, Nov 21, 2018 at 07:28:44PM -0800, Andrew Morton wrote:
> > > On Mon, 19 Nov 2018 19:17:52 +0800 Yafang Shao <[email protected]> wrote:
> > >
> > > > Just add the missing newline.
> > > >
> > > > ...
> > > >
> > > > --- a/fs/proc/base.c
> > > > +++ b/fs/proc/base.c
> > > > @@ -370,11 +370,12 @@ static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns,
> > > > wchan = get_wchan(task);
> > > > if (wchan && !lookup_symbol_name(wchan, symname)) {
> > > > seq_puts(m, symname);
> > > > + seq_putc(m, '\n');
> > > > return 0;
> > > > }
> > > >
> > > > print0:
> > > > - seq_putc(m, '0');
> > > > + seq_puts(m, "0\n");
> > > > return 0;
> > > > }
> > > > #endif /* CONFIG_KALLSYMS */
> > >
> > > What is presently wrong with the wchan output? The changelog
> > > should explain such things, please.
> >
> > It is just newline to make "cat /proc/*/wchan" output look cool.
> > But newline can break something.
>
> Could you pls. show some examples for what the newline may break ?

char buf[16];
rv = read(fd, buf, sizeof(buf));
assert(rv == 1);

2018-11-24 08:21:58

by Yafang Shao

[permalink] [raw]
Subject: Re: [PATCH] procfs: fix the output format in /proc/PID/wchan

On Thu, Nov 22, 2018 at 9:38 PM Alexey Dobriyan <[email protected]> wrote:
>
> On Thu, Nov 22, 2018 at 09:29:52PM +0800, Yafang Shao wrote:
> > On Thu, Nov 22, 2018 at 7:40 PM Alexey Dobriyan <[email protected]> wrote:
> > >
> > > On Wed, Nov 21, 2018 at 07:28:44PM -0800, Andrew Morton wrote:
> > > > On Mon, 19 Nov 2018 19:17:52 +0800 Yafang Shao <[email protected]> wrote:
> > > >
> > > > > Just add the missing newline.
> > > > >
> > > > > ...
> > > > >
> > > > > --- a/fs/proc/base.c
> > > > > +++ b/fs/proc/base.c
> > > > > @@ -370,11 +370,12 @@ static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns,
> > > > > wchan = get_wchan(task);
> > > > > if (wchan && !lookup_symbol_name(wchan, symname)) {
> > > > > seq_puts(m, symname);
> > > > > + seq_putc(m, '\n');
> > > > > return 0;
> > > > > }
> > > > >
> > > > > print0:
> > > > > - seq_putc(m, '0');
> > > > > + seq_puts(m, "0\n");
> > > > > return 0;
> > > > > }
> > > > > #endif /* CONFIG_KALLSYMS */
> > > >
> > > > What is presently wrong with the wchan output? The changelog
> > > > should explain such things, please.
> > >
> > > It is just newline to make "cat /proc/*/wchan" output look cool.
> > > But newline can break something.
> >
> > Could you pls. show some examples for what the newline may break ?
>
> char buf[16];
> rv = read(fd, buf, sizeof(buf));
> assert(rv == 1);

That's really a break, so we can't apply this patch.

Hi Andrew,
I found that you have applied this patch to -mm tree, could you pls.
help revert it as it may break something ?

Thanks
Yafang