Re: merge struct ib_device_attr into struct ib_device V2

[Sagi Grimberg]

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: merge struct ib_device_attr into struct ib_device V2

[Bart Van Assche]

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 ib_query_device() seems a great idea to me and an idea that I 
welcome very much. The ib_device_attr structure is too large to be 
allocated on the stack. This means that with Christoph's patch it is no 
longer needed to call kmalloc() + ib_query_device() + kfree() when a 
device attribute is needed from kernel code.


Bart.
--
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: merge struct ib_device_attr into struct ib_device V2

[Jason Gunthorpe]

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 not yet had the time to review the entire patch, removing
> ib_query_device() seems a great idea to me and an idea that I welcome very
> much. The ib_device_attr structure is too large to be allocated on the
> stack. This means that with Christoph's patch it is no longer needed to call
> kmalloc() + ib_query_device() + kfree() when a device attribute is needed
> from kernel code.

I agree, this is absolutely the right way to go.

The bikeshedding is not important, nobody has come up with a reason
why we need to maintain the attr structure as-is and Christoph already
has a patch - I say go with it.

Jason
--
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: merge struct ib_device_attr into struct ib_device V2

[Steve Wise]

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
hear more opinions.

Although I have not yet had the time to review the entire patch, removing
ib_query_device() seems a great idea to me and an idea that I welcome very
much. The ib_device_attr structure is too large to be allocated on the
stack. This means that with Christoph's patch it is no longer needed to call
kmalloc() + ib_query_device() + kfree() when a device attribute is needed
from kernel code.

I agree, this is absolutely the right way to go.

The bikeshedding is not important, nobody has come up with a reason
why we need to maintain the attr structure as-is and Christoph already
has a patch - I say go with it.

Jason



While I don't really like the uber patch review-wise, I'm all for nuking 
ib_query_device() and the attr struct.


Steve.
--
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: merge struct ib_device_attr into struct ib_device V2

[Or Gerlitz]

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 opinions.
>
> Although I have not yet had the time to review the entire patch, removing
> ib_query_device() seems a great idea to me and an idea that I welcome very
> much. The ib_device_attr structure is too large to be allocated on the
> stack. This means that with Christoph's patch it is no longer needed to call
> kmalloc() + ib_query_device() + kfree() when a device attribute is needed
> from kernel code.


Bart, Jason, Steve,

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 pointer) which is filled by the device driver before
they register themselves with the IB core.

This way there's no need for kmalloc() + ib_query_device() + kfree()
when a device attribute is needed and we don't introduce tens/hundreds
new fields for struct ib_device.

Thoughts?


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: merge struct ib_device_attr into struct ib_device V2

[Or Gerlitz]

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 kernel developers, put the no pointer member @ the
end

> I have a slight preference toward ibdev->foo to match other subsystems.

I have a slight preference to keep the struct ib_device_attr notion to
match other portions of the RDMA stack such as the UAPI and the user
space API to applications.

> I have no idea why you care so much about something so trivial.

I don't care so much, I have the small right to speak without getting
so much mud on my head.

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: merge struct ib_device_attr into struct ib_device V2

[Jason Gunthorpe]

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 pointer) which is filled by the device driver before
> they register themselves with the IB core.

No to the pointer idea.

> This way there's no need for kmalloc() + ib_query_device() + kfree()
> when a device attribute is needed and we don't introduce tens/hundreds
> new fields for struct ib_device.

I care very little if the name is ibdev->attr.foo or ibdev->foo, that is
just bikeshedding.

I have a slight preference toward ibdev->foo to match other subsystems.

I have no idea why you care so much about something so trivial.

Jason
--
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: merge struct ib_device_attr into struct ib_device V2

[Christoph Hellwig]

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 of the structure.  Maybe it's time for a
little cache line 101 primer?
--
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: merge struct ib_device_attr into struct ib_device V2

[Christoph Hellwig]

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 structure?
>
> Keeping struct ib_device_attr embedded at the end of struct ib_device is
> exactly the same isn't it?

In terms of layout it is, in terms of usability it's worse.  Just to take
the networking example Or seems to like so much there is no struct netdev_attr
to which fields of struct netdev are moved out to, which you then need to
query and cache anyway in your private device structure.   This design simply
was a idiotic idea to start with and I don't understand why anyone would
han onto it.  I do understand Chuck's concern about churn, but I'd rather
fix the infiniband subsystem up before we're growing even more users.
--
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: merge struct ib_device_attr into struct ib_device V2

