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

Groups > linux.kernel > #1274292 > unrolled thread

[PATCH v2 0/4] improve fault-tolerance of rhashtable runtime-test

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/4] improve fault-tolerance of rhashtable runtime-test Phil Sutter <phil@nwl.cc> - 2015-11-20 18:20 +0100
    [PATCH v2 1/4] rhashtable-test: add cond_resched() to thread test Phil Sutter <phil@nwl.cc> - 2015-11-20 18:20 +0100
    Re: [PATCH v2 0/4] improve fault-tolerance of rhashtable  runtime-test David Miller <davem@davemloft.net> - 2015-11-23 18:40 +0100
    Re: [PATCH v2 0/4] improve fault-tolerance of rhashtable runtime-test Herbert Xu <herbert@gondor.apana.org.au> - 2015-11-30 10:40 +0100
      Re: [PATCH v2 0/4] improve fault-tolerance of rhashtable runtime-test Phil Sutter <phil@nwl.cc> - 2015-11-30 11:20 +0100
        Re: [PATCH v2 0/4] improve fault-tolerance of rhashtable runtime-test Herbert Xu <herbert@gondor.apana.org.au> - 2015-11-30 11:20 +0100

#1274292 — [PATCH v2 0/4] improve fault-tolerance of rhashtable runtime-test

The following series aims to improve lib/test_rhashtable in different
situations:

Patch 1 allows the kernel to reschedule so the test does not block too
        long on slow systems.
Patch 2 fixes behaviour under pressure, retrying inserts in non-permanent
        error case (-EBUSY).
Patch 3 auto-adjusts the upper table size limit according to the number
        of threads (in concurrency test). In fact, the current default is
	already too small.
Patch 4 makes it possible to retry inserts even in supposedly permanent
        error case (-ENOMEM) to expose rhashtable's remaining problem of
	-ENOMEM being not as permanent as it is expected to be.

Changes since v1:
- Introduce insert_retry() which is then used in single-threaded test as
  well.
- Do not retry inserts by default if -ENOMEM was returned.
- Rename the retry counter to be a bit more verbose about what it
  contains.
- Add patch 4 as a debugging aid.

Phil Sutter (4):
  rhashtable-test: add cond_resched() to thread test
  rhashtable-test: retry insert operations
  rhashtable-test: calculate max_entries value by default
  rhashtable-test: allow to retry even if -ENOMEM was returned

 lib/test_rhashtable.c | 76 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 50 insertions(+), 26 deletions(-)

-- 
2.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1274293 — [PATCH v2 1/4] rhashtable-test: add cond_resched() to thread test

This should fix for soft lockup bugs triggered on slow systems.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 lib/test_rhashtable.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c
index 8c1ad1c..63654e3 100644
--- a/lib/test_rhashtable.c
+++ b/lib/test_rhashtable.c
@@ -236,6 +236,8 @@ static int thread_lookup_test(struct thread_data *tdata)
 			       obj->value, key);
 			err++;
 		}
+
+		cond_resched();
 	}
 	return err;
 }
