Re: [PATCH v20 0/7] Virtio-balloon Enhancement

[Wei Wang <wei.w.wang@intel.com>]

On 12/21/2017 01:10 AM, Matthew Wilcox wrote:
> On Wed, Dec 20, 2017 at 04:13:16PM +0000, Wang, Wei W wrote:
> > On Wednesday, December 20, 2017 8:26 PM, Matthew Wilcox wrote:
> > > 	unsigned long bit;
> > > 	xb_preload(GFP_KERNEL);
> > > 	xb_set_bit(xb, 700);
> > > 	xb_preload_end();
> > > 	bit = xb_find_set(xb, ULONG_MAX, 0);
> > > 	assert(bit == 700);
> > This above test will result in "!node with bitmap !=NULL", and it goes to the regular "if (bitmap)" path, which finds 700.
> >
> > A better test would be
> > ...
> > xb_set_bit(xb, 700);
> > assert(xb_find_set(xb, ULONG_MAX, 800) == ULONG_MAX);
> > ...
> I decided to write a test case to show you what I meant, then I discovered
> the test suite didn't build, then the test I wrote took forever to run, so
> I rewrote xb_find_set() using the radix tree iterators.  So I have no idea
> what bugs may be in your implementation, but at least this function passes
> the current test suite.  Of course, there may be gaps in the test suite.
> And since I changed the API to not have the ambiguous return value, I
> also changed the test suite, and maybe I introduced a bug.

Thanks for the effort. That's actually caused by the previous "!node" 
path, which incorrectly changed "index = (index | RADIX_TREE_MAP_MASK) + 
1". With the change below, it will run pretty well with the test cases.

if (!node && !bitmap)
    return size;

Would you mind to have a try with the v20 RESEND patch that was just 
shared? It makes the above change and added the test case you suggested?

One more question is about the return value, why would it be ambiguous? 
I think it is the same as find_next_bit() which returns the found bit or 
size if not found.


Best,
Wei