Re: [PATCH 37/37] IB/rdmavt: Add support for new memory registration API

2015-12-17 Thread Christoph Hellwig
On Thu, Dec 17, 2015 at 10:52:29AM -0500, Dennis Dalessandro wrote:
> I am not opposed to leaving the code in rdmavt. It gets removed from qib in
> the other patch series I posted. My preference is to leave it in rdmavt
> since it will be needed down the road. However I can go either way here, its
> easy to add back later.

Without setting IB_DEVICE_MEM_MGT_EXTENSIONS and implementing all the
features required for it it's dead code.  There is no point to keep
it 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: [PATCH 37/37] IB/rdmavt: Add support for new memory registration API

2015-12-17 Thread Dennis Dalessandro

On Thu, Dec 17, 2015 at 08:35:53AM -0800, Christoph Hellwig wrote:

On Thu, Dec 17, 2015 at 10:52:29AM -0500, Dennis Dalessandro wrote:

I am not opposed to leaving the code in rdmavt. It gets removed from qib in
the other patch series I posted. My preference is to leave it in rdmavt
since it will be needed down the road. However I can go either way here, its
easy to add back later.


Without setting IB_DEVICE_MEM_MGT_EXTENSIONS and implementing all the
features required for it it's dead code.  There is no point to keep
it around.


Ok, sounds good to me. I will drop this patch when I post the v2.

-Denny
--
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: [PATCH 37/37] IB/rdmavt: Add support for new memory registration API

2015-12-17 Thread Dennis Dalessandro

On Wed, Dec 16, 2015 at 04:37:39PM +0200, Sagi Grimberg wrote:



This patch exists to provide parity for what is in qib. Should we not
have it?  If not, why do we have:

commit 38071a461f0a ("IB/qib: Support the new memory registration API")


That was done by me because I saw this in qib and assumed that it was
supported. Now that I found out that it isn't, I'd say it should be
removed altogether shouldn't it?


I am not opposed to leaving the code in rdmavt. It gets removed from qib in 
the other patch series I posted. My preference is to leave it in rdmavt 
since it will be needed down the road. However I can go either way here, its 
easy to add back later.





That doesn't mean it can't be added to rdmavt as a future enhancement
though if there is a need.


Well, given that we're trying to consolidate on post send registration
interface it's kind of a must I'd say.


Are you asking because soft-roce will need it?


I was asking in general, but in specific soft-roce as a consumer will
need to support that yes.


I think it makes sense to revisit when soft-roce comes in,


I agree.


since qib/hfi do not need IB_WR_LOCAL_INV.


Can you explain? Does qib/hfi have a magic way to invalidate memory
regions?


Hfi1 does not currently have any support for memory registration in its post 
send. Qib had the "old FRWR API" support in post_send, which you removed 
since according to the commit message, is not used anymore.


I suppose a follow on patch to your "new memory registration API" patch 
would be needed to add the invalidate. This is the piece I think can be 
added later in rdmavt.


-Denny
--
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: [PATCH 37/37] IB/rdmavt: Add support for new memory registration API

2015-12-16 Thread Dennis Dalessandro

On Wed, Dec 16, 2015 at 03:21:02PM +0200, Sagi Grimberg wrote:



This question is not directly related to this patch, but given that
this is a copy-paste from the qib driver I'll go ahead and take it
anyway. How does qib (and rvt now) do memory key invalidation? I didn't
see any reference to IB_WR_LOCAL_INV anywhere in the qib driver...

What am I missing?


ping?


In short, it doesn't look like qib or hfi1 support this.


Oh, I'm surprised to learn that. At least I see that
qib is not exposing IB_DEVICE_MEM_MGT_EXTENSIONS. But whats
the point in doing something with a IB_WR_REG_MR at all?



Given that this is not supported anyway, why does this patch
exist?


This patch exists to provide parity for what is in qib. Should we not have 
it?  If not, why do we have:


commit 38071a461f0a ("IB/qib: Support the new memory registration API")


That doesn't mean it can't be added to rdmavt as a future enhancement
though if there is a need.


Well, given that we're trying to consolidate on post send registration
interface it's kind of a must I'd say.


