concurrent rhashtable test failure

concurrent rhashtable test failure

[Geert Uytterhoeven]

Hi Phil,

On m68k/ARAnyM, test_rhashtable fails with:

    Test failed: thread 0 returned: -4

(-4 = -EINTR)

I traced this back to your commit f4a3e90ba5739cfd ("rhashtable-test: extend
to test concurrency"), which added that part of the test.

Diff of the test output between the failing commit and its parent:

 Running rhashtable test nelem=8, max_size=65536, shrinking=0
 Test 00:
   Adding 50000 keys
   Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
   Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
   Deleting 50000 keys
-  Duration of test: 1029960000 ns
+  Duration of test: 990000000 ns
 Test 01:
   Adding 50000 keys
   Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
   Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
   Deleting 50000 keys
-  Duration of test: 990000000 ns
+  Duration of test: 720000000 ns
 Test 02:
   Adding 50000 keys
   Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
   Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
   Deleting 50000 keys
-  Duration of test: 1130000000 ns
+  Duration of test: 700000000 ns
 Test 03:
   Adding 50000 keys
   Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
   Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
   Deleting 50000 keys
-  Duration of test: 1080000000 ns
-Average test time: 1057490000
+  Duration of test: 700000000 ns
+Average test time: 777500000
+Testing concurrent rhashtable access from 10 threads
+Test failed: thread 0 returned: -4
+Started 10 threads, 1 failed

Do you have any clue?

Thanks!

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

Re: concurrent rhashtable test failure

[Phil Sutter]

Hi Geert,

On Mon, Oct 24, 2016 at 02:11:32PM +0200, Geert Uytterhoeven wrote:
> On m68k/ARAnyM, test_rhashtable fails with:
> 
>     Test failed: thread 0 returned: -4
> 
> (-4 = -EINTR)

How reproducible is this? I wonder why out of the ten threads only the
first one fails.

> I traced this back to your commit f4a3e90ba5739cfd ("rhashtable-test: extend
> to test concurrency"), which added that part of the test.
> 
> Diff of the test output between the failing commit and its parent:
> 
>  Running rhashtable test nelem=8, max_size=65536, shrinking=0
>  Test 00:
>    Adding 50000 keys
>    Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
>    Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
>    Deleting 50000 keys
> -  Duration of test: 1029960000 ns
> +  Duration of test: 990000000 ns
>  Test 01:
>    Adding 50000 keys
>    Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
>    Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
>    Deleting 50000 keys
> -  Duration of test: 990000000 ns
> +  Duration of test: 720000000 ns
>  Test 02:
>    Adding 50000 keys
>    Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
>    Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
>    Deleting 50000 keys
> -  Duration of test: 1130000000 ns
> +  Duration of test: 700000000 ns
>  Test 03:
>    Adding 50000 keys
>    Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
>    Traversal complete: counted=50000, nelems=50000, entries=50000, table-jumps=0
>    Deleting 50000 keys
> -  Duration of test: 1080000000 ns
> -Average test time: 1057490000
> +  Duration of test: 700000000 ns
> +Average test time: 777500000
> +Testing concurrent rhashtable access from 10 threads
> +Test failed: thread 0 returned: -4
> +Started 10 threads, 1 failed
> 
> Do you have any clue?

Not really, I merely implemented the test.

Thanks, Phil

Re: concurrent rhashtable test failure

[Geert Uytterhoeven]

On Wed, Oct 26, 2016 at 9:33 AM, Phil Sutter <phil@nwl.cc> wrote:
> On Mon, Oct 24, 2016 at 02:11:32PM +0200, Geert Uytterhoeven wrote:
>> On m68k/ARAnyM, test_rhashtable fails with:
>>
>>     Test failed: thread 0 returned: -4
>>
>> (-4 = -EINTR)
>
> How reproducible is this? I wonder why out of the ten threads only the
> first one fails.

100% reproducible.

Does it need SMP?

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

Re: concurrent rhashtable test failure

[Thomas Graf]

On 10/24/16 at 02:11pm, Geert Uytterhoeven wrote:
> Hi Phil,
> 
> On m68k/ARAnyM, test_rhashtable fails with:
> 
>     Test failed: thread 0 returned: -4
> 
> (-4 = -EINTR)

The error is returned by kthread_stop(), I suspect we are running into
this:

static int kthread(void *_create)
{
	[...]
        complete(done);
        schedule();

        ret = -EINTR;

        if (!test_bit(KTHREAD_SHOULD_STOP, &self.flags)) {
                __kthread_parkme(&self);
                ret = threadfn(data);
        }
        /* we can't just return, we must preserve "self" on stack */
        do_exit(ret);
}

Re: concurrent rhashtable test failure

[Geert Uytterhoeven]

Hi Thomas,

On Wed, Oct 26, 2016 at 11:51 AM, Thomas Graf <tgraf@suug.ch> wrote:
> On 10/24/16 at 02:11pm, Geert Uytterhoeven wrote:
>> On m68k/ARAnyM, test_rhashtable fails with:
>>
>>     Test failed: thread 0 returned: -4
>>
>> (-4 = -EINTR)
>
> The error is returned by kthread_stop(), I suspect we are running into
> this:
>
> static int kthread(void *_create)
> {
>         [...]
>         complete(done);
>         schedule();
>
>         ret = -EINTR;
>
>         if (!test_bit(KTHREAD_SHOULD_STOP, &self.flags)) {
>                 __kthread_parkme(&self);
>                 ret = threadfn(data);
>         }
>         /* we can't just return, we must preserve "self" on stack */
>         do_exit(ret);
> }

Looks reasonable. Adding a small delay like in the (whitespace-damaged)
patch below fixes the issue for me.

However, shouldn't the prestart_sem take care of making sure that
all threads have been started?

--- a/lib/test_rhashtable.c
+++ b/lib/test_rhashtable.c
@@ -13,6 +13,7 @@
  * Self Test
  **************************************************************************/

+#include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/jhash.h>
 #include <linux/kernel.h>
@@ -403,6 +404,7 @@ static int __init test_rht_init(void)
                pr_err("  down interruptible failed\n");
        for (i = 0; i < tcount; i++)
                up(&startup_sem);
+       msleep(1000);
        for (i = 0; i < tcount; i++) {
                if (IS_ERR(tdata[i].task))
                        continue;

Gr{oetje,eeting}s,

                        Geert