[PATCH V3 0/2] Add support for enhanced atomic operations

[PATCH V3 0/2] Add support for enhanced atomic operations

[Vladimir Sokolovsky]

Hi Roland,

This patchset adds support for the following enhanced atomic
operations:
- Masked atomic compare and swap
- Masked atomic fetch and add

These operations enable using a smaller amount of memory when using
multiple locks by using portions of a 64 bit value in an atomic
operation.
For some applications the memory savings are very significant. One
example is fine grain lock implementations for huge data sets. In
other cases, the benefit is the ability to update multiple fields with
a single io operation.

Vladimir Sokolovsky(2):
IB/core: Add support for enhanced atomic operations
mlx4/IB: Add support for enhanced atomic operations

changes from V2:
- patch #1: 
  Updated description
  Renamed:
        IB_WR_ATOMIC_MASKED_CMP_AND_SWP -> IB_WR_MASKED_ATOMIC_CMP_AND_SWP
        IB_WR_ATOMIC_MASKED_FETCH_AND_ADD -> IB_WR_MASKED_ATOMIC_FETCH_AND_ADD
  In the ib_send_wr struct the new fields added before the rkey field

- patch #2:
  Set IB_DEVICE_MASKED_ATOMIC flag with other flags that get set for
  all devices

Regards,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/2] Add support for enhanced atomic operations

[Håkon Bugge]

Hi Vlad,

Did you consider my input in http://www.mail-archive.com/linux-rdma@vger.kernel.org/msg02803.html wrt. to these enhancements?

 

Thanks, Håkon

On Mar 10, 2010, at 16:57 , Vladimir Sokolovsky wrote:

> Hi Roland,
> 
> This patchset adds support for the following enhanced atomic
> operations:
> - Masked atomic compare and swap
> - Masked atomic fetch and add
> 
> These operations enable using a smaller amount of memory when using
> multiple locks by using portions of a 64 bit value in an atomic
> operation.
> For some applications the memory savings are very significant. One
> example is fine grain lock implementations for huge data sets. In
> other cases, the benefit is the ability to update multiple fields with
> a single io operation.
> 
> Vladimir Sokolovsky(2):
> IB/core: Add support for enhanced atomic operations
> mlx4/IB: Add support for enhanced atomic operations
> 
> changes from V2:
> - patch #1: 
>  Updated description
>  Renamed:
>        IB_WR_ATOMIC_MASKED_CMP_AND_SWP -> IB_WR_MASKED_ATOMIC_CMP_AND_SWP
>        IB_WR_ATOMIC_MASKED_FETCH_AND_ADD -> IB_WR_MASKED_ATOMIC_FETCH_AND_ADD
>  In the ib_send_wr struct the new fields added before the rkey field
> 
> - patch #2:
>  Set IB_DEVICE_MASKED_ATOMIC flag with other flags that get set for
>  all devices
> 
> Regards,
> Vladimir
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Håkon Bugge
Haakon.bugge-U0mLk4xYmo8@public.gmane.org
+47 924 84 514



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/2] Add support for enhanced atomic operations

[Roland Dreier]

 > Did you consider my input in http://www.mail-archive.com/linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg02803.html wrt. to these enhancements?

I think we can worry about that if/when an HCA comes along that supports
global atomics for ordinary atomics but not enhanced atomics.  Although
perhaps it would be cleaner to change the atomic_cap enum to:

	/*
	 * IB_ATOMIC_NONE:	no atomic capability
	 * IB_ATOMIC_HCA:	all ops are atomic within HCA
	 * IB_ATOMIC_GLOB:	standard ops atomic with respect to all
				memory ops; masked ops atomic within HCA
	 * IB_ATOMIC_GLOB_MASKED: all ops atomic with respect to all
	 *			memory ops
	 */
	enum ib_atomic_cap {
		IB_ATOMIC_NONE,
		IB_ATOMIC_HCA,
		IB_ATOMIC_GLOB,
		IB_ATOMIC_GLOB_MASKED
	};

(with better wording for the comments)
Thoughts?

 - R.