Are you asking because soft-roce will need it?


I was asking in general, but in specific soft-roce as a consumer will
need to support that yes.


I think it makes sense to revisit when soft-roce comes in, since qib/hfi do 
not need IB_WR_LOCAL_INV.


-Denny
--
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: [PATCH 37/37] IB/rdmavt: Add support for new memory registration API

2015-12-16 Thread Sagi Grimberg



This patch exists to provide parity for what is in qib. Should we not
have it?  If not, why do we have:

commit 38071a461f0a ("IB/qib: Support the new memory registration API")


That was done by me because I saw this in qib and assumed that it was
supported. Now that I found out that it isn't, I'd say it should be
removed altogether shouldn't it?



That doesn't mean it can't be added to rdmavt as a future enhancement
though if there is a need.


Well, given that we're trying to consolidate on post send registration
interface it's kind of a must I'd say.


Are you asking because soft-roce will need it?


I was asking in general, but in specific soft-roce as a consumer will
need to support that yes.


I think it makes sense to revisit when soft-roce comes in,


I agree.


since qib/hfi do not need IB_WR_LOCAL_INV.


Can you explain? Does qib/hfi have a magic way to invalidate memory
regions?
--
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: [PATCH 37/37] IB/rdmavt: Add support for new memory registration API

2015-12-16 Thread Sagi Grimberg



This question is not directly related to this patch, but given that
this is a copy-paste from the qib driver I'll go ahead and take it
anyway. How does qib (and rvt now) do memory key invalidation? I didn't
see any reference to IB_WR_LOCAL_INV anywhere in the qib driver...

What am I missing?


ping?


In short, it doesn't look like qib or hfi1 support this.


Oh, I'm surprised to learn that. At least I see that
qib is not exposing IB_DEVICE_MEM_MGT_EXTENSIONS. But whats
the point in doing something with a IB_WR_REG_MR at all?

Given that this is not supported anyway, why does this patch
exist?


That doesn't mean it can't be added to rdmavt as a future enhancement
though if there is a need.


Well, given that we're trying to consolidate on post send registration
interface it's kind of a must I'd say.


Are you asking because soft-roce will need it?


I was asking in general, but in specific soft-roce as a consumer will
need to support that yes.
--
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: [PATCH 37/37] IB/rdmavt: Add support for new memory registration API

2015-12-14 Thread Sagi Grimberg




Hi Dennis and Ira,

This question is not directly related to this patch, but given that
this is a copy-paste from the qib driver I'll go ahead and take it
anyway. How does qib (and rvt now) do memory key invalidation? I didn't
see any reference to IB_WR_LOCAL_INV anywhere in the qib driver...

What am I missing?


ping?
--
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: [PATCH 37/37] IB/rdmavt: Add support for new memory registration API

2015-12-14 Thread Dennis Dalessandro

On Mon, Dec 14, 2015 at 06:18:48PM +0200, Sagi Grimberg wrote:

This question is not directly related to this patch, but given that
this is a copy-paste from the qib driver I'll go ahead and take it
anyway. How does qib (and rvt now) do memory key invalidation? I didn't
see any reference to IB_WR_LOCAL_INV anywhere in the qib driver...

What am I missing?


ping?


In short, it doesn't look like qib or hfi1 support this.

That doesn't mean it can't be added to rdmavt as a future enhancement though 
if there is a need. Are you asking because soft-roce will need it?


-Denny
--
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: [PATCH 37/37] IB/rdmavt: Add support for new memory registration API

2015-12-10 Thread Sagi Grimberg



commit 38071a461f0a ("IB/qib: Support the new memory registration API")"
added support for map_mr_sg to qib. This patch brings that
support into rdmavt, it also adds a register mr function that will be
called from the post send routine which is inline with the qib patch.


Hi Dennis and Ira,

This question is not directly related to this patch, but given that
this is a copy-paste from the qib driver I'll go ahead and take it
anyway. How does qib (and rvt now) do memory key invalidation? I didn't
see any reference to IB_WR_LOCAL_INV anywhere in the qib driver...

What am I missing?
--
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