All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	mingo@kernel.org, mgorman@suse.de, juri.lelli@redhat.com,
	vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
	rostedt@goodmis.org, bsegall@google.com, bristot@redhat.com,
	joshdon@google.com, valentin.schneider@arm.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 6/9] debugfs: Implement debugfs_create_str()
Date: Sat, 27 Mar 2021 11:41:36 +0100	[thread overview]
Message-ID: <YF8L4BMSMeuIq2JS@kroah.com> (raw)
In-Reply-To: <20210326151912.GL4746@worktop.programming.kicks-ass.net>

On Fri, Mar 26, 2021 at 04:19:12PM +0100, Peter Zijlstra wrote:
> On Fri, Mar 26, 2021 at 03:58:37PM +0100, Rasmus Villemoes wrote:
> > >   kmalloc(len + 2, ...);
> > 
> > No, because nul-terminating the stuff you pass to
> > simple_read_from_buffer is pointless cargo-culting. Yeah, read_file_bool
> > does it, but that's just bogus.
> 
> Urgh, feel yuck to not have it zero terminated, but if you feel strongly
> about it I suppose I can make it go away.
> 
> > > 	len = strscpy(copy, str, len);
> > > 	if (len < 0)
> > > 		return len;
> > 
> > To what end? The only way that could possibly return -EFOO is if the
> > nul-terminator in str vanished between the strlen() and here, and in
> > that case you have bigger problems.
> 
> There are strings in the kernel which we rewrite in most ugly ways,
> task_struct::comm comes to mind. Best be paranoid.
> 
> > > Copy-pasta from debugfs_*_bool(). This thing seems to export everything
> > > and I figured I'd go along with that.
> > 
> > I thought the convention was not to export anything until the kernel
> > itself had a (modular) user.
> 
> That's generally how I feel too. But this really isn't my subsystem so I
> more or less try to mimmick what I see done there.

I'll take some time next week and go through and remove any exports in
debugfs that are not actually needed for exports as it's been a while
since I last looked...

But that should not affect this change, it's fine to me as-is.

thanks,

greg k-h

  reply	other threads:[~2021-03-27 10:41 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-26 10:33 [PATCH 0/9] sched: Clean up SCHED_DEBUG Peter Zijlstra
2021-03-26 10:33 ` [PATCH 1/9] sched/numa: Allow runtime enabling/disabling of NUMA balance without SCHED_DEBUG Peter Zijlstra
2021-03-26 10:33 ` [PATCH 2/9] sched: Remove sched_schedstats sysctl out from under SCHED_DEBUG Peter Zijlstra
2021-03-26 10:33 ` [PATCH 3/9] sched: Dont make LATENCYTOP select SCHED_DEBUG Peter Zijlstra
2021-03-26 10:33 ` [PATCH 4/9] sched: Move SCHED_DEBUG to debugfs Peter Zijlstra
2021-03-26 11:06   ` Greg KH
2021-04-07 10:46   ` Valentin Schneider
2021-04-07 12:26     ` Peter Zijlstra
2021-04-07 12:57       ` Valentin Schneider
2021-03-26 10:33 ` [PATCH 5/9] sched,preempt: Move preempt_dynamic to debug.c Peter Zijlstra
2021-03-26 10:33 ` [PATCH 6/9] debugfs: Implement debugfs_create_str() Peter Zijlstra
2021-03-26 11:05   ` Greg KH
2021-03-26 11:18     ` Peter Zijlstra
2021-03-26 11:30       ` Greg KH
2021-03-26 11:38         ` [PATCH v2 " Peter Zijlstra
2021-03-26 12:18           ` Greg KH
2021-03-26 12:53           ` Rasmus Villemoes
2021-03-26 12:57             ` Greg KH
2021-03-26 13:10               ` Rasmus Villemoes
2021-03-26 14:12                 ` Peter Zijlstra
2021-03-26 14:19                   ` Greg KH
2021-03-26 14:22             ` Peter Zijlstra
2021-03-26 14:58               ` Rasmus Villemoes
2021-03-26 15:19                 ` Peter Zijlstra
2021-03-27 10:41                   ` Greg KH [this message]
2021-03-26 14:50   ` [PATCH v3 " Peter Zijlstra
2021-03-27 10:42     ` Greg KH
2021-03-27 22:24   ` [PATCH " Al Viro
2021-03-28  0:33     ` Steven Rostedt
2021-03-26 10:33 ` [PATCH 7/9] sched,debug: Convert sysctl sched_domains to debugfs Peter Zijlstra
2021-03-26 13:11   ` Dietmar Eggemann
2021-04-07 10:46   ` Valentin Schneider
2021-04-07 12:18     ` Peter Zijlstra
2021-03-26 10:34 ` [PATCH 8/9] sched: Move /proc/sched_debug " Peter Zijlstra
2021-03-26 11:05   ` Greg KH
2021-03-26 10:34 ` [PATCH 9/9] sched,fair: Alternative sched_slice() Peter Zijlstra
2021-03-26 12:08   ` Dietmar Eggemann
2021-03-26 14:07     ` Peter Zijlstra
2021-03-26 15:37   ` Vincent Guittot
2021-03-26 18:30     ` Peter Zijlstra
2021-04-12 10:14 [PATCH v2 0/9] sched: Clean up SCHED_DEBUG Peter Zijlstra
2021-04-12 10:14 ` [PATCH v2 6/9] debugfs: Implement debugfs_create_str() Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YF8L4BMSMeuIq2JS@kroah.com \
    --to=greg@kroah.com \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=joshdon@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mgorman@suse.de \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=valentin.schneider@arm.com \
    --cc=vincent.guittot@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.