IB/ehca: Make sure user pages are from hugetlb before using MR large pages
PATCH 1/1 IB/ehca: Make sure user pages are from hugetlb before using MR large pages
From: Hoang-Nam Nguyen <[email protected]>
...because, on virtualized hardware like System p, we can't be sure that the physical pages behind them are contiguous.
---
Another patch for 2.6.24 that will apply cleanly on top of my previous patchset. Please review and apply. Thanks!
drivers/infiniband/hw/ehca/ehca_classes.h | 8 ++-- drivers/infiniband/hw/ehca/ehca_mrmw.c | 82 +++++++++++++++++++++++++---- 2 files changed, 75 insertions(+), 15 deletions(-)
drivers/infiniband/hw/ehca/ehca_classes.h +4 -4
@@ -99,10 +99,10 @@ struct ehca_sport {struct ehca_sma_attr saved_attr; }; -#define HCA_CAP_MR_PGSIZE_4K 1 -#define HCA_CAP_MR_PGSIZE_64K 2 -#define HCA_CAP_MR_PGSIZE_1M 4 -#define HCA_CAP_MR_PGSIZE_16M 8 +#define HCA_CAP_MR_PGSIZE_4K 0x80000000 +#define HCA_CAP_MR_PGSIZE_64K 0x40000000 +#define HCA_CAP_MR_PGSIZE_1M 0x20000000 +#define HCA_CAP_MR_PGSIZE_16M 0x10000000struct ehca_shca {struct ib_device ib_device;
drivers/infiniband/hw/ehca/ehca_mrmw.c +71 -11
107 lines not on this page · raw message
Signature · 1 line
1.5.2
drivers/infiniband/hw/ehca/ehca_classes.h struct ehca_sport { · line 105
Quoted 8 lines
drivers/infiniband/hw/ehca/ehca_classes.h +4
-4-#define HCA_CAP_MR_PGSIZE_4K 1 -#define HCA_CAP_MR_PGSIZE_64K 2 -#define HCA_CAP_MR_PGSIZE_1M 4 -#define HCA_CAP_MR_PGSIZE_16M 8 +#define HCA_CAP_MR_PGSIZE_4K 0x80000000 +#define HCA_CAP_MR_PGSIZE_64K 0x40000000 +#define HCA_CAP_MR_PGSIZE_1M 0x20000000 +#define HCA_CAP_MR_PGSIZE_16M 0x10000000
Not sure I understand what this has to do with things... is this an unrelated fix?
Quoted 1 line
+static int
ehca_is_mem_hugetlb(unsigned long addr, unsigned long size)
This is rather awful -- another call to get_user_pages() to iterate over all the vmas...
I would suggest extending ib_umem_get() to check the vmas and adding a member to struct ib_umem to say whether the memory is entirely covered by hugetlb pages or not.
Quoted 9 lines
drivers/infiniband/hw/ehca/ehca_mrmw.c +9
-0+ ret =ehca_is_mem_hugetlb (virt, length); +switch (ret) { +case 0 :/* mem is not from hugetlb */ + hwpage_size =PAGE_SIZE ; +break ; +case 1 : +if (length <=EHCA_MR_PGSIZE4K + &&PAGE_SIZE ==EHCA_MR_PGSIZE4K ) + hwpage_size =EHCA_MR_PGSIZE4K ;
drivers/infiniband/hw/ehca/ehca_mrmw.c struct ib_mr *ehca_reg_user_mr(struct ib_pd *pd, u64 start, u64 length, · line 417
Quoted 10 lines
drivers/infiniband/hw/ehca/ehca_mrmw.c +10
-0+else if (length <=EHCA_MR_PGSIZE64K ) + hwpage_size =EHCA_MR_PGSIZE64K ; +else if (length <=EHCA_MR_PGSIZE1M ) + hwpage_size =EHCA_MR_PGSIZE1M ; +else + hwpage_size =EHCA_MR_PGSIZE16M ; +break ; +default :/* out of mem */ + ib_mr =ERR_PTR (-ENOMEM ); + goto reg_user_mr_exit1;
It seems like it would be better to just assume the memory is not from a hugetlb is ehca_is_mem_hugetlb() fails its memory allocation and fall back to the PAGE_SIZE case rather than failing entirely.
Also if someone runs a kernel with 64K pages on a machine where they end up being simulated from 4K pages, do you have the same issue with the hypervisor ganging together non-contiguous pages?
- R.
Roland Dreier wrote on 13.09.2007 06:33:45:
Quoted 4 lines
Also if someone runs a kernel with 64K pages on a machine where they end up being simulated from 4K pages, do you have the same issue with the hypervisor ganging together non-contiguous pages?
With todays hypervisor and todays pagesizes and todays MMUs we don't have this problem if eHCA is enabled.
It is difficult to make predictions about the future, but that's not specific to driver development. ;-)
Quoted 2 lines
- R.
- Christoph R.
Roland Dreier <[email protected]> wrote on 13.09.2007 06:33:45:
Quoted 11 lines
drivers/infiniband/hw/ehca/ehca_classes.h struct ehca_sport { · line 105
Quoted 8 lines
drivers/infiniband/hw/ehca/ehca_classes.h +4
-4-#define HCA_CAP_MR_PGSIZE_4K 1 -#define HCA_CAP_MR_PGSIZE_64K 2 -#define HCA_CAP_MR_PGSIZE_1M 4 -#define HCA_CAP_MR_PGSIZE_16M 8 +#define HCA_CAP_MR_PGSIZE_4K 0x80000000 +#define HCA_CAP_MR_PGSIZE_64K 0x40000000 +#define HCA_CAP_MR_PGSIZE_1M 0x20000000 +#define HCA_CAP_MR_PGSIZE_16M 0x10000000Not sure I understand what this has to do with things... is this an unrelated fix?
Kinda. I can put it into its own patch if you want.
Quoted 3 lines
I would suggest extending
ib_umem_get()to check the vmas and adding a member to structib_umemto say whether the memory is entirely covered by hugetlb pages or not.
I like that approach - one patch coming right up! =)
Quoted 7 lines
drivers/infiniband/hw/ehca/ehca_mrmw.c struct ib_mr *ehca_reg_user_mr(struct ib_pd *pd, u64 start, u64 length, · line 417
Quoted 3 lines
drivers/infiniband/hw/ehca/ehca_mrmw.c +3
-0+default :/* out of mem */ + ib_mr =ERR_PTR (-ENOMEM ); + goto reg_user_mr_exit1;It seems like it would be better to just assume the memory is not from a hugetlb is
ehca_is_mem_hugetlb()fails its memory allocation and fall back to thePAGE_SIZEcase rather than failing entirely.
If ehca_is_mem_hugetlb() runs out of memory, ehca_reg_mr() is rather unlikely to get the memory, but it's worth a try, I'll give you that. I'll make the umem patch work that way.
Joachim