-- 
Roland Dreier  <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/2] Add support for enhanced atomic operations

[Håkon Bugge]

On Mar 11, 2010, at 19:59 , Roland Dreier wrote:
> I think we can worry about that if/when an HCA comes along that supports
> global atomics for ordinary atomics but not enhanced atomics.

With the proposed patches in place, how do you know if masked atomics are implemented or not? Guess apps need to know this information already on todays HCAs.

> Although perhaps it would be cleaner to change the atomic_cap enum to:
> 
> 	/*
> 	 * IB_ATOMIC_NONE:	no atomic capability
> 	 * IB_ATOMIC_HCA:	all ops are atomic within HCA

But IB_ATOMIC_HCA  does not tell you if the masked ones are supported or not.

> 	 * IB_ATOMIC_GLOB:	standard ops atomic with respect to all
> 				memory ops; masked ops atomic within HCA

What if an HCA supports standards ops with respect to all memory ops, but the HCA does not support masked atomics?

Hence, I think it would be cleaner if a new capability, masked_atomic_cap, were introduced, using the original definitions (NONE, HCA, GLOB).


Thanks, Håkon


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/2] Add support for enhanced atomic operations

[Roland Dreier]

 > With the proposed patches in place, how do you know if masked atomics
 > are implemented or not? Guess apps need to know this information
 > already on todays HCAs.

>From the patch:

 > @@ -105,6 +105,7 @@ enum ib_device_cap_flags {
 >  	IB_DEVICE_UD_TSO		= (1<<19),
 >  	IB_DEVICE_MEM_MGT_EXTENSIONS	= (1<<21),
 >  	IB_DEVICE_BLOCK_MULTICAST_LOOPBACK = (1<<22),
 > +	IB_DEVICE_MASKED_ATOMIC		= (1<<23),
 >  };

 > Hence, I think it would be cleaner if a new capability,
 > masked_atomic_cap, were introduced, using the original definitions
 > (NONE, HCA, GLOB).

Actually that does seem more orthogonal to me.
-- 
Roland Dreier  <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/2] Add support for enhanced atomic operations

[Vladimir Sokolovsky]

Håkon Bugge wrote:
> Hi Vlad,
> 
> Did you consider my input in http://www.mail-archive.com/linux-rdma@vger.kernel.org/msg02803.html wrt. to these enhancements?
> 
>  
> 
> Thanks, Håkon
> 

Hi Håkon,
ConnectX devices do not support, currently, PCIe atomic operations.

Regards,
Vladimir

> On Mar 10, 2010, at 16:57 , Vladimir Sokolovsky wrote:
> 
>> Hi Roland,
>>
>> This patchset adds support for the following enhanced atomic
>> operations:
>> - Masked atomic compare and swap
>> - Masked atomic fetch and add
>>
>> These operations enable using a smaller amount of memory when using
>> multiple locks by using portions of a 64 bit value in an atomic
>> operation.
>> For some applications the memory savings are very significant. One
>> example is fine grain lock implementations for huge data sets. In
>> other cases, the benefit is the ability to update multiple fields with
>> a single io operation.
>>
>> Vladimir Sokolovsky(2):
>> IB/core: Add support for enhanced atomic operations
>> mlx4/IB: Add support for enhanced atomic operations
>>
>> changes from V2:
>> - patch #1: 
>>  Updated description
>>  Renamed:
>>        IB_WR_ATOMIC_MASKED_CMP_AND_SWP -> IB_WR_MASKED_ATOMIC_CMP_AND_SWP
>>        IB_WR_ATOMIC_MASKED_FETCH_AND_ADD -> IB_WR_MASKED_ATOMIC_FETCH_AND_ADD
>>  In the ib_send_wr struct the new fields added before the rkey field
>>
>> - patch #2:
>>  Set IB_DEVICE_MASKED_ATOMIC flag with other flags that get set for
>>  all devices
>>
>> Regards,
>> Vladimir
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Håkon Bugge
> Haakon.bugge-U0mLk4xYmo8@public.gmane.org
> +47 924 84 514
> 
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/2] Add support for enhanced atomic operations

[Roland Dreier]

 > Hence, I think it would be cleaner if a new capability,
 > masked_atomic_cap, were introduced, using the original definitions
 > (NONE, HCA, GLOB).

Vlad, what do you think about that?  The more I think about it, the
cleaner this seems to me.  And it doesn't even consume a device
capability flag bit, which is a nice bonus.
-- 
Roland Dreier  <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/2] Add support for enhanced atomic operations

[Vladimir Sokolovsky]

Roland Dreier wrote:
>  > Hence, I think it would be cleaner if a new capability,
>  > masked_atomic_cap, were introduced, using the original definitions
>  > (NONE, HCA, GLOB).
> 
> Vlad, what do you think about that?  The more I think about it, the
> cleaner this seems to me.  And it doesn't even consume a device
> capability flag bit, which is a nice bonus.

Hi Roland,
Do you propose to use IB_ATOMIC_GLOB instead of IB_ATOMIC_HCA while setting
atomic capability in the code below?

         props->atomic_cap          = dev->dev->caps.flags & MLX4_DEV_CAP_FLAG_ATOMIC ?
                 IB_ATOMIC_HCA : IB_ATOMIC_NONE;

Or add IB_MASKED_ATOMIC to ib_atomic_cap enum and use this one instead of IB_ATOMIC_HCA?

All this, of course, comes to replace setting IB_DEVICE_MASKED_ATOMIC for device capability.

Thanks,
Vladimir

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/2] Add support for enhanced atomic operations

[Vladimir Sokolovsky]

Vladimir Sokolovsky wrote:
> Roland Dreier wrote:
>>  > Hence, I think it would be cleaner if a new capability,
>>  > masked_atomic_cap, were introduced, using the original definitions
>>  > (NONE, HCA, GLOB).
>>
>> Vlad, what do you think about that?  The more I think about it, the
>> cleaner this seems to me.  And it doesn't even consume a device
>> capability flag bit, which is a nice bonus.
> 
> Hi Roland,
> Do you propose to use IB_ATOMIC_GLOB instead of IB_ATOMIC_HCA while setting
> atomic capability in the code below?
> 
>         props->atomic_cap          = dev->dev->caps.flags & 
> MLX4_DEV_CAP_FLAG_ATOMIC ?
>                 IB_ATOMIC_HCA : IB_ATOMIC_NONE;
> 
> Or add IB_MASKED_ATOMIC to ib_atomic_cap enum and use this one instead 
> of IB_ATOMIC_HCA?
> 
> All this, of course, comes to replace setting IB_DEVICE_MASKED_ATOMIC 
> for device capability.
> 
> Thanks,
> Vladimir
> 
> 

Hi Roland,
Can you comment?

Thanks,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/2] Add support for enhanced atomic operations

[Roland Dreier]

> Do you propose to use IB_ATOMIC_GLOB instead of IB_ATOMIC_HCA while setting
> atomic capability in the code below?
>
>         props->atomic_cap          = dev->dev->caps.flags &
> MLX4_DEV_CAP_FLAG_ATOMIC ?
>                 IB_ATOMIC_HCA : IB_ATOMIC_NONE;
>
> Or add IB_MASKED_ATOMIC to ib_atomic_cap enum and use this one
> instead of IB_ATOMIC_HCA?
>
> All this, of course, comes to replace setting
> IB_DEVICE_MASKED_ATOMIC for device capability.

Sorry, missed this reply first time around I guess.

Anyway, no I don't think we should use ATOMIC_GLOB unless an HCA truly
provides global atomic ops; not possible without PCIe 3.0 atomic support
I think.  But as I said, I think the original suggestion:

 > > Hence, I think it would be cleaner if a new capability,
 > > masked_atomic_cap, were introduced, using the original definitions
 > > (NONE, HCA, GLOB).

makes sense, ie create a new props->masked_atomic_cap and set it to
NONE, HCA or GLOB as appropriate (ie NONE for everyone except mlx4, and
HCA for mlx4).
-- 
Roland Dreier <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html