Re: merge struct ib_device_attr into struct ib_device V2

Did we converge on this? Just a heads up to Doug, this conflicts with [PATCH v4 11/16] xprtrdma: Pre-allocate Work Requests for backchannel but it's trivial to sort out... -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to

Re: merge struct ib_device_attr into struct ib_device V2

On 10/21/2015 12:11 AM, Or Gerlitz wrote: haven't found any review or ack to your giant patch that touches the > whole subsystem (drivers, core and ULPs) expect from Sagi's -- lets > hear more opinions. Although I have not yet had the time to review the entire patch, removing

Re: merge struct ib_device_attr into struct ib_device V2

On Wed, Oct 21, 2015 at 08:48:10AM -0700, Bart Van Assche wrote: > On 10/21/2015 12:11 AM, Or Gerlitz wrote: > >haven't found any review or ack to your giant patch that touches the > > whole subsystem (drivers, core and ULPs) expect from Sagi's -- lets > > hear more opinions. > > Although I have

Re: merge struct ib_device_attr into struct ib_device V2

On 10/21/2015 11:43 AM, Jason Gunthorpe wrote: On Wed, Oct 21, 2015 at 08:48:10AM -0700, Bart Van Assche wrote: On 10/21/2015 12:11 AM, Or Gerlitz wrote: haven't found any review or ack to your giant patch that touches the whole subsystem (drivers, core and ULPs) expect from Sagi's -- lets

Re: merge struct ib_device_attr into struct ib_device V2

On Wed, Oct 21, 2015 at 6:48 PM, Bart Van Assche wrote: > On 10/21/2015 12:11 AM, Or Gerlitz wrote: >> >> haven't found any review or ack to your giant patch that touches the > >> whole subsystem (drivers, core and ULPs) expect from Sagi's -- lets >> hear more

Re: merge struct ib_device_attr into struct ib_device V2

On Wed, Oct 21, 2015 at 9:20 PM, Jason Gunthorpe wrote: > No to the pointer idea. can you spare few words why? And if you have a valid argument, as Christoph said they teach in cache 101 course which he's pretty sure I skipped as twenty other courses taken by

Re: merge struct ib_device_attr into struct ib_device V2

