Re: [PATCH 0/6] improve list_sort test

don.mullis
2010-08-21T16:59:09Z

Artem Bityutskiy <dedekind1@gmail.com> writes:
> Any feedback?
>
> I'm also going to add this patch to the series:
>
> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Subject: [PATCH 1/7] lib/list_sort: improve comments
>
> Document the fact that 'list_sort()' can call the 'cmp()' function with
> 'a' == 'b' for the sake of 'cond_reshed()'.
>
> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> ---
> lib/list_sort.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/lib/list_sort.c b/lib/list_sort.c
> index 4b5cb79..d134b41 100644
> --- a/lib/list_sort.c
> +++ b/lib/list_sort.c
> @@ -93,6 +93,11 @@ static void merge_and_restore_back_links(void *priv,
> * should sort before @b, and a positive value if @a should sort after
> * @b. If @a and @b are equivalent, and their original relative
> * ordering is to be preserved, @cmp must return 0.
> + *
> + * This function can be used in atomic context. This means that @cmp has to
> + * take care of calling 'cond_resched()' when needed. And 'list_sort()' will
> + * sometimes call @cmp with @a equivalent to @b, just to let the user call
> + * 'cond_resched()'.
> */
> void list_sort(void *priv, struct list_head *head,
> int (*cmp)(void *priv, struct list_head *a,
> --

Thanks, Artem. I've prepared and tested locally a patch series that
brings in most of the series you sent earlier, and adds a few more
cleanups. The one patch of yours that I did replace was the last one,
that modifies semantics of the correctness test. My reasoning is that
the value of testing lies in exposing bugs, as economically as possible,
and your test wasn't exposing my power-of-two bug. So I created an
alternative that's as simple as possible while picking at the corner
cases, e.g. power-of-two.

Last in my local patch series is the bug fix itself. Setting
CONFIG_TEST_LIST_SORT and testing with all but the last patch applied
produces failure messages on the boot console. Pushing the final "fix"
patch makes them go away :-)

Okay if I incorporate your comment addition, above, and post the series
for review?

Don