[Or Gerlitz]

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 and friends.

But it doesn't get near the end of the structure.  Maybe it's time for a
little cache line 101 primer?


Oh, not that I am fully qualified on that front, but I understand  the 
end-of-structure argument.


Fact is that for struct net_device you will not add 333 new fields over 
night in the coming 33 years, for sure.


This makes much sense to me, as a guideline. I don't think we should 
have a device structure with the current
100 fields and another few hundreds without any notable benefit and 
against the common practice in the networking

subsystem.

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 to cache an device attr 
pointer on the device structure.


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: merge struct ib_device_attr into struct ib_device V2

[Christoph Hellwig]

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 netdev is that it move all
the function pointers out to a different structure so it could be kept
const.  This introduces one more level of pointer chasing but has other
advantages.

> This makes much sense to me, as a guideline. I don't think we should have a 
> device structure with the current
> 100 fields and another few hundreds without any notable benefit and against 
> the common practice in the networking
> subsystem.

Please understand the networking subsystem (or SCSI, or NVMe, or ...) before
making such incorrect comments.  We're moving towards how all other
subsystems work.

> 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 to cache an device attr pointer 
> on the device structure.

Take a look at the code.  The only time we ever call into ->query_device is
for the userspace only timestamping extensions only implemented for mlx4.

With all the stuff we have on the plate the kernel API will look substatially
different from the arkane 'Verbs' implementation in userspace, and they will
use less and less common code.  Libuvers and the ABIs it uses are something
we can't change unfortunately, but we can make the kernel API much much
better by learining lessons from other kernel subsystems.  That's work
that Jason Sagi and me have been doing for a while.

I'd also suggest you update your Linux knowledge before trying to
micromanage patch submissions.
--
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: merge struct ib_device_attr into struct ib_device V2

[Or Gerlitz]

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 only time we ever call into ->query_device is
for the userspace only timestamping extensions only implemented for mlx4.


We will have many more device query extensions, 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 
UAPI, and don't see any reason to
avoid it in the kernel, just because some other subsystems do that, 
according to your view. As I said, you
would have to work real (real) hard to convince the networking ppl to 
add fields to struct net_device sk_buff
and friends... the nature of rdma/offloading is such that new attr come 
often and I don't think we need

to touch our device struct every release.



With all the stuff we have on the plate the kernel API will look substatially
different from the arkane 'Verbs' implementation in userspace, and they will
use less and less common code.  Libuvers and the ABIs it uses are something
we can't change unfortunately, but we can make the kernel API much much
better by learining lessons from other kernel subsystems.  That's work
that Jason Sagi and me have been doing for a while.

I'd also suggest you update your Linux knowledge before trying to
micromanage patch submissions.


not trying to micro-manage @ all, I went over the archives and 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. Re my education -- can you make the argument 
more precise: what are we

making much much better in the kernel API by thins house moving exercise?

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: merge struct ib_device_attr into struct ib_device V2

[Christoph Hellwig]

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 UAPI, 

It's an entirely separate ib_uverbs_query_device_resp structure. 

> and don't see any reason to
> avoid it in the kernel, just because some other subsystems do that, 
> according to your view. As I said, you
> would have to work real (real) hard to convince the networking ppl to add 
> fields to struct net_device sk_buff

Or, please stop these bullshit strawman arguments.  Yes, adding fields to
struct sk_buff will be hard, but that's not the right comparism.  struct
sk_buff is a structured allocated for the data buffers in great quantities and
not the net_device structure allocated once per device.  I'm going to
finish this email, but if you don't even try to get your facts right I'll stop
responding ro you because it's futile.

> and friends... the nature of rdma/offloading is such that new attr come 
> often and I don't think we need
> to touch our device struct every release.

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 structure, and iser, isert, 
srpt and nfs cache the full structure, so if you use one of those you're
already having one of them per device.  If you use two of them you
have multiple copies plus individual fields caches by other ULDs and core
code.
--
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: merge struct ib_device_attr into struct ib_device V2

[Or Gerlitz]

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 structure, and iser, isert,
srpt and nfs cache the full structure,


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 having one of them per device.  If you use two of them you
have multiple copies plus individual fields caches by other ULDs and core
code.


