> I was thinking about applications which create and destroy HCA
> resources at a high rate which might be affected.
I would expect all the other costs of mapping pages into/out of the ICM
to be large enough that the cost of zeroing a page is not too big a
deal. But if there is anyone who cares we can always make this
__GFP_ZERO flag conditional on FW version I guess.
> Looks to me like using __GFP_ZERO is the cleanest approach. I like
> less clear_highpage() since it uses kmap_atomic() which could fail.
To the best of my knowledge, kmap_atomic() cannot fail -- and if you
think about it, what could make it fail? The whole point of
kmap_atomic() is that there is a per-cpu pre-reserved slot to map the
memory at for highmem pages, so it has to always work. As far as I can
see, __GFP_ZERO allocations use clear_highpage() internally anyway, so
it ends up being the same thing.
Since just adding __GFP_ZERO is so much simpler, I'll just commit the
patch below and send it to Linus in a day or two if it seems OK to you:
commit 801d1ad7b6bdb3418a462c6b4950aee56dbac940
Author: Eli Cohen <[EMAIL PROTECTED]>
Date: Sun Jun 22 12:56:58 2008 -0700
IB/mthca: Clear ICM pages before handing to FW
Current memfree FW has a bug which in some cases, assumes that ICM
pages passed to it are cleared. This patch uses __GFP_ZERO to
allocate all ICM pages passed to the FW. Once firmware with a fix is
released, we can make the workaround conditional on firmware version.
This fixes the bug reported by Arthur Kepner <[EMAIL PROTECTED]> here:
http://lists.openfabrics.org/pipermail/general/2008-May/050026.html
Signed-off-by: Eli Cohen <[EMAIL PROTECTED]>
[ Rewritten to be a one-liner using __GFP_ZERO instead of vmap()ing
ICM memory and memset()ing it to 0. - Roland ]
Signed-off-by: Roland Dreier <[EMAIL PROTECTED]>
diff --git a/drivers/infiniband/hw/mthca/mthca_memfree.c
b/drivers/infiniband/hw/mthca/mthca_memfree.c
index 9e77ba9..1f7d1a2 100644
--- a/drivers/infiniband/hw/mthca/mthca_memfree.c
+++ b/drivers/infiniband/hw/mthca/mthca_memfree.c
@@ -107,7 +107,11 @@ static int mthca_alloc_icm_pages(struct scatterlist *mem,
int order, gfp_t gfp_m
{
struct page *page;
- page = alloc_pages(gfp_mask, order);
+ /*
+ * Use __GFP_ZERO because buggy firmware assumes ICM pages are
+ * cleared, and subtle failures are seen if they aren't.
+ */
+ page = alloc_pages(gfp_mask | __GFP_ZERO, order);
if (!page)
return -ENOMEM;
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
