All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Yury Norov <yury.norov@gmail.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] bitmap: Add test for bitmap_cut()
Date: Mon, 15 Jun 2020 13:08:25 +0200	[thread overview]
Message-ID: <20200615130825.60283f1b@redhat.com> (raw)
In-Reply-To: <20200615094616.GS2428291@smile.fi.intel.com>

On Mon, 15 Jun 2020 12:46:16 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Mon, Jun 15, 2020 at 12:41:55PM +0300, Andy Shevchenko wrote:
> > On Sun, Jun 14, 2020 at 07:40:54PM +0200, Stefano Brivio wrote:  
> > > Inspired by an original patch from Yury Norov: introduce a test for
> > > bitmap_cut() that also makes sure functionality is as described for
> > > partially overlapping src and dst.  
> > 
> > Taking into account recent fixes for BE 64-bit, do we have test cases for a such?  
> 
> It might be enough to have only these, but perhaps s390 guys can help?

There's no behaviour difference due to endianness in this test itself --
just word size was a topic, hence that BITS_PER_LONG usage with
redundant values (checked on i686).

That is, if you have:
	{ 0x0000ffffUL, 0x5a5a5a5aUL, 0x5a5a5a5aUL, 0x5a5a5a5aUL },

then 1 as array subscript always denotes the second item (from the left)
there, it doesn't matter how and where different architectures store it.

Indeed, if bitmap_cut() directly addressed single bytes within the
words, I would need to pay special attention there. The values I picked
for these tests are also meant to show any issue in that sense.

> Alexander, can you apply this patch (w/o the first one, which is suppose to
> fix) and confirm that you have test case failure, followed by applying first
> one and confirm a fix?

I did that already on s390x (of course, I thought :)), I can confirm
that. Without patch 1/2 the test also fails there:

[   20.917848] test_bitmap: [lib/test_bitmap.c:666] bitmaps contents differ: expected "0-16,18-19,21,24,26-27,29", got "1,3-4,6,9,11-12,14,16,18-19,21,24,26-27,29"

If Alexander wants to test this on a z14 or z15, sure, it won't harm.

By the way, tests for 'parse', 'parse_user' and 'parselist' report
issues:

[   20.390401] test_bitmap: loaded.
[   20.394839] test_bitmap: parse: 4: input is 1, result is 0x100000000, expected 0x1
[   20.395011] test_bitmap: parse: 5: input is deadbeef, result is 0xdeadbeef00000000, expected 0xdeadbeef
[   20.395059] test_bitmap: parse: 6: input is 1,0, result is 0x1, expected 0x100000000
[   20.395099] test_bitmap: parse: 7: input is deadbeef,
               ,0,1, result is 0x1, expected 0xdeadbeef
[   20.396696] test_bitmap: parse: 8: input is deadbeef,1,0, result is 0x1, expected 0x100000000
[   20.396735] test_bitmap: parse: 9: input is baadf00d,deadbeef,1,0, result is 0x1, expected 0x100000000
[   20.396835] test_bitmap: parse: 10: input is badf00d,deadbeef,1,0, errno is -75, expected 0
[   20.396878] test_bitmap: parse: 11: input is badf00d,deadbeef,1,0, errno is -75, expected 0
[   20.396913] test_bitmap: parse: 12: input is   badf00d,deadbeef,1,0  , errno is -75, expected 0
[   20.396957] test_bitmap: parse: 13: input is  , badf00d,deadbeef,1,0 , , errno is -75, expected 0
[   20.396983] test_bitmap: parse: 14: input is  , badf00d, ,, ,,deadbeef,1,0 , , errno is -75, expected 0
[   20.397052] test_bitmap: parse: 16: input is 3,0, errno is 0, expected -75
[   20.397712] test_bitmap: parse_user: 4: input is 1, result is 0x100000000, expected 0x1
[   20.397832] test_bitmap: parse_user: 5: input is deadbeef, result is 0xdeadbeef00000000, expected 0xdeadbeef
[   20.397928] test_bitmap: parse_user: 6: input is 1,0, result is 0x1, expected 0x100000000
[   20.398022] test_bitmap: parse_user: 7: input is deadbeef,
               ,0,1, result is 0x1, expected 0xdeadbeef
[   20.398116] test_bitmap: parse_user: 8: input is deadbeef,1,0, result is 0x1, expected 0x100000000
[   20.398209] test_bitmap: parse_user: 9: input is baadf00d,deadbeef,1,0, result is 0x1, expected 0x100000000
[   20.398301] test_bitmap: parse_user: 10: input is badf00d,deadbeef,1,0, errno is -75, expected 0
[   20.398393] test_bitmap: parse_user: 11: input is badf00d,deadbeef,1,0, errno is -75, expected 0
[   20.398484] test_bitmap: parse_user: 12: input is   badf00d,deadbeef,1,0  , errno is -75, expected 0
[   20.398574] test_bitmap: parse_user: 13: input is  , badf00d,deadbeef,1,0 , , errno is -75, expected 0
[   20.398666] test_bitmap: parse_user: 14: input is  , badf00d, ,, ,,deadbeef,1,0 , , errno is -75, expected 0
[   20.398794] test_bitmap: parse_user: 16: input is 3,0, errno is 0, expected -75
[   20.399906] test_bitmap: parselist: 14: input is '0-2047:128/256' OK, Time: 2641
[   20.400914] test_bitmap: parselist_user: 14: input is '0-2047:128/256' OK, Time: 19961
[   20.421406] test_bitmap: all 1679 tests passed

and at a glance those *seem* to be bugs in the tests themselves, not in
the actual functions they test. Sure, they should be fixed, but I can't
take care of that right now.

-- 
Stefano


  reply	other threads:[~2020-06-15 11:08 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-14 17:40 [PATCH 0/2] lib: Fix bitmap_cut() for overlaps, add test Stefano Brivio
2020-06-14 17:40 ` [PATCH 1/2] bitmap: Fix bitmap_cut() for partial overlapping case Stefano Brivio
2020-06-15  9:42   ` Andy Shevchenko
2020-06-14 17:40 ` [PATCH 2/2] bitmap: Add test for bitmap_cut() Stefano Brivio
2020-06-15  9:41   ` Andy Shevchenko
2020-06-15  9:46     ` Andy Shevchenko
2020-06-15 11:08       ` Stefano Brivio [this message]
2020-06-15 11:43         ` Andy Shevchenko
2020-06-15 13:16           ` Stefano Brivio
2020-06-15 12:04       ` Alexander Gordeev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200615130825.60283f1b@redhat.com \
    --to=sbrivio@redhat.com \
    --cc=agordeev@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=pablo@netfilter.org \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.