Thanks for working on this Guy.
I received these errors when I tried to apply this patch:
patching file ib/dapl_openib_util.c
Hunk #1 FAILED at 234.
Hunk #2 FAILED at 281.
Hunk #3 FAILED at 300.
Hunk #4 FAILED at 309.
Hunk #5 FAILED at 318.
5 out of 5 hunks FAILED -- saving rejects to file
ib/dapl_openib_util.c.rej
patching file ib/dapl_openib_util.h
Hunk #1 FAILED at 92.
1 out of 1 hunk FAILED -- saving rejects to file
ib/dapl_openib_util.h.rej
patching file ib/dapl_lmr.c
Hunk #1 FAILED at 125.
Hunk #2 succeeded at 147 with fuzz 2.
Hunk #3 FAILED at 164.
Hunk #4 FAILED at 240.
Hunk #5 FAILED at 257.
Hunk #6 FAILED at 284.
Hunk #7 FAILED at 307.
Hunk #8 FAILED at 324.
7 out of 8 hunks FAILED -- saving rejects to file ib/dapl_lmr.c.rej
Due to the errors I had some trouble looking over your changes. Can
you resend?
One high level comment. I would prefer to see the DAT_MEM_TYPE_IA
support added in new function called dapl_lmr_create_ia rather than
overloading the dapl_lmr_create_physical function. Adding a
dapl_lmr_create_ia function would be more in keeping with the existing
structure.
A few comments/questions below:
On Wed, 27 Jul 2005, Guy German wrote:
[kdapl]: this patch adds the option of using DAT_MEM_TYPE_IA in
dat_lmr_kcreate
Signed-off-by: Guy German <[EMAIL PROTECTED]>
Index: infiniband/ulp/kdapl/ib/dapl_openib_util.c
===================================================================
--- infiniband/ulp/kdapl/ib/dapl_openib_util.c (revision 2914)
+++ infiniband/ulp/kdapl/ib/dapl_openib_util.c (working copy)
@@ -234,8 +234,43 @@ int dapl_ib_mr_register(struct dapl_ia *
return -ENOSYS;
}
+int dapl_ib_mr_register_ia(struct dapl_ia *ia, struct dapl_lmr *lmr,
+ union dat_region_description phys_addr, u64
length,
+ enum dat_mem_priv_flags privileges)
+{
+ int status;
+ int acl;
+ struct ib_mr *mr;
+ struct ib_phys_buf buf_list;
+ u64 iova = 0;
+ buf_list.addr = phys_addr.for_pa;
+ buf_list.size = length;
+ + iova = buf_list.addr;
+ acl = dapl_ib_convert_mem_privileges(privileges);
+ acl |= IB_ACCESS_MW_BIND;
+ mr = ib_reg_phys_mr(((struct dapl_pz *)lmr->param.pz)->pd,
+ &buf_list, 1, acl, &iova); + if
(IS_ERR(mr)) {
+ status = PTR_ERR(mr);
+ dapl_dbg_log(DAPL_DBG_TYPE_ERR,
+ "%s: ib_reg_phys_mr error code return = %d\n",
+ __FUNCTION__, status);
+ return status;
+ }
+ dapl_dbg_log(DAPL_DBG_TYPE_UTIL, "%s: got handle mr=%p\n",
+ __FUNCTION__, mr);
+
+ lmr->param.lmr_context = mr->lkey;
+ lmr->param.rmr_context = mr->rkey;
+ lmr->param.registered_size = length;
+ lmr->param.registered_address = phys_addr.for_pa;
+ lmr->mr = mr;
+ return 0;
+}
+
int dapl_ib_mr_register_physical(struct dapl_ia *ia, struct dapl_lmr *lmr,
- void *phys_addr, u64 length,
+ union dat_region_description phys_addr, u64
length,
enum dat_mem_priv_flags privileges)
{
int status;
@@ -246,7 +281,7 @@ int dapl_ib_mr_register_physical(struct
u64 iova = 0;
u64 *array;
- array = (u64 *) phys_addr;
+ array = (u64 *)phys_addr.for_array; /* need to add for_u64_array to
union */
What does this comment mean?
buf_list = kmalloc(length * sizeof *buf_list, GFP_ATOMIC);
if (!buf_list)
return -ENOMEM;
@@ -265,8 +300,8 @@ int dapl_ib_mr_register_physical(struct
if (IS_ERR(mr)) {
status = PTR_ERR(mr);
dapl_dbg_log(DAPL_DBG_TYPE_ERR,
- " ib_reg_phys_mr error code return = %d\n",
- status);
+ "%s: ib_reg_phys_mr error code return = %d\n",
+ __FUNCTION__, status);
return status;
}
#if 0
@@ -274,8 +309,8 @@ int dapl_ib_mr_register_physical(struct
status = ib_query_mr(mr, &attr);
if (status < 0) {
dapl_dbg_log(DAPL_DBG_TYPE_ERR,
- " ib_query_mr error code return from ib_query_mr
= %d\n",
- status);
+ "%s: ib_query_mr error code return from
ib_query_mr = %d\n",
+ __FUNCTION__, status);
We try to keep lines less than 80 characters. Is there a more concise
way to convey the message above? In this case the original message
appears to be poorly written.
ib_dereg_mr(mr);
return status;
}
@@ -283,10 +318,12 @@ int dapl_ib_mr_register_physical(struct
lmr->param.lmr_context = mr->lkey;
lmr->param.rmr_context = mr->rkey;
+ lmr->param.registered_size = length * PAGE_SIZE;
+ lmr->param.registered_address = array[0];
lmr->mr = mr;
dapl_dbg_log(DAPL_DBG_TYPE_UTIL,
- "dapl_ib_mr_register_physical(%p %d) got lkey 0x%x \n",
+ "%s: (%p %d) got lkey 0x%x \n", __FUNCTION__,
buf_list, length, lmr->param.lmr_context);
return 0;
}
Index: infiniband/ulp/kdapl/ib/dapl_openib_util.h
===================================================================
--- infiniband/ulp/kdapl/ib/dapl_openib_util.h (revision 2914)
+++ infiniband/ulp/kdapl/ib/dapl_openib_util.h (working copy)
@@ -92,8 +92,13 @@ int dapl_ib_mr_register(struct dapl_ia *
void *virt_addr, u64 length,
enum dat_mem_priv_flags privileges);
+int dapl_ib_mr_register_ia(struct dapl_ia *ia, struct dapl_lmr *lmr,
+ union dat_region_description phys_addr, u64
length,
+ enum dat_mem_priv_flags privileges);
+
int dapl_ib_mr_register_physical(struct dapl_ia *ia, struct dapl_lmr *lmr,
- void *phys_addr, u64 length,
+ union dat_region_description phys_addr, +
u64 length,
enum dat_mem_priv_flags privileges);
int dapl_ib_mr_deregister(struct dapl_lmr *lmr);
Index: infiniband/ulp/kdapl/ib/dapl_lmr.c
===================================================================
--- infiniband/ulp/kdapl/ib/dapl_lmr.c (revision 2914)
+++ infiniband/ulp/kdapl/ib/dapl_lmr.c (working copy)
@@ -125,20 +125,21 @@ error1:
}
static inline int dapl_lmr_create_physical(struct dapl_ia *ia,
- DAT_REGION_DESCRIPTION phys_addr,
- u64 page_count, struct dapl_pz
*pz,
+ union dat_region_description
phys_addr,
+ u64 page_count, +
enum dat_mem_type mem_type,
+ struct dapl_pz *pz,
enum dat_mem_priv_flags
privileges,
struct dat_lmr **lmr,
- DAT_LMR_CONTEXT *lmr_context,
- DAT_RMR_CONTEXT *rmr_context,
+ u32 *lmr_context,
+ u32 *rmr_context,
Why remove the DAT_LMR_CONTEXT and DAT_RMR_CONTEXT types? I would
prefer to remove everyone of them at once.
u64 *registered_length,
u64 *registered_address)
{
struct dapl_lmr *new_lmr;
- u64 *array = phys_addr.for_array;
int status;
- new_lmr = dapl_lmr_alloc(ia, DAT_MEM_TYPE_PHYSICAL, phys_addr,
+ new_lmr = dapl_lmr_alloc(ia, mem_type, phys_addr,
page_count, (struct dat_pz *) pz,
privileges);
if (NULL == new_lmr) {
@@ -146,8 +147,14 @@ static inline int dapl_lmr_create_physic
goto error1;
}
- status = dapl_ib_mr_register_physical(ia, new_lmr,
phys_addr.for_array,
- page_count, privileges);
Something is strange here. The call to dapl_ib_mr_register_physical is
inside an else clause in the current code.
+ if (DAT_MEM_TYPE_IA == mem_type) {
+ status = dapl_ib_mr_register_ia(ia, new_lmr, phys_addr,
+ page_count, privileges);
+ }
+ else {
+ status = dapl_ib_mr_register_physical(ia, new_lmr, phys_addr,
+ page_count,
privileges);
+ }
if (0 != status)
goto error2;
@@ -157,13 +164,13 @@ static inline int dapl_lmr_create_physic
if (lmr)
*lmr = (struct dat_lmr *)new_lmr;
if (lmr_context)
- *lmr_context = (DAT_LMR_CONTEXT)new_lmr->param.lmr_context;
+ *lmr_context = (u32)new_lmr->param.lmr_context;
if (rmr_context)
- *rmr_context = (DAT_LMR_CONTEXT)new_lmr->param.rmr_context;
+ *rmr_context = (u32)new_lmr->param.rmr_context;
if (registered_address)
- *registered_address = array[0];
+ *registered_address = new_lmr->param.registered_address;
if (registered_length)
- *registered_length = page_count * PAGE_SIZE;
+ *registered_length = new_lmr->param.registered_size;
return 0;
@@ -233,7 +240,7 @@ int dapl_lmr_kcreate(struct dat_ia *ia,
struct dapl_ia *dapl_ia;
struct dapl_pz *dapl_pz;
int status;
-
+
dapl_dbg_log(DAPL_DBG_TYPE_API,
"dapl_lmr_kcreate(ia:%p, mem_type:%x, ...)\n",
ia, mem_type);
@@ -250,10 +257,12 @@ int dapl_lmr_kcreate(struct dat_ia *ia,
rmr_context,
registered_length,
registered_address);
break;
+ case DAT_MEM_TYPE_PLATFORM: // used as proprietary Tavor-FMR
case DAT_MEM_TYPE_PHYSICAL:
+ case DAT_MEM_TYPE_IA:
status = dapl_lmr_create_physical(dapl_ia,
region_description,
- length, dapl_pz,
privileges,
- lmr, lmr_context,
+ length, mem_type, dapl_pz,
+ privileges, lmr,
lmr_context,
rmr_context,
registered_length,
registered_address);
@@ -275,8 +284,6 @@ int dapl_lmr_kcreate(struct dat_ia *ia,
registered_address);
break;
}
- case DAT_MEM_TYPE_PLATFORM:
- case DAT_MEM_TYPE_IA:
case DAT_MEM_TYPE_BYPASS:
status = -ENOSYS;
break;
@@ -300,7 +307,8 @@ int dapl_lmr_free(struct dat_lmr *lmr)
switch (dapl_lmr->param.mem_type) {
case DAT_MEM_TYPE_PHYSICAL:
- case DAT_MEM_TYPE_VIRTUAL:
+ case DAT_MEM_TYPE_PLATFORM:
+ case DAT_MEM_TYPE_IA:
case DAT_MEM_TYPE_LMR:
{
struct dapl_pz *pz;
@@ -316,8 +324,7 @@ int dapl_lmr_free(struct dat_lmr *lmr)
}
break;
}
- case DAT_MEM_TYPE_PLATFORM:
- case DAT_MEM_TYPE_IA:
+ case DAT_MEM_TYPE_VIRTUAL:
case DAT_MEM_TYPE_BYPASS:
status = -ENOSYS;
break;
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
