Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1310079 > unrolled thread

Re: [PATCH 2/8] kernfs: implement kernfs_walk_and_get()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/8] kernfs: implement kernfs_walk_and_get() Geert Uytterhoeven <geert@linux-m68k.org> - 2016-01-15 13:30 +0100
    [PATCH] kernfs: make kernfs_walk_ns() use kernfs_pr_cont_buf[] Tejun Heo <tj@kernel.org> - 2016-01-15 18:40 +0100
      Re: [PATCH] kernfs: make kernfs_walk_ns() use kernfs_pr_cont_buf[] Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-01-15 23:10 +0100

#1310079 — Re: [PATCH 2/8] kernfs: implement kernfs_walk_and_get()

On Mon, Dec 7, 2015 at 11:38 PM, Tejun Heo <tj@kernel.org> wrote:
> Implement kernfs_walk_and_get() which is similar to
> kernfs_find_and_get() but can walk a path instead of just a name.
>
> v2: Use strlcpy() instead of strlen() + memcpy() as suggested by
>     David.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: David Miller <davem@davemloft.net>
> ---
>  fs/kernfs/dir.c        | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/kernfs.h | 12 ++++++++++++
>  2 files changed, 58 insertions(+)
>
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index 91e0045..742bf4a 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -694,6 +694,29 @@ static struct kernfs_node *kernfs_find_ns(struct kernfs_node *parent,
>         return NULL;
>  }
>
> +static struct kernfs_node *kernfs_walk_ns(struct kernfs_node *parent,
> +                                         const unsigned char *path,
> +                                         const void *ns)
> +{
> +       static char path_buf[PATH_MAX]; /* protected by kernfs_mutex */

This is showing up in bloat-o-meter as increasing kernel size by 4 KiB.

> +       size_t len = strlcpy(path_buf, path, PATH_MAX);

The full input string is copied because strsep() replaces the delimiters by
zeroes?
Can this be handled in some other way?

> +       char *p = path_buf;
> +       char *name;
> +
> +       lockdep_assert_held(&kernfs_mutex);
> +
> +       if (len >= PATH_MAX)
> +               return NULL;
> +
> +       while ((name = strsep(&p, "/")) && parent) {
> +               if (*name == '\0')
> +                       continue;
> +               parent = kernfs_find_ns(parent, name, ns);
> +       }
> +
> +       return parent;
> +}

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [next] | [standalone]

#1310307 — [PATCH] kernfs: make kernfs_walk_ns() use kernfs_pr_cont_buf[]

kernfs_walk_ns() uses a static path_buf[PATH_MAX] to separate out path
components.  Keeping around the 4k buffer just for kernfs_walk_ns() is
wasteful.  This patch makes it piggyback on kernfs_pr_cont_buf[]
instead.  This requires kernfs_walk_ns() to hold kernfs_rename_lock.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Hello,

Greg, can you please route this one?

Thanks.

 fs/kernfs/dir.c |   19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -698,15 +698,22 @@ static struct kernfs_node *kernfs_walk_n
 					  const unsigned char *path,
 					  const void *ns)
 {
-	static char path_buf[PATH_MAX];	/* protected by kernfs_mutex */
-	size_t len = strlcpy(path_buf, path, PATH_MAX);
-	char *p = path_buf;
-	char *name;
+	size_t len;
+	char *p, *name;
 
 	lockdep_assert_held(&kernfs_mutex);
 
-	if (len >= PATH_MAX)
+	/* grab kernfs_rename_lock to piggy back on kernfs_pr_cont_buf */
+	spin_lock_irq(&kernfs_rename_lock);
+
+	len = strlcpy(kernfs_pr_cont_buf, path, sizeof(kernfs_pr_cont_buf));
+
+	if (len >= sizeof(kernfs_pr_cont_buf)) {
+		spin_unlock_irq(&kernfs_rename_lock);
 		return NULL;
+	}
+
+	p = kernfs_pr_cont_buf;
 
 	while ((name = strsep(&p, "/")) && parent) {
 		if (*name == '\0')
@@ -714,6 +721,8 @@ static struct kernfs_node *kernfs_walk_n
 		parent = kernfs_find_ns(parent, name, ns);
 	}
 
+	spin_unlock_irq(&kernfs_rename_lock);
+
 	return parent;
 }
 

[toc] | [prev] | [next] | [standalone]

#1310501 — Re: [PATCH] kernfs: make kernfs_walk_ns() use kernfs_pr_cont_buf[]

On Fri, Jan 15, 2016 at 12:30:14PM -0500, Tejun Heo wrote:
> kernfs_walk_ns() uses a static path_buf[PATH_MAX] to separate out path
> components.  Keeping around the 4k buffer just for kernfs_walk_ns() is
> wasteful.  This patch makes it piggyback on kernfs_pr_cont_buf[]
> instead.  This requires kernfs_walk_ns() to hold kernfs_rename_lock.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> Hello,
> 
> Greg, can you please route this one?

Thanks, will take this after 4.5-rc1 is out.

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web