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

Groups > linux.kernel > #1408543 > unrolled thread

Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function"

Back to article view | Back to linux.kernel

Contents

  Build errors in mainline due to "fs/namei.c:,Add hashlen_string()  function" Guenter Roeck <linux@roeck-us.net> - 2016-05-29 07:20 +0200
    [PATCH] Rename other copy of hash_string to hashlen_string "George Spelvin" <linux@sciencehorizons.net> - 2016-05-29 07:30 +0200
      Re: [PATCH] Rename other copy of hash_string to hashlen_string Geert Uytterhoeven <geert@linux-m68k.org> - 2016-05-29 12:50 +0200
        Re: [PATCH] Rename other copy of hash_string to hashlen_string "George Spelvin" <linux@sciencehorizons.net> - 2016-05-29 14:10 +0200
        [PATCH] fs/namei.c. If we way we want zero-length string support, mean it "George Spelvin" <linux@sciencehorizons.net> - 2016-05-29 14:20 +0200
          Re: [PATCH] fs/namei.c. If we way we want zero-length string support,  mean it Geert Uytterhoeven <geert@linux-m68k.org> - 2016-05-29 19:20 +0200
        [PATCH] fs/namei.c: Update !CONFIG_DCACHE_WORD_ACCESS case "George Spelvin" <linux@sciencehorizons.net> - 2016-05-29 14:50 +0200
    Re: Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function" "George Spelvin" <linux@sciencehorizons.net> - 2016-05-29 07:30 +0200
    Re: Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function" "George Spelvin" <linux@sciencehorizons.net> - 2016-05-29 08:00 +0200

#1408543 — Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function"

I see:

