Re: [RFC PATCH 03/16] ib/mad: Add check for jumbo MADs support on a device
On 1/7/2015 6:32 PM, Weiny, Ira wrote:
>On 11/27/2014 3:51 PM, Sagi Grimberg wrote:
> >We're already short on bits in device_cap_flags
>
>no shortage @ the kernel... we can add more 32 bits if/when we need it
Why is there a gap in the bits?
enum ib_device_cap_flags {
...
IB_DEVICE_MEM_WINDOW_TYPE_2B= (1<<24),
IB_DEVICE_MANAGED_FLOW_STEERING = (1<<29),
...
};
25-28 are not used? Is there some legacy software out there which uses these?
sort of... the verbs RSS design which wasn't accepted upstream uses bits
25/26/27 and Mellanox Core-Direct (whose verbs model called
"Cross-Channel") which wasn't submitted upstream yet uses bit 28. Since
in the kernel we can trivially use 64 bits, I would opt to keep these
entries free for now.
Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH 03/16] ib/mad: Add check for jumbo MADs support on a device
>
> On 11/27/2014 3:51 PM, Sagi Grimberg wrote:
> > We're already short on bits in device_cap_flags
>
> no shortage @ the kernel... we can add more 32 bits if/when we need it
Why is there a gap in the bits?
enum ib_device_cap_flags {
...
IB_DEVICE_MEM_WINDOW_TYPE_2B= (1<<24),
IB_DEVICE_MANAGED_FLOW_STEERING = (1<<29),
...
};
25-28 are not used? Is there some legacy software out there which uses these?
Ira
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 03/16] ib/mad: Add check for jumbo MADs support on a device
On Wed, Dec 10, 2014 at 12:36 AM, Weiny, Ira wrote: >> On Mon, Dec 8, 2014 at 2:23 AM, Weiny, Ira wrote: >> 1. add a struct ib_device_attr field to struct ib_device >> 2. when the device registers itself with the IB core, go and run the >> query_device verb with the param being pointer to that field > I see where you are going. Then the MAD stack does not have to cache a > "max_mad_size" value but rather looks in the ib_device structure "on the > fly"... > So, something like the diff below? exactly, thanks. > What are the chances we end up with attributes which are not constant? I don't see how this can happen. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH 03/16] ib/mad: Add check for jumbo MADs support on a device
>
> On Sun, Dec 7, 2014 at 4:23 PM, Weiny, Ira wrote:
> > I don't think this is a bad idea, however, the complication is determining
> > the
> kmem_cache object size in that case. How about we limit this value to < 2K
> until such time as there is a need for larger support?
>
> Can we just get rid of all the kmem_caches used in the MAD code? I don't
> think any of this is so performance-critical that we couldn't just use
> kmalloc...
>
On an SM node the MAD code is _very_ performance-critical. I don’t think we
should give that up.
Another alternative is to have a cache per device based on the size reported.
-- Ira
N�r��yb�X��ǧv�^�){.n�+{��ٚ�{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj"��!�i
RE: [RFC PATCH 03/16] ib/mad: Add check for jumbo MADs support on a device
>
> On Mon, Dec 8, 2014 at 2:23 AM, Weiny, Ira wrote:
>
> >> I find it very annoying that upper level drivers replicate in
> >> different ways elements from the IB device attributes returned by
> >> ib_query_device. I met that in multiple drivers and upcoming designs
> >> for which I do code review. Are you up to come up with a patch that
> >> caches the device attributes on the device structure?
>
> > I don't follow what you are asking for. Could you give more details?
>
> 1. add a struct ib_device_attr field to struct ib_device
>
> 2. when the device registers itself with the IB core, go and run the
> query_device verb with the param being pointer to that field
I see where you are going. Then the MAD stack does not have to cache a
"max_mad_size" value but rather looks in the ib_device structure "on the fly"...
So, something like the diff below? What are the chances we end up with
attributes which are not constant?
If Roland would like to go this way I can rework my series based on the
attributes being cached.
-- Ira
17:15:59 > git di
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 18c1ece..db18795 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -322,6 +322,8 @@ int ib_register_device(struct ib_device *device,
client->add(device);
}
+ device->query_device(device, &device->attributes);
+
out:
mutex_unlock(&device_mutex);
return ret;
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 470a011..241a882 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1630,6 +1630,7 @@ struct ib_device {
u32 local_dma_lkey;
u8 node_type;
u8 phys_port_cnt;
+ struct ib_device_attrattributes;
};
struct ib_client {
Re: [RFC PATCH 03/16] ib/mad: Add check for jumbo MADs support on a device
On Mon, Dec 8, 2014 at 2:23 AM, Weiny, Ira wrote: >> I find it very annoying that upper level drivers replicate in different ways >> elements from the IB device attributes returned by ib_query_device. I met >> that >> in multiple drivers and upcoming designs for which I do code review. Are you >> up to come up with a patch that caches the device attributes on the device >> structure? > I don't follow what you are asking for. Could you give more details? 1. add a struct ib_device_attr field to struct ib_device 2. when the device registers itself with the IB core, go and run the query_device verb with the param being pointer to that field -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 03/16] ib/mad: Add check for jumbo MADs support on a device
On Sun, Dec 7, 2014 at 4:23 PM, Weiny, Ira wrote: > I don't think this is a bad idea, however, the complication is determining > the kmem_cache object size in that case. How about we limit this value to < > 2K until such time as there is a need for larger support? Can we just get rid of all the kmem_caches used in the MAD code? I don't think any of this is so performance-critical that we couldn't just use kmalloc... - R. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH 03/16] ib/mad: Add check for jumbo MADs support on a device
> > On 11/13/2014 9:54 PM, ira.we...@intel.com wrote: > > > Add a device capability flag for OPA devices to signal their support of > > "jumbo" > > MADs. > > > > Check for IB_DEVICE_JUMBO_MAD_SUPPORT in the device capabilities and > > if supported mark the special QPs created. > > Ira, > > Few comments here, the device capability is OK, specifically if it helps for > the > mad layer logic to be simpler and/or more robust. > > You should add device attribute telling what is the size of MADs supported by > the device, I think the IBTA mandated 256Band in the OPA case, the driver will > fill there 2k. I don't think this is a bad idea, however, the complication is determining the kmem_cache object size in that case. How about we limit this value to < 2K until such time as there is a need for larger support? > > You can't just state that (jumbo == 2k) and go to complete design/changes > which use this hard-coded assumption. You need to see how to re-spin this > series w.o this assumption. I'm looking into it. I believe I will still need to have a capability bit, but it may end up having a different meaning. > > I find it very annoying that upper level drivers replicate in different ways > elements from the IB device attributes returned by ib_query_device. I met that > in multiple drivers and upcoming designs for which I do code review. Are you > up to come up with a patch that caches the device attributes on the device > structure? I don't follow what you are asking for. Could you give more details? > if not, > I can do that.. and have your code to see it. > > Also, I would go and unify (squash) this patch and the preceding one. > I'll keep this in mind as I rework the patches. Ira -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 03/16] ib/mad: Add check for jumbo MADs support on a device
On 11/27/2014 3:51 PM, Sagi Grimberg wrote: We're already short on bits in device_cap_flags no shortage @ the kernel... we can add more 32 bits if/when we need it -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 03/16] ib/mad: Add check for jumbo MADs support on a device
On 11/27/2014 1:47 PM, Or Gerlitz wrote: On 11/13/2014 9:54 PM, ira.we...@intel.com wrote: Add a device capability flag for OPA devices to signal their support of "jumbo" MADs. Check for IB_DEVICE_JUMBO_MAD_SUPPORT in the device capabilities and if supported mark the special QPs created. Ira, Few comments here, the device capability is OK, specifically if it helps for the mad layer logic to be simpler and/or more robust. You should add device attribute telling what is the size of MADs supported by the device, I think the IBTA mandated 256Band in the OPA case, the driver will fill there 2k. You can't just state that (jumbo == 2k) and go to complete design/changes which use this hard-coded assumption. You need to see how to re-spin this series w.o this assumption. Why do we need both? can't we just rely on just the size? We're already short on bits in device_cap_flags so maybe we can do without? Sagi. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 03/16] ib/mad: Add check for jumbo MADs support on a device
On 11/13/2014 9:54 PM, ira.we...@intel.com wrote: Add a device capability flag for OPA devices to signal their support of "jumbo" MADs. Check for IB_DEVICE_JUMBO_MAD_SUPPORT in the device capabilities and if supported mark the special QPs created. Ira, Few comments here, the device capability is OK, specifically if it helps for the mad layer logic to be simpler and/or more robust. You should add device attribute telling what is the size of MADs supported by the device, I think the IBTA mandated 256Band in the OPA case, the driver will fill there 2k. You can't just state that (jumbo == 2k) and go to complete design/changes which use this hard-coded assumption. You need to see how to re-spin this series w.o this assumption. I find it very annoying that upper level drivers replicate in different ways elements from the IB device attributes returned by ib_query_device. I met that in multiple drivers and upcoming designs for which I do code review. Are you up to come up with a patch that caches the device attributes on the device structure? if not, I can do that.. and have your code to see it. Also, I would go and unify (squash) this patch and the preceding one. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