@@ -251,6 +253,7 @@ static int threadfunc(void *data)
 
 	for (i = 0; i < entries; i++) {
 		tdata->objs[i].value = (tdata->id << 16) | i;
+		cond_resched();
 		err = rhashtable_insert_fast(&ht, &tdata->objs[i].node,
 		                             test_rht_params);
 		if (err == -ENOMEM || err == -EBUSY) {
@@ -285,6 +288,8 @@ static int threadfunc(void *data)
 				goto out;
 			}
 			tdata->objs[i].value = TEST_INSERT_FAIL;
+
+			cond_resched();
 		}
 		err = thread_lookup_test(tdata);
 		if (err) {
-- 
2.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1275660 — Re: [PATCH v2 0/4] improve fault-tolerance of rhashtable runtime-test

From: Phil Sutter <phil@nwl.cc>
Date: Fri, 20 Nov 2015 18:17:16 +0100

> The following series aims to improve lib/test_rhashtable in different
> situations:
> 
> Patch 1 allows the kernel to reschedule so the test does not block too
>         long on slow systems.
> Patch 2 fixes behaviour under pressure, retrying inserts in non-permanent
>         error case (-EBUSY).
> Patch 3 auto-adjusts the upper table size limit according to the number
>         of threads (in concurrency test). In fact, the current default is
> 	already too small.
> Patch 4 makes it possible to retry inserts even in supposedly permanent
>         error case (-ENOMEM) to expose rhashtable's remaining problem of
> 	-ENOMEM being not as permanent as it is expected to be.
> 
> Changes since v1:
> - Introduce insert_retry() which is then used in single-threaded test as
>   well.
> - Do not retry inserts by default if -ENOMEM was returned.
> - Rename the retry counter to be a bit more verbose about what it
>   contains.
> - Add patch 4 as a debugging aid.

Series applied, thanks Phil.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1279733

Phil Sutter <phil@nwl.cc> wrote:
> The following series aims to improve lib/test_rhashtable in different
> situations:
> 
> Patch 1 allows the kernel to reschedule so the test does not block too
>        long on slow systems.
> Patch 2 fixes behaviour under pressure, retrying inserts in non-permanent
>        error case (-EBUSY).
> Patch 3 auto-adjusts the upper table size limit according to the number
>        of threads (in concurrency test). In fact, the current default is
>        already too small.
> Patch 4 makes it possible to retry inserts even in supposedly permanent
>        error case (-ENOMEM) to expose rhashtable's remaining problem of
>        -ENOMEM being not as permanent as it is expected to be.

I'm sorry but this patch series is simply bogus.

If rhashtable is indeed returning such errors under normal
conditions then rhashtable is broken and we must fix it instead
of working around it in the test code!

FWIW I still haven't been able to reproduce this problem, perhaps
because my machines have too few CPUs?

So can someone please help me reproduce this? Because just loading
test_rhashtable isn't doing it.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1279745

On Mon, Nov 30, 2015 at 05:37:55PM +0800, Herbert Xu wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > The following series aims to improve lib/test_rhashtable in different
> > situations:
> > 
> > Patch 1 allows the kernel to reschedule so the test does not block too
> >        long on slow systems.
> > Patch 2 fixes behaviour under pressure, retrying inserts in non-permanent
> >        error case (-EBUSY).
> > Patch 3 auto-adjusts the upper table size limit according to the number
> >        of threads (in concurrency test). In fact, the current default is
> >        already too small.
> > Patch 4 makes it possible to retry inserts even in supposedly permanent
> >        error case (-ENOMEM) to expose rhashtable's remaining problem of
> >        -ENOMEM being not as permanent as it is expected to be.
> 
> I'm sorry but this patch series is simply bogus.

The whole series?!

> If rhashtable is indeed returning such errors under normal
> conditions then rhashtable is broken and we must fix it instead
> of working around it in the test code!

You're stating the obvious. Remember, the reason I prepared patch 4 was
because you wanted to fix just that bug in rhashtable in the first
place.

Just to make this clear: Patches 1-3 are reasonable on their own, the
only connection to the bug is that patch 2 makes it visible (at least on
my system it wasn't before).

> FWIW I still haven't been able to reproduce this problem, perhaps
> because my machines have too few CPUs?

Did you try with my bogus patch series applied? How many CPUs does your
test system actually have?

> So can someone please help me reproduce this? Because just loading
> test_rhashtable isn't doing it.

As said, maybe you need to increase the number of spawned threads
(tcount=50 or so).

Cheers, Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1279747

On Mon, Nov 30, 2015 at 11:14:01AM +0100, Phil Sutter wrote:
> On Mon, Nov 30, 2015 at 05:37:55PM +0800, Herbert Xu wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > > The following series aims to improve lib/test_rhashtable in different
> > > situations:
> > > 
> > > Patch 1 allows the kernel to reschedule so the test does not block too
> > >        long on slow systems.
> > > Patch 2 fixes behaviour under pressure, retrying inserts in non-permanent
> > >        error case (-EBUSY).
> > > Patch 3 auto-adjusts the upper table size limit according to the number
> > >        of threads (in concurrency test). In fact, the current default is
> > >        already too small.
> > > Patch 4 makes it possible to retry inserts even in supposedly permanent
> > >        error case (-ENOMEM) to expose rhashtable's remaining problem of
> > >        -ENOMEM being not as permanent as it is expected to be.
> > 
> > I'm sorry but this patch series is simply bogus.
> 
> The whole series?!

Well at least patch two and four seem clearly wrong because no
rhashtable user should need to retry insertions.

> Did you try with my bogus patch series applied? How many CPUs does your
> test system actually have?
> 
> > So can someone please help me reproduce this? Because just loading
> > test_rhashtable isn't doing it.
> 
> As said, maybe you need to increase the number of spawned threads
> (tcount=50 or so).

OK that's better.  I think I see the problem.  The test in
rhashtable_insert_rehash is racy and if two threads both try
to grow the table one of them may be tricked into doing a rehash
instead.

I'm working on a fix.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web