fs/dcache.c:1673: undefined reference to `hashlen_string'

For:
	total: 148 pass: 22 fail: 126

I didn't bisect, but I strongly suspect commit fcfd2fbf22d2 ("fs/namei.c:
Add hashlen_string() function") because the new function is not provided
if DCACHE_WORD_ACCESS is not enabled. This affects almost all architectures.

Reverting 468a9428521e, 2a18da7a9c78, and fcfd2fbf22d2 fixed the problem
for me in a couple of test builds, but I don't know if all three patches
need to be reverted, or if reverting those patches fixes the problem for
all builds, or if the revert is sufficient.

Guenter

[toc] | [next] | [standalone]

#1408544 — [PATCH] Rename other copy of hash_string to hashlen_string

The original name was simply hash_string(), but that conflicted with a
function with that name in drivers/base/power/trace.c, and I decided that
calling it "hashlen_" was better anyway.

But you have to do it in two places.

Signed-off-by: George Spelvin <linux@sciencehorizons.net>
Fixes: fcfd2fbf22d2587196890103d41e3d554c47da0e
---
 fs/namei.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 968dae02..aefba699 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1952,7 +1952,7 @@ unsigned int full_name_hash(const char *name, unsigned int len)
 EXPORT_SYMBOL(full_name_hash);
 
 /* Return the "hash_len" (hash and length) of a null-terminated string */
-u64 hash_string(const char *name)
+u64 hashlen_string(const char *name)
 {
 	unsigned long hash = init_name_hash();
 	unsigned long len = 0, c;
@@ -1965,7 +1965,7 @@ u64 hash_string(const char *name)
 	} while (c);
 	return hashlen_create(end_name_hash(hash), len);
 }
-EXPORT_SYMBOL(hash_string);
+EXPORT_SYMBOL(hashlen_string);
 
 /*
  * We know there's a real path component here of at least
-- 
2.8.1

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

#1408569 — Re: [PATCH] Rename other copy of hash_string to hashlen_string

Hi Georges,

On Sun, May 29, 2016 at 7:26 AM, George Spelvin
<linux@sciencehorizons.net> wrote:
> The original name was simply hash_string(), but that conflicted with a
> function with that name in drivers/base/power/trace.c, and I decided that
> calling it "hashlen_" was better anyway.
>
> But you have to do it in two places.
>
> Signed-off-by: George Spelvin <linux@sciencehorizons.net>
> Fixes: fcfd2fbf22d2587196890103d41e3d554c47da0e
> ---
>  fs/namei.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 968dae02..aefba699 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1952,7 +1952,7 @@ unsigned int full_name_hash(const char *name, unsigned int len)
>  EXPORT_SYMBOL(full_name_hash);
>
>  /* Return the "hash_len" (hash and length) of a null-terminated string */
> -u64 hash_string(const char *name)
> +u64 hashlen_string(const char *name)
>  {
>         unsigned long hash = init_name_hash();
>         unsigned long len = 0, c;
> @@ -1965,7 +1965,7 @@ u64 hash_string(const char *name)
>         } while (c);
>         return hashlen_create(end_name_hash(hash), len);
>  }
> -EXPORT_SYMBOL(hash_string);
> +EXPORT_SYMBOL(hashlen_string);

That version fails the test. On m68k/ARAnyM:

test_hash: hashlen_string(256..256) returned length 1, expected 0

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] | [prev] | [next] | [standalone]

#1408573 — Re: [PATCH] Rename other copy of hash_string to hashlen_string

> That version fails the test. On m68k/ARAnyM:
>
> test_hash: hashlen_string(256..256) returned length 1, expected 0

Just kill me. :-(

I didn't change that code, so I didn't re-test it.  But I changed
the test.

diff --git a/fs/namei.c b/fs/namei.c
index aefba699..dcb85255 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1958,11 +1958,11 @@ u64 hashlen_string(const char *name)
	unsigned long len = 0, c;
 
	c = (unsigned char)*name;
-       do {
+       while (c) {
		len++;
		hash = partial_name_hash(c, hash);
		c = (unsigned char)name[len];
-       } while (c);
+       }
	return hashlen_create(end_name_hash(hash), len);
 }
 EXPORT_SYMBOL(hashlen_string);

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

#1408574 — [PATCH] fs/namei.c. If we way we want zero-length string support, mean it

From 4b237fc0c5addf58ef0e0bbd3740d64007c36375 Mon Sep 17 00:00:00 2001
From: George Spelvin <linux@sciencehorizons.net>
Date: Sun, 29 May 2016 08:05:56 -0400
Subject: [PATCH] fs/namei.c. If we way we want zero-length string suppor mean it.

hash_name() is not changed for speed, but the more general
purpose hashlen_string need to support zero-length strings.

Signed-off-by: George Spelvin <linux@sciencehorizons.net>
Fixes: fcfd2fbf22d2587196890103d41e3d554c47da0e
---
Just kill me.  I didn't re-test the side of the code that I "didn't touch",
but at Bruce Field's (correct!) prompting, I changed the test to cover
a corner case it hadn't addressed.

 fs/namei.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index aefba699..dcb85255 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1958,11 +1958,11 @@ u64 hashlen_string(const char *name)
 	unsigned long len = 0, c;
 
 	c = (unsigned char)*name;
-	do {
+	while (c) {
 		len++;
 		hash = partial_name_hash(c, hash);
 		c = (unsigned char)name[len];
-	} while (c);
+	}
 	return hashlen_create(end_name_hash(hash), len);
 }
 EXPORT_SYMBOL(hashlen_string);
-- 
2.8.1

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

#1408618 — Re: [PATCH] fs/namei.c. If we way we want zero-length string support, mean it

Hi George,

On Sun, May 29, 2016 at 2:12 PM, George Spelvin
<linux@sciencehorizons.net> wrote:
> From 4b237fc0c5addf58ef0e0bbd3740d64007c36375 Mon Sep 17 00:00:00 2001
> From: George Spelvin <linux@sciencehorizons.net>
> Date: Sun, 29 May 2016 08:05:56 -0400
> Subject: [PATCH] fs/namei.c. If we way we want zero-length string suppor mean it.
>
> hash_name() is not changed for speed, but the more general
> purpose hashlen_string need to support zero-length strings.
>
> Signed-off-by: George Spelvin <linux@sciencehorizons.net>
> Fixes: fcfd2fbf22d2587196890103d41e3d554c47da0e

FTR:
Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

test_hash: __hash_32() has no arch implementation to test.
test_hash: hash_32() has no arch implementation to test.
test_hash: hash_64() has no arch implementation to test.
test_hash: 33152 tests passed.

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] | [prev] | [next] | [standalone]

#1408575 — [PATCH] fs/namei.c: Update !CONFIG_DCACHE_WORD_ACCESS case

From be5e653c0d5ec890c32a48851dcd0a347df8e5fa Mon Sep 17 00:00:00 2001
From: George Spelvin <linux@sciencehorizons.net>
Date: Sun, 29 May 2016 01:23:14 -0400
Subject: [PATCH] fs/namei.c: Update !CONFIG_DCACHE_WORD_ACCESS case

A couple of late changes to the previous patch didn't update the
!CONFIG_DCACHE_WORD_ACCESS option correspondingly.  It hadn't
changed, so didn't need re-testing, right?

Yeah.  Let that be a lesson to me.

Specifically the caller got changed in two ways:
- The function name was changed from v1 to avoid a collision.
- The self-test was updated to cover zero-length strings; the function
  needs to be updated, too.

Signed-off-by: George Spelvin <linux@sciencehorizons.net>
Fixes: fcfd2fbf22d2587196890103d41e3d554c47da0e
---
Here's the combined patch, if that's preferred.  Obviously Correct(tm),
but I'm starting the full test quite right now.

That just takes a few hours.  (I have to reboot two machines multiple
times with 32- and 64-bit kernels and and test NFS between them.)

 fs/namei.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 968dae02..dcb85255 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1952,20 +1952,20 @@ unsigned int full_name_hash(const char *name, unsigned int len)
 EXPORT_SYMBOL(full_name_hash);
 
 /* Return the "hash_len" (hash and length) of a null-terminated string */
-u64 hash_string(const char *name)
+u64 hashlen_string(const char *name)
 {
 	unsigned long hash = init_name_hash();
 	unsigned long len = 0, c;
 
 	c = (unsigned char)*name;
-	do {
+	while (c) {
 		len++;
 		hash = partial_name_hash(c, hash);
 		c = (unsigned char)name[len];
-	} while (c);
+	}
 	return hashlen_create(end_name_hash(hash), len);
 }
-EXPORT_SYMBOL(hash_string);
+EXPORT_SYMBOL(hashlen_string);
 
 /*
  * We know there's a real path component here of at least
-- 
2.8.1

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

#1408545 — Re: Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function"

> Reverting 468a9428521e, 2a18da7a9c78, and fcfd2fbf22d2 fixed the problem
> for me in a couple of test builds, but I don't know if all three patches
> need to be reverted, or if reverting those patches fixes the problem for
> all builds, or if the revert is sufficient.

Oh, fuck me so fucking hard.

the function is there, just when I renamed it from "hash_string"
I only did one copy.

Find hash_string() and rename it (and the EXPORT).

Patch coming ASAP.

Sorry.

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

#1408546 — Re: Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function"

Thank you or the prompt report which saved me the greater embarrassment
of seeing the bug in -rc1.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web