again, the method I propose does remove these duplications all together.

I am still waiting to hear what is the precise argument for having this 
change.


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: merge struct ib_device_attr into struct ib_device V2

[Christoph Hellwig]

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 having one of them per device.  If you use two of them you
>> have multiple copies plus individual fields caches by other ULDs and core
>> code.
>
> again, the method I propose does remove these duplications all together.
>
> I am still waiting to hear what is the precise argument for having this 
> change.

It does everything the pointer does, but in addition saves memory (no
rounding up of the slab size), reduces pointer chasing and makes and API
that's easier to use and more similar to how all other major Linux subsystems
work.
--
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: merge struct ib_device_attr into struct ib_device V2

[Or Gerlitz]

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 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.

oops, sorry, I was referring to the patch that deals with
re-structuring of struct ib_wc, so...

(1) did we even got performance gains achieved using **that** patch?

(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 benefit from this patch.
--
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: merge struct ib_device_attr into struct ib_device V2

[Sagi Grimberg]

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 patch?
--
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: merge struct ib_device_attr into struct ib_device V2

[Or Gerlitz]

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 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?

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: merge struct ib_device_attr into struct ib_device V2

[Sagi Grimberg]

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 affect the fast path - am I missing something?



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 structure?


Keeping struct ib_device_attr embedded at the end of struct ib_device is
exactly the same isn't 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: merge struct ib_device_attr into struct ib_device V2

[Or Gerlitz]

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 benefit from this patch.

> Christoph commented on that:
>
> "I'm strongly against this.  As the reviews show the move is highly
> confusing.  The attributes don't change and there is no need to 'cache'
> or 'query' them.  Just merge them into the device, follow years of
> experience with how every other Linux subsystem does it and be done
> with it."
>
> If the attributes are automatically a part of the ib_device then it
> does seem a bit redundant keeping the structure around...

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.

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 structure?

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: merge struct ib_device_attr into struct ib_device V2

[Or Gerlitz]

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 fields to our device structure?

> Keeping struct ib_device_attr embedded at the end of struct ib_device is
> exactly the same isn't it?

I said " (best with pointer)"
--
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: merge struct ib_device_attr into struct ib_device V2

[Sagi Grimberg]

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 achieved using the patch?



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.


oops, sorry, I was referring to the patch that deals with
re-structuring of struct ib_wc, so...

(1) did we even got performance gains achieved using **that** patch?


I don't know if we'd see noticeable performance gains from **that**
patch either.. The benefit is mostly stack relief as usually ULPs keep
the work request structure on the stack.



(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 benefit from this patch.



Christoph commented on that:

"I'm strongly against this.  As the reviews show the move is highly
confusing.  The attributes don't change and there is no need to 'cache'
or 'query' them.  Just merge them into the device, follow years of
experience with how every other Linux subsystem does it and be done
with it."

If the attributes are automatically a part of the ib_device then it
does seem a bit redundant keeping the structure around...
--
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: merge struct ib_device_attr into struct ib_device V2

[Sagi Grimberg]

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 not performance critical. It just replaces each and
every ULP device attributes caching.
--
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: merge struct ib_device_attr into struct ib_device V2

[Christoph Hellwig]

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 is that of ULD writers :)
--
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: merge struct ib_device_attr into struct ib_device V2

[Sagi Grimberg]

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



Christoph,

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.

--
diff --git a/drivers/infiniband/hw/mlx5/main.c 
b/drivers/infiniband/hw/mlx5/main.c

index 67b979f..5b73322 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -1321,6 +1321,10 @@ static void *mlx5_ib_add(struct mlx5_core_dev *mdev)

dev->mdev = mdev;

+   err = mlx5_ib_init_device_flags(>ib_dev);
+   if (err)
+   goto err_dealloc;
+
err = get_port_caps(dev);
if (err)
goto err_dealloc;
@@ -1433,10 +1437,6 @@ static void *mlx5_ib_add(struct mlx5_core_dev *mdev)
if (err)
goto err_rsrc;

-   err = mlx5_ib_init_device_flags(>ib_dev);
-   if (err)
-   goto err_rsrc;
-
err = ib_register_device(>ib_dev, NULL);
if (err)
goto err_odp;
--
--
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: merge struct ib_device_attr into struct ib_device V2

[Christoph Hellwig]

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 tree.
--
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