Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-11 Thread Christoph Hellwig
On Wed, Nov 11, 2015 at 11:09:03AM +0200, Sagi Grimberg wrote: > It does, but it doesn't look like something we'd want to check for each > IO... Both potential callers already have a access flags variable in object that's assigned to at setup time so I don't see a problem here. -- To unsubscribe

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-11 Thread Sagi Grimberg
On 10/11/2015 15:41, Christoph Hellwig wrote: FYI, this is the API I'd aim for (only SRP and no HW driver converted yet): This looks fine, although personally I find scope and direction flags more confusing than access_flags (but maybe it's just me). I think that the real issue here is the

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-11 Thread Sagi Grimberg
On 11/11/2015 10:08, Christoph Hellwig wrote: On Tue, Nov 10, 2015 at 11:01:56AM -0700, Jason Gunthorpe wrote: No need to change every driver. I'd suggest something like unsigned int rdma_cap_rdma_read_mr_flags(const struct ib_pd *pd) { if (rdma_protocol_iwarp(pd->device,

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-11 Thread Christoph Hellwig
On Wed, Nov 11, 2015 at 11:07:14AM +0200, Sagi Grimberg wrote: > > > On 10/11/2015 15:41, Christoph Hellwig wrote: > >FYI, this is the API I'd aim for (only SRP and no HW driver converted > >yet): > > This looks fine, although personally I find scope and direction flags > more confusing than

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-11 Thread Christoph Hellwig
On Tue, Nov 10, 2015 at 11:36:27AM -0700, Jason Gunthorpe wrote: > > n = ib_map_mr_sg(desc->mr, state->sg, state->sg_nents, > > -dev->mr_page_size); > > +dev->mr_page_size, > > +/* > > + * XXX: add a bool write

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-11 Thread Christoph Hellwig
On Tue, Nov 10, 2015 at 11:01:56AM -0700, Jason Gunthorpe wrote: > No need to change every driver. > > I'd suggest something like > > unsigned int rdma_cap_rdma_read_mr_flags(const struct ib_pd *pd) > { > if (rdma_protocol_iwarp(pd->device, rdma_start_port(pd->device))) >

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-10 Thread Jason Gunthorpe
On Tue, Nov 10, 2015 at 12:51:10PM +0100, Yann Droneaud wrote: > Why were those hw providers not modified to > enforce IB_ACCESS_REMOTE_WRITE when needed, instead of asking users to > set it for them ? iWarp hardware simply cannot do it it all. Jason -- To unsubscribe from this list: send the

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-10 Thread Jason Gunthorpe
On Tue, Nov 10, 2015 at 12:44:13PM +0200, Sagi Grimberg wrote: > Different devices may require different access permissions > for rdma reads e.g. for Infiniband devices, local write access > suffices while iWARP devices require remote write permissions as > well. > > This situation generates

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-10 Thread Jason Gunthorpe
On Tue, Nov 10, 2015 at 05:41:47AM -0800, Christoph Hellwig wrote: > FYI, this is the API I'd aim for (only SRP and no HW driver converted > yet): > n = ib_map_mr_sg(desc->mr, state->sg, state->sg_nents, > - dev->mr_page_size); > +

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-10 Thread Sagi Grimberg
On 10/11/2015 14:28, Sagi Grimberg wrote: Hi Yann, Why were those hw providers not modified to enforce IB_ACCESS_REMOTE_WRITE when needed, instead of asking users to set it for them ? Do you mean that ULPs will set IB_ACCESS_LOCAL_WRITE and iWARP providers executing the memory

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-10 Thread Tom Talpey
On 11/10/2015 6:51 AM, Yann Droneaud wrote: Le mardi 10 novembre 2015 à 12:44 +0200, Sagi Grimberg a écrit : Also, set rdma_read_access_flags in the relevant device drivers: mlx4, mlx5, qib, ocrdma, nes: IB_ACCESS_LOCAL_WRITE cxgb3, cxgb4, hfi: IB_ACCESS_LOCAL_WRITE | IB_ACCESS_REMOTE_WRITE

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-10 Thread Sagi Grimberg
Sagi, the Windows NDKPI has an NDK_MR_FLAG_RDMA_READ_SINK attribute which the upper layer can use to convey this information, I've mentioned it here before. https://msdn.microsoft.com/en-us/library/windows/hardware/hh439908(v=vs.85).aspx Thanks for the tip Tom. When this approach is used,

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-10 Thread Christoph Hellwig
On Tue, Nov 10, 2015 at 12:44:13PM +0200, Sagi Grimberg wrote: > Different devices may require different access permissions > for rdma reads e.g. for Infiniband devices, local write access > suffices while iWARP devices require remote write permissions as > well. > > This situation generates

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-10 Thread Sagi Grimberg
From all I can tell nes also is a iWarp driver. It is.. I don't know why I treated it as IB :) -- 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

[PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-10 Thread Sagi Grimberg
Different devices may require different access permissions for rdma reads e.g. for Infiniband devices, local write access suffices while iWARP devices require remote write permissions as well. This situation generates extra logic for ULPs that need to be aware of the underlying device to

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-10 Thread Yann Droneaud
Hi, Le mardi 10 novembre 2015 à 12:44 +0200, Sagi Grimberg a écrit : > Different devices may require different access permissions > for rdma reads e.g. for Infiniband devices, local write access > suffices while iWARP devices require remote write permissions as > well. > > This situation

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-10 Thread Sagi Grimberg
Hi Yann, Why were those hw providers not modified to enforce IB_ACCESS_REMOTE_WRITE when needed, instead of asking users to set it for them ? Do you mean that ULPs will set IB_ACCESS_LOCAL_WRITE and iWARP providers executing the memory registration will add IB_ACCESS_REMOTE_WRITE? That's

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-10 Thread Christoph Hellwig
On Tue, Nov 10, 2015 at 02:42:44PM +0200, Sagi Grimberg wrote: > >When this approach is used, the upper layer doesn't have to be aware > >at all of the lower layer's details. > > Yea, we could do that too. Any preferences from other people? > I'm pretty indifferent on which way to go... Yes,