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