<br><tt><font size=2>Roland Dreier <rdreier@cisco.com> wrote on 01/30/2008
11:47:55 AM:<br>
> <br>
>  > I'm still annoyed by the (num_phys_buf == 1) special case.
 I'm wondering <br>
>  > if it's still needed.  If you leave out that if-statement
entirely, you <br>
>  > may end up using a page size that is larger (maybe much
larger) than <br>
>  > necessary, but I think things will still work, given that
the <br>
>  > virtual-to-physical alignment constraints are respected.
 If you remove <br>
>  > the special case, you can replace the whole loop with an
ffs() call.<br>
> <br>
> Makes sense... let me post a patch for discussion.</font></tt>
<br>
<br><tt><font size=2>For what it's worth, I removed the single-buffer special
case and replaced the whole second loop with an __ffs() call.  As
expected, it uses a very large page size to cover even a small single buffer
if the alignment constraints allow it, and it works.  Here's what
I've been playing with:</font></tt>
<br>
<br><tt><font size=2>================================================================================</font></tt>
<div>
<br><tt><font size=2>diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c
b/drivers/infiniband/hw/mthca/mthca_provider.c<br>
index 6bcde1c..b00ab71 100644<br>
--- a/drivers/infiniband/hw/mthca/mthca_provider.c<br>
+++ b/drivers/infiniband/hw/mthca/mthca_provider.c<br>
@@ -929,11 +929,7 @@ static struct ib_mr *mthca_reg_phys_mr(struct ib_pd
      *pd,<br>
         int err;<br>
         int i, j, n;<br>
 <br>
-        /* First check that we have enough
alignment */<br>
-        if ((*iova_start & ~PAGE_MASK)
!= (buffer_list[0].addr & ~PAGE_MASK))<br>
-                return
ERR_PTR(-EINVAL);<br>
-<br>
-        mask = 0;<br>
+        mask = buffer_list[0].addr ^ *iova_start;<br>
         total_size = 0;<br>
         for (i = 0; i < num_phys_buf; ++i)
{<br>
                 if
(i != 0)<br>
@@ -948,16 +944,7 @@ static struct ib_mr *mthca_reg_phys_mr(struct ib_pd
      *pd,<br>
                 return
ERR_PTR(-EINVAL);<br>
 <br>
         /* Find largest page shift we can
use to cover buffers */<br>
-        for (shift = PAGE_SHIFT; shift <
31; ++shift)<br>
-                if
(num_phys_buf > 1) {<br>
-                
       if ((1ULL << shift) & mask)<br>
-                
               break;<br>
-                }
else {<br>
-                
       if (1ULL << shift >=<br>
-                
           buffer_list[0].size +<br>
-                
           (buffer_list[0].addr &
((1ULL << shift) - 1)))</font></tt>
<br><tt><font size=2>-            
               
   break;<br>
-                }<br>
+        shift = __ffs((unsigned long) ((mask
& ((1ull<<31)-1)) | (1ull<<31)));<br>
 <br>
         buffer_list[0].size += buffer_list[0].addr
& ((1ULL << shift) - 1);<br>
         buffer_list[0].addr &= ~0ull <<
shift;</font></tt>
<br><tt><font size=2>================================================================================<br>
</font></tt></div>