On Wed, Oct 21, 2015 at 09:08:31PM +0300, Or Gerlitz wrote: > The alternative approach to address this which was also running here > in the form of a patch from Ira following a reviewer comment from me, > is the have struct ib_device to contain a struct ib_device_attr member > (potentially a

Re: merge struct ib_device_attr into struct ib_device V2

On Tue, Oct 20, 2015 at 06:07:31PM +0300, Or Gerlitz wrote: > But this makes struct ib_device much much bigger, and this structure > is used in **fast** path, > e.g the ULP traverses through a pointer from struct ib_device to > post_send/recv, poll_cq and friends. But it doesn't get near the end

Re: merge struct ib_device_attr into struct ib_device V2

On Tue, Oct 20, 2015 at 06:13:35PM +0300, Sagi Grimberg wrote: >> The networking community will let you work for 10y before they add a >> field to struct net_device exactly b/c >> of the reason I brought. Why here we can come out of the blue and add >> tens if not hundreds of fields to our device

Re: merge struct ib_device_attr into struct ib_device V2

On 10/21/2015 9:38 AM, Christoph Hellwig wrote: On Tue, Oct 20, 2015 at 06:07:31PM +0300, Or Gerlitz wrote: >But this makes struct ib_device much much bigger, and this structure >is used in **fast** path, >e.g the ULP traverses through a pointer from struct ib_device to >post_send/recv, poll_cq

Re: merge struct ib_device_attr into struct ib_device V2

On Wed, Oct 21, 2015 at 09:44:41AM +0300, Or Gerlitz wrote: > Fact is that for struct net_device you will not add 333 new fields over > night in the coming 33 years, for sure. That's because they never had this split and added fields to struct netdev as required. One interesting difference in

Re: merge struct ib_device_attr into struct ib_device V2

On 10/21/2015 9:51 AM, Christoph Hellwig wrote: We have a UAPI that requires us to keep the device query verb for ever, and hence the returned device attr struct, it would be a much smaller and noisy patch pointer to cache an device attr on the device structure. Take a look at the code. The

Re: merge struct ib_device_attr into struct ib_device V2

On Wed, Oct 21, 2015 at 10:11:29AM +0300, Or Gerlitz wrote: > > We will have many more device query extensions, None of which use struct ib_device_attr I hope! > but the point I tried to > make here is a bit different -- > we do need to keep the user/kernel device attr struct as part of the

Re: merge struct ib_device_attr into struct ib_device V2

On 10/21/2015 10:33 AM, Christoph Hellwig wrote: For anything you want to add you need to touch_a_ struct. It's not any difference in efforts if it's ib_device_attr or ib_device. You're not even saving much memory if at all as every driver caches at least some fields of it in their own device

Re: merge struct ib_device_attr into struct ib_device V2

On Wed, Oct 21, 2015 at 10:41:13AM +0300, Or Gerlitz wrote: > I know, but a patch that adds caching an attr pointer on the device will > remove these local caches, > we actually had that/similar patch posted here and it was dropped/forgotten. > >> so if you use one of those you're >> already

Re: merge struct ib_device_attr into struct ib_device V2

On Tue, Oct 20, 2015 at 4:08 PM, Sagi Grimberg wrote: >>> I think this is very useful to have and I'd love having it in 4.4, >>> does anyone have any other comments on this patch? >> Were we ever presented with performance gains achieved using the patch? > Can you

Re: merge struct ib_device_attr into struct ib_device V2

So this patch seems to work well for me. I've ran some kernel applications (ipoib, iser, srp, nfs) and a user-space application (ibsrpdm) over mlx4/mlx5 and didn't spot any issues. I think this is very useful to have and I'd love having it in 4.4, does anyone have any other comments on this

Re: merge struct ib_device_attr into struct ib_device V2

On Tue, Oct 20, 2015 at 3:00 PM, Sagi Grimberg wrote: > So this patch seems to work well for me. I've ran some kernel > applications (ipoib, iser, srp, nfs) and a user-space application > (ibsrpdm) over mlx4/mlx5 and didn't spot any issues. > > I think this is very

Re: merge struct ib_device_attr into struct ib_device V2

But this makes struct ib_device much much bigger, and this structure is used in **fast** path, e.g the ULP traverses through a pointer from struct ib_device to post_send/recv, poll_cq and friends. Umm, all the caps are appended to the end of the ib_device structure so I don't see how it will

Re: merge struct ib_device_attr into struct ib_device V2

On Tue, Oct 20, 2015 at 6:00 PM, Sagi Grimberg wrote: >> (2) re this one, as I wrote in the past, I am in favor of simple >> caching of struct ib_device_attr on struct ib_device (best with pointer) and >> not adding >> 333 fields to struct ib_device, I don't see the

Re: merge struct ib_device_attr into struct ib_device V2

On Tue, Oct 20, 2015 at 6:13 PM, Sagi Grimberg wrote: >> The networking community will let you work for 10y before they add a >> field to struct net_device exactly b/c >> of the reason I brought. Why here we can come out of the blue and add >> tens if not hundreds of

Re: merge struct ib_device_attr into struct ib_device V2

On 10/20/2015 5:08 PM, Or Gerlitz wrote: On Tue, Oct 20, 2015 at 4:08 PM, Sagi Grimberg wrote: I think this is very useful to have and I'd love having it in 4.4, does anyone have any other comments on this patch? Were we ever presented with performance gains

Re: merge struct ib_device_attr into struct ib_device V2

I think this is very useful to have and I'd love having it in 4.4, does anyone have any other comments on this patch? Were we ever presented with performance gains achieved using the patch? Hi Or, Can you explain what you mean by performance gains? My understanding is that this patch is

Re: merge struct ib_device_attr into struct ib_device V2

On Tue, Oct 20, 2015 at 04:08:54PM +0300, Sagi Grimberg wrote: > Can you explain what you mean by performance gains? My understanding is > that this patch is not performance critical. It just replaces each and > every ULP device attributes caching. Exactly. The only 'performance' we care about

[PATCH] IB: merge struct ib_device_attr into struct ib_device

Avoid the need to query for device attributes and store them in a separate structure by merging struct ib_device_attr into struct ib_device. This matches how the device structures are used in most Linux subsystems. Signed-off-by: Christoph Hellwig --- drivers/infiniband/core/cm.c

merge struct ib_device_attr into struct ib_device V2

This patch gets rid of struct ib_device_attr and cleans up drivers nicely. It goes on top of my send_wr cleanups and the memory registration udpates from Sagi. Changes since V1: - rebased on top of the Sagi's latest reg_api.6 branch -- To unsubscribe from this list: send the line "unsubscribe

Re: merge struct ib_device_attr into struct ib_device V2

On 10/12/2015 9:57 AM, Christoph Hellwig wrote: This patch gets rid of struct ib_device_attr and cleans up drivers nicely. It goes on top of my send_wr cleanups and the memory registration udpates from Sagi. Changes since V1: - rebased on top of the Sagi's latest reg_api.6 branch

Re: merge struct ib_device_attr into struct ib_device V2

On Mon, Oct 12, 2015 at 12:26:06PM +0300, Sagi Grimberg wrote: > First go with this looks OK for mlx4. mlx5 needs the below incremental > patch to be folded in. > > we need dev->ib_dev.max_pkeys set when get_port_caps() is called. Thanks, I've folded your patch and force pushed out the updated

Re: [PATCH] IB: merge struct ib_device_attr into struct ib_device

On Sun, Oct 11, 2015 at 07:16:27PM +0300, Sagi Grimberg wrote: > Christoph, would you mind rebasing it on top of 4.3-rc4 or so? I > want to develop over it so I can test it on the fly. I can do a rebase to whatever you want. Initially this was over your reg_api branch. Is a rebase to the latest

Re: [PATCH] IB: merge struct ib_device_attr into struct ib_device

On 9/24/2015 4:35 PM, Christoph Hellwig wrote: On Thu, Sep 24, 2015 at 08:41:17AM +0300, Or Gerlitz wrote: We had a smaller volume move to cache the device attributes on the IB device structure, and I just realized it was dropped on the floor. Ira, that was a reviewer comment you got when

Re: [PATCH] IB: merge struct ib_device_attr into struct ib_device

I can do a rebase to whatever you want. Initially this was over your reg_api branch. Is a rebase to the latest reg_api.6 fine? That would be great. Sagi. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More

Re: [PATCH] IB: merge struct ib_device_attr into struct ib_device

Christoph, We had a smaller volume move to cache the device attributes on the IB device structure, and I just realized it was dropped on the floor. Ira, that was a reviewer comment you got when worked on OPA and I missed the fact it didn't reach to acceptance

RE: message size, was Re: merge struct ib_device_attr into struct ib_device

> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Tuesday, September 22, 2015 6:08 PM > > > How do we change the message size limits? Reviewing w/o it being > > inline is painful for the (many) reviewers... > > I've increased it. Thanks! -- To

[PATCH] IB: merge struct ib_device_attr into struct ib_device

Avoid the need to query for device attributes and store them in a separate structure by merging struct ib_device_attr into struct ib_device. This matches how the device structures are used in most Linux subsystems. Signed-off-by: Christoph Hellwig --- drivers/infiniband/core/cm.c

Re: [PATCH] IB: merge struct ib_device_attr into struct ib_device

Hey Christoph- > On Sep 23, 2015, at 8:52 AM, Christoph Hellwig wrote: > > Avoid the need to query for device attributes and store them in a > separate structure by merging struct ib_device_attr into struct > ib_device. This matches how the device structures are used in most >

Re: [PATCH] IB: merge struct ib_device_attr into struct ib_device

On Wed, Sep 23, 2015 at 10:23:15AM -0700, Chuck Lever wrote: > Getting rid of ib_query_device() makes sense. Moving the device > attributes into ib_device is nice. Getting rid of ib_device_attr > is of questionable value. Why do we need to go there? > > IB core API changes generate merge

Re: message size, was Re: merge struct ib_device_attr into struct ib_device

Hi, Le mardi 22 septembre 2015 à 23:55 +0200, 'Christoph Hellwig' a écrit : > On Tue, Sep 22, 2015 at 04:06:17PM -0500, Steve Wise wrote: > > Can you create a series of smaller patches that will fit on the > > list? > > That would make it easier for everyone to review/comment. > > I don't see

Re: [PATCH] IB: merge struct ib_device_attr into struct ib_device

On 9/23/2015 9:41 PM, Christoph Hellwig wrote: On Wed, Sep 23, 2015 at 10:23:15AM -0700, Chuck Lever wrote: Getting rid of ib_query_device() makes sense. Moving the device attributes into ib_device is nice. Getting rid of ib_device_attr is of questionable value. Why do we need to go there? IB

Re: merge struct ib_device_attr into struct ib_device

Hi Yann, looks like the patch was too large and majordomo ate it Here is a link: http://git.infradead.org/users/hch/rdma.git/commitdiff/0e46553467cd01b63ab9c985f87c18c5328880bb -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to

RE: merge struct ib_device_attr into struct ib_device

> -Original Message- > From: linux-rdma-ow...@vger.kernel.org > [mailto:linux-rdma-ow...@vger.kernel.org] On Behalf Of Christoph Hellwig > Sent: Tuesday, September 22, 2015 3:32 PM > To: Yann Droneaud > Cc: linux-rdma@vger.kernel.org > Subject: Re: merge

message size, was Re: merge struct ib_device_attr into struct ib_device

On Tue, Sep 22, 2015 at 04:06:17PM -0500, Steve Wise wrote: > Can you create a series of smaller patches that will fit on the list? > That would make it easier for everyone to review/comment. I don't see how that is possible, as it's a flag day change. But maybe we really need to bump up the

Re: message size, was Re: merge struct ib_device_attr into struct ib_device

From: "Steve Wise" Date: Tue, 22 Sep 2015 17:15:15 -0500 > How do we change the message size limits? Reviewing w/o it being > inline is painful for the (many) reviewers... I've increased it. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma"

Re: merge struct ib_device_attr into struct ib_device

Le lundi 21 septembre 2015 à 13:59 -0700, Christoph Hellwig a écrit : > This patch gets rid of struct ib_device_attr and cleans up drivers > nicely. > > It goes on top of my send_wr cleanups and the memory registration > udpates > from Sagi. > Is the patch missing ? Regards. -- Yann Droneaud

merge struct ib_device_attr into struct ib_device

This patch gets rid of struct ib_device_attr and cleans up drivers nicely. It goes on top of my send_wr cleanups and the memory registration udpates from Sagi. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More