On 12/23/2017 10:33 PM, Tetsuo Handa wrote:
+ bitmap = rcu_dereference_raw(*slot);
+ if (!bitmap) {
+ bitmap = this_cpu_xchg(ida_bitmap, NULL);
+ if (!bitmap)
+ return -ENOMEM;
I can't understand this. I can understand if it were
BUG_ON(!bitmap);
because you called xb_preload().
But
/*
* Regular test 2
* set bit 2000, 2001, 2040
* Next 1 in [0, 2048) --> 2000
* Next 1 in [2000, 2002) --> 2000
* Next 1 in [2002, 2041) --> 2040
* Next 1 in [2002, 2040) --> none
* Next 0 in [2000, 2048) --> 2002
* Next 0 in [2048, 2060) --> 2048
*/
xb_preload(GFP_KERNEL);
assert(!xb_set_bit(&xb1, 2000));
assert(!xb_set_bit(&xb1, 2001));
assert(!xb_set_bit(&xb1, 2040));
[...]
xb_preload_end();
you are not calling xb_preload() prior to each xb_set_bit() call.
This means that, if each xb_set_bit() is not surrounded with
xb_preload()/xb_preload_end(), there is possibility of hitting
this_cpu_xchg(ida_bitmap, NULL) == NULL.
This is just a lazy test. We "know" that the bits in the range 1024-2047
will all land in the same bitmap, so there's no need to preload for each
of them.
Testcases also serves as how to use that API.
Assuming such thing leads to incorrect usage.
If callers are aware that the bits that they going to record locate in
the same bitmap, I think they should also perform the xb_ APIs with only
one preload. So the test cases here have shown them a correct example.
We can probably add some comments above to explain this.
Best,
Wei
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
