linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function"
@ 2016-05-29  5:18 Guenter Roeck
  2016-05-29  5:22 ` George Spelvin
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Guenter Roeck @ 2016-05-29  5:18 UTC (permalink / raw)
  To: linux-kernel, Linus Torvalds, George Spelvin

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function"
  2016-05-29  5:18 Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function" Guenter Roeck
@ 2016-05-29  5:22 ` George Spelvin
  2016-05-29  5:26 ` [PATCH] Rename other copy of hash_string to hashlen_string George Spelvin
  2016-05-29  5:50 ` Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function" George Spelvin
  2 siblings, 0 replies; 9+ messages in thread
From: George Spelvin @ 2016-05-29  5:22 UTC (permalink / raw)
  To: linux-kernel, linux, linux, torvalds

> 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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] Rename other copy of hash_string to hashlen_string
  2016-05-29  5:18 Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function" Guenter Roeck
  2016-05-29  5:22 ` George Spelvin
@ 2016-05-29  5:26 ` George Spelvin
  2016-05-29 10:48   ` Geert Uytterhoeven
  2016-05-29  5:50 ` Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function" George Spelvin
  2 siblings, 1 reply; 9+ messages in thread
From: George Spelvin @ 2016-05-29  5:26 UTC (permalink / raw)
  To: linux-kernel, linux, linux, torvalds

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function"
  2016-05-29  5:18 Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function" Guenter Roeck
  2016-05-29  5:22 ` George Spelvin
  2016-05-29  5:26 ` [PATCH] Rename other copy of hash_string to hashlen_string George Spelvin
@ 2016-05-29  5:50 ` George Spelvin
  2 siblings, 0 replies; 9+ messages in thread
From: George Spelvin @ 2016-05-29  5:50 UTC (permalink / raw)
  To: linux-kernel, linux, linux

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Rename other copy of hash_string to hashlen_string
  2016-05-29  5:26 ` [PATCH] Rename other copy of hash_string to hashlen_string George Spelvin
@ 2016-05-29 10:48   ` Geert Uytterhoeven
  2016-05-29 12:02     ` George Spelvin
                       ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2016-05-29 10:48 UTC (permalink / raw)
  To: George Spelvin; +Cc: linux-kernel, Guenter Roeck, Linus Torvalds

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Rename other copy of hash_string to hashlen_string
  2016-05-29 10:48   ` Geert Uytterhoeven
@ 2016-05-29 12:02     ` George Spelvin
  2016-05-29 12:12     ` [PATCH] fs/namei.c. If we way we want zero-length string support, mean it George Spelvin
  2016-05-29 12:41     ` [PATCH] fs/namei.c: Update !CONFIG_DCACHE_WORD_ACCESS case George Spelvin
  2 siblings, 0 replies; 9+ messages in thread
From: George Spelvin @ 2016-05-29 12:02 UTC (permalink / raw)
  To: geert, linux; +Cc: linux-kernel, linux, torvalds

> 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);

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] fs/namei.c. If we way we want zero-length string support, mean it
  2016-05-29 10:48   ` Geert Uytterhoeven
  2016-05-29 12:02     ` George Spelvin
@ 2016-05-29 12:12     ` George Spelvin
  2016-05-29 17:13       ` Geert Uytterhoeven
  2016-05-29 12:41     ` [PATCH] fs/namei.c: Update !CONFIG_DCACHE_WORD_ACCESS case George Spelvin
  2 siblings, 1 reply; 9+ messages in thread
From: George Spelvin @ 2016-05-29 12:12 UTC (permalink / raw)
  To: geert, linux; +Cc: linux-kernel, linux, torvalds

>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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] fs/namei.c: Update !CONFIG_DCACHE_WORD_ACCESS case
  2016-05-29 10:48   ` Geert Uytterhoeven
  2016-05-29 12:02     ` George Spelvin
  2016-05-29 12:12     ` [PATCH] fs/namei.c. If we way we want zero-length string support, mean it George Spelvin
@ 2016-05-29 12:41     ` George Spelvin
  2 siblings, 0 replies; 9+ messages in thread
From: George Spelvin @ 2016-05-29 12:41 UTC (permalink / raw)
  To: geert, linux; +Cc: linux-kernel, linux, torvalds

>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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] fs/namei.c. If we way we want zero-length string support, mean it
  2016-05-29 12:12     ` [PATCH] fs/namei.c. If we way we want zero-length string support, mean it George Spelvin
@ 2016-05-29 17:13       ` Geert Uytterhoeven
  0 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2016-05-29 17:13 UTC (permalink / raw)
  To: George Spelvin; +Cc: linux-kernel, Guenter Roeck, Linus Torvalds

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-05-29 17:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-29  5:18 Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function" Guenter Roeck
2016-05-29  5:22 ` George Spelvin
2016-05-29  5:26 ` [PATCH] Rename other copy of hash_string to hashlen_string George Spelvin
2016-05-29 10:48   ` Geert Uytterhoeven
2016-05-29 12:02     ` George Spelvin
2016-05-29 12:12     ` [PATCH] fs/namei.c. If we way we want zero-length string support, mean it George Spelvin
2016-05-29 17:13       ` Geert Uytterhoeven
2016-05-29 12:41     ` [PATCH] fs/namei.c: Update !CONFIG_DCACHE_WORD_ACCESS case George Spelvin
2016-05-29  5:50 ` Build errors in mainline due to "fs/namei.c:,Add hashlen_string() function" George Spelvin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).