* [PATCH 1/2] powerpc/msi: Fix the msi bitmap alignment tests
@ 2014-10-10 8:04 Michael Ellerman
2014-10-10 8:04 ` [PATCH 2/2] powerpc/msi: Use WARN_ON() in msi bitmap selftests Michael Ellerman
0 siblings, 1 reply; 3+ messages in thread
From: Michael Ellerman @ 2014-10-10 8:04 UTC (permalink / raw)
To: linuxppc-dev
When we added the alignment tests recently we failed to check they were
actually passing - oops.
They weren't passing, because the bitmap was full. We should also be a
bit more careful when checking the return code, a negative error return
could by divisible by our alignment value.
Fixes: b0345bbc6d09 ("powerpc/msi: Improve IRQ bitmap allocator")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/sysdev/msi_bitmap.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/sysdev/msi_bitmap.c b/arch/powerpc/sysdev/msi_bitmap.c
index 0c75214b6f92..8155d93dee1d 100644
--- a/arch/powerpc/sysdev/msi_bitmap.c
+++ b/arch/powerpc/sysdev/msi_bitmap.c
@@ -151,7 +151,7 @@ void msi_bitmap_free(struct msi_bitmap *bmp)
static void __init test_basics(void)
{
struct msi_bitmap bmp;
- int i, size = 512;
+ int rc, i, size = 512;
/* Can't allocate a bitmap of 0 irqs */
check(msi_bitmap_alloc(&bmp, 0, NULL) != 0);
@@ -185,14 +185,24 @@ static void __init test_basics(void)
msi_bitmap_free_hwirqs(&bmp, size / 2, 1);
check(msi_bitmap_alloc_hwirqs(&bmp, 1) == size / 2);
+ /* Free most of them for the alignment tests */
+ msi_bitmap_free_hwirqs(&bmp, 3, size - 3);
+
/* Check we get a naturally aligned offset */
- check(msi_bitmap_alloc_hwirqs(&bmp, 2) % 2 == 0);
- check(msi_bitmap_alloc_hwirqs(&bmp, 4) % 4 == 0);
- check(msi_bitmap_alloc_hwirqs(&bmp, 8) % 8 == 0);
- check(msi_bitmap_alloc_hwirqs(&bmp, 9) % 16 == 0);
- check(msi_bitmap_alloc_hwirqs(&bmp, 3) % 4 == 0);
- check(msi_bitmap_alloc_hwirqs(&bmp, 7) % 8 == 0);
- check(msi_bitmap_alloc_hwirqs(&bmp, 121) % 128 == 0);
+ rc = msi_bitmap_alloc_hwirqs(&bmp, 2);
+ check(rc >= 0 && rc % 2 == 0);
+ rc = msi_bitmap_alloc_hwirqs(&bmp, 4);
+ check(rc >= 0 && rc % 4 == 0);
+ rc = msi_bitmap_alloc_hwirqs(&bmp, 8);
+ check(rc >= 0 && rc % 8 == 0);
+ rc = msi_bitmap_alloc_hwirqs(&bmp, 9);
+ check(rc >= 0 && rc % 16 == 0);
+ rc = msi_bitmap_alloc_hwirqs(&bmp, 3);
+ check(rc >= 0 && rc % 4 == 0);
+ rc = msi_bitmap_alloc_hwirqs(&bmp, 7);
+ check(rc >= 0 && rc % 8 == 0);
+ rc = msi_bitmap_alloc_hwirqs(&bmp, 121);
+ check(rc >= 0 && rc % 128 == 0);
msi_bitmap_free(&bmp);
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] powerpc/msi: Use WARN_ON() in msi bitmap selftests
2014-10-10 8:04 [PATCH 1/2] powerpc/msi: Fix the msi bitmap alignment tests Michael Ellerman
@ 2014-10-10 8:04 ` Michael Ellerman
2014-10-10 8:11 ` Laurentiu Tudor
0 siblings, 1 reply; 3+ messages in thread
From: Michael Ellerman @ 2014-10-10 8:04 UTC (permalink / raw)
To: linuxppc-dev
As demonstrated in the previous commit, the failure message from the msi
bitmap selftests is a bit subtle, it's easy to miss a failure in a busy
boot log.
So drop our check() macro and use WARN_ON() instead. This necessitates
inverting all the conditions as well.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/sysdev/msi_bitmap.c | 54 ++++++++++++++++++----------------------
1 file changed, 24 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/sysdev/msi_bitmap.c b/arch/powerpc/sysdev/msi_bitmap.c
index 8155d93dee1d..73b64c73505b 100644
--- a/arch/powerpc/sysdev/msi_bitmap.c
+++ b/arch/powerpc/sysdev/msi_bitmap.c
@@ -145,69 +145,64 @@ void msi_bitmap_free(struct msi_bitmap *bmp)
#ifdef CONFIG_MSI_BITMAP_SELFTEST
-#define check(x) \
- if (!(x)) printk("msi_bitmap: test failed at line %d\n", __LINE__);
-
static void __init test_basics(void)
{
struct msi_bitmap bmp;
int rc, i, size = 512;
/* Can't allocate a bitmap of 0 irqs */
- check(msi_bitmap_alloc(&bmp, 0, NULL) != 0);
+ WARN_ON(msi_bitmap_alloc(&bmp, 0, NULL) == 0);
/* of_node may be NULL */
- check(0 == msi_bitmap_alloc(&bmp, size, NULL));
+ WARN_ON(msi_bitmap_alloc(&bmp, size, NULL));
/* Should all be free by default */
- check(0 == bitmap_find_free_region(bmp.bitmap, size,
- get_count_order(size)));
+ WARN_ON(bitmap_find_free_region(bmp.bitmap, size, get_count_order(size)));
bitmap_release_region(bmp.bitmap, 0, get_count_order(size));
/* With no node, there's no msi-available-ranges, so expect > 0 */
- check(msi_bitmap_reserve_dt_hwirqs(&bmp) > 0);
+ WARN_ON(msi_bitmap_reserve_dt_hwirqs(&bmp) <= 0);
/* Should all still be free */
- check(0 == bitmap_find_free_region(bmp.bitmap, size,
- get_count_order(size)));
+ WARN_ON(bitmap_find_free_region(bmp.bitmap, size, get_count_order(size)));
bitmap_release_region(bmp.bitmap, 0, get_count_order(size));
/* Check we can fill it up and then no more */
for (i = 0; i < size; i++)
- check(msi_bitmap_alloc_hwirqs(&bmp, 1) >= 0);
+ WARN_ON(msi_bitmap_alloc_hwirqs(&bmp, 1) < 0);
- check(msi_bitmap_alloc_hwirqs(&bmp, 1) < 0);
+ WARN_ON(msi_bitmap_alloc_hwirqs(&bmp, 1) >= 0);
/* Should all be allocated */
- check(bitmap_find_free_region(bmp.bitmap, size, 0) < 0);
+ WARN_ON(bitmap_find_free_region(bmp.bitmap, size, 0) >= 0);
/* And if we free one we can then allocate another */
msi_bitmap_free_hwirqs(&bmp, size / 2, 1);
- check(msi_bitmap_alloc_hwirqs(&bmp, 1) == size / 2);
+ WARN_ON(msi_bitmap_alloc_hwirqs(&bmp, 1) != size / 2);
/* Free most of them for the alignment tests */
msi_bitmap_free_hwirqs(&bmp, 3, size - 3);
/* Check we get a naturally aligned offset */
rc = msi_bitmap_alloc_hwirqs(&bmp, 2);
- check(rc >= 0 && rc % 2 == 0);
+ WARN_ON(rc < 0 && rc % 2 != 0);
rc = msi_bitmap_alloc_hwirqs(&bmp, 4);
- check(rc >= 0 && rc % 4 == 0);
+ WARN_ON(rc < 0 && rc % 4 != 0);
rc = msi_bitmap_alloc_hwirqs(&bmp, 8);
- check(rc >= 0 && rc % 8 == 0);
+ WARN_ON(rc < 0 && rc % 8 != 0);
rc = msi_bitmap_alloc_hwirqs(&bmp, 9);
- check(rc >= 0 && rc % 16 == 0);
+ WARN_ON(rc < 0 && rc % 16 != 0);
rc = msi_bitmap_alloc_hwirqs(&bmp, 3);
- check(rc >= 0 && rc % 4 == 0);
+ WARN_ON(rc < 0 && rc % 4 != 0);
rc = msi_bitmap_alloc_hwirqs(&bmp, 7);
- check(rc >= 0 && rc % 8 == 0);
+ WARN_ON(rc < 0 && rc % 8 != 0);
rc = msi_bitmap_alloc_hwirqs(&bmp, 121);
- check(rc >= 0 && rc % 128 == 0);
+ WARN_ON(rc < 0 && rc % 128 != 0);
msi_bitmap_free(&bmp);
- /* Clients may check bitmap == NULL for "not-allocated" */
- check(bmp.bitmap == NULL);
+ /* Clients may WARN_ON bitmap == NULL for "not-allocated" */
+ WARN_ON(bmp.bitmap != NULL);
kfree(bmp.bitmap);
}
@@ -229,14 +224,13 @@ static void __init test_of_node(void)
of_node_init(&of_node);
of_node.full_name = node_name;
- check(0 == msi_bitmap_alloc(&bmp, size, &of_node));
+ WARN_ON(msi_bitmap_alloc(&bmp, size, &of_node));
/* No msi-available-ranges, so expect > 0 */
- check(msi_bitmap_reserve_dt_hwirqs(&bmp) > 0);
+ WARN_ON(msi_bitmap_reserve_dt_hwirqs(&bmp) <= 0);
/* Should all still be free */
- check(0 == bitmap_find_free_region(bmp.bitmap, size,
- get_count_order(size)));
+ WARN_ON(bitmap_find_free_region(bmp.bitmap, size, get_count_order(size)));
bitmap_release_region(bmp.bitmap, 0, get_count_order(size));
/* Now create a fake msi-available-ranges property */
@@ -250,11 +244,11 @@ static void __init test_of_node(void)
of_node.properties = ∝
/* msi-available-ranges, so expect == 0 */
- check(msi_bitmap_reserve_dt_hwirqs(&bmp) == 0);
+ WARN_ON(msi_bitmap_reserve_dt_hwirqs(&bmp));
/* Check we got the expected result */
- check(0 == bitmap_parselist(expected_str, expected, size));
- check(bitmap_equal(expected, bmp.bitmap, size));
+ WARN_ON(bitmap_parselist(expected_str, expected, size));
+ WARN_ON(!bitmap_equal(expected, bmp.bitmap, size));
msi_bitmap_free(&bmp);
kfree(bmp.bitmap);
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] powerpc/msi: Use WARN_ON() in msi bitmap selftests
2014-10-10 8:04 ` [PATCH 2/2] powerpc/msi: Use WARN_ON() in msi bitmap selftests Michael Ellerman
@ 2014-10-10 8:11 ` Laurentiu Tudor
0 siblings, 0 replies; 3+ messages in thread
From: Laurentiu Tudor @ 2014-10-10 8:11 UTC (permalink / raw)
To: linuxppc-dev, mpe
Hi Michael,
Comment inline.
On 10/10/2014 11:04 AM, Michael Ellerman wrote:
> As demonstrated in the previous commit, the failure message from the msi
> bitmap selftests is a bit subtle, it's easy to miss a failure in a busy
> boot log.
>
> So drop our check() macro and use WARN_ON() instead. This necessitates
> inverting all the conditions as well.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/sysdev/msi_bitmap.c | 54 ++++++++++++++++++----------------------
[snip]
>
> /* Free most of them for the alignment tests */
> msi_bitmap_free_hwirqs(&bmp, 3, size - 3);
>
> /* Check we get a naturally aligned offset */
> rc = msi_bitmap_alloc_hwirqs(&bmp, 2);
> - check(rc >= 0 && rc % 2 == 0);
> + WARN_ON(rc < 0 && rc % 2 != 0);
Here and below, shouldn't these be:
WARN_ON(rc < 0 || rc % 2 != 0);
?
> rc = msi_bitmap_alloc_hwirqs(&bmp, 4);
> - check(rc >= 0 && rc % 4 == 0);
> + WARN_ON(rc < 0 && rc % 4 != 0);
> rc = msi_bitmap_alloc_hwirqs(&bmp, 8);
> - check(rc >= 0 && rc % 8 == 0);
> + WARN_ON(rc < 0 && rc % 8 != 0);
> rc = msi_bitmap_alloc_hwirqs(&bmp, 9);
> - check(rc >= 0 && rc % 16 == 0);
> + WARN_ON(rc < 0 && rc % 16 != 0);
> rc = msi_bitmap_alloc_hwirqs(&bmp, 3);
> - check(rc >= 0 && rc % 4 == 0);
> + WARN_ON(rc < 0 && rc % 4 != 0);
> rc = msi_bitmap_alloc_hwirqs(&bmp, 7);
> - check(rc >= 0 && rc % 8 == 0);
> + WARN_ON(rc < 0 && rc % 8 != 0);
> rc = msi_bitmap_alloc_hwirqs(&bmp, 121);
> - check(rc >= 0 && rc % 128 == 0);
> + WARN_ON(rc < 0 && rc % 128 != 0);
>
> msi_bitmap_free(&bmp);
>
> - /* Clients may check bitmap == NULL for "not-allocated" */
> - check(bmp.bitmap == NULL);
> + /* Clients may WARN_ON bitmap == NULL for "not-allocated" */
> + WARN_ON(bmp.bitmap != NULL);
>
> kfree(bmp.bitmap);
> }
> @@ -229,14 +224,13 @@ static void __init test_of_node(void)
> of_node_init(&of_node);
> of_node.full_name = node_name;
>
> - check(0 == msi_bitmap_alloc(&bmp, size, &of_node));
> + WARN_ON(msi_bitmap_alloc(&bmp, size, &of_node));
>
> /* No msi-available-ranges, so expect > 0 */
> - check(msi_bitmap_reserve_dt_hwirqs(&bmp) > 0);
> + WARN_ON(msi_bitmap_reserve_dt_hwirqs(&bmp) <= 0);
>
> /* Should all still be free */
> - check(0 == bitmap_find_free_region(bmp.bitmap, size,
> - get_count_order(size)));
> + WARN_ON(bitmap_find_free_region(bmp.bitmap, size, get_count_order(size)));
> bitmap_release_region(bmp.bitmap, 0, get_count_order(size));
>
> /* Now create a fake msi-available-ranges property */
> @@ -250,11 +244,11 @@ static void __init test_of_node(void)
> of_node.properties = ∝
>
> /* msi-available-ranges, so expect == 0 */
> - check(msi_bitmap_reserve_dt_hwirqs(&bmp) == 0);
> + WARN_ON(msi_bitmap_reserve_dt_hwirqs(&bmp));
>
> /* Check we got the expected result */
> - check(0 == bitmap_parselist(expected_str, expected, size));
> - check(bitmap_equal(expected, bmp.bitmap, size));
> + WARN_ON(bitmap_parselist(expected_str, expected, size));
> + WARN_ON(!bitmap_equal(expected, bmp.bitmap, size));
>
> msi_bitmap_free(&bmp);
> kfree(bmp.bitmap);
>
---
Best Regards, Laurentiu
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-10-10 8:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-10 8:04 [PATCH 1/2] powerpc/msi: Fix the msi bitmap alignment tests Michael Ellerman
2014-10-10 8:04 ` [PATCH 2/2] powerpc/msi: Use WARN_ON() in msi bitmap selftests Michael Ellerman
2014-10-10 8:11 ` Laurentiu Tudor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).