kernelnewbies.kernelnewbies.org archive mirror
 help / color / mirror / Atom feed
* atomic test and set
@ 2017-10-18  0:40 Tobin C. Harding
  2017-10-18  1:33 ` Anish Kumar
  2017-10-20 14:22 ` Yubin Ruan
  0 siblings, 2 replies; 6+ messages in thread
From: Tobin C. Harding @ 2017-10-18  0:40 UTC (permalink / raw)
  To: kernelnewbies

Hi,

Do we have an atomic test and set function in the kernel. I have tried

const int KEY_FLAG_BIT = 1;

...

	static siphash_key_t ptr_secret __read_mostly;
	static unsigned long have_key = 0;

	if (test_and_set_bit(KEY_FLAG_BIT, &have_key))
		get_random_bytes(&ptr_secret, sizeof(ptr_secret));


But that doesn't work.

I looked in include/linux/atomic.h and thought about using

	static atomic_t have_key = ATOMIC_INIT(0);

        if (atomic_xchg(&have_key, 1) == 0)
        	get_random_bytes(&ptr_secret, sizeof(ptr_secret));


This works. My question is; does this code LOAD the value at have_key and STORE the argument on
every call? Or does it LOAD the value, check if it is the same as the argument, and STORE _only_ if
it is different? 

(Is this whole discussion just premature optimization?)

I cannot grok the macros in atomic.h, they seem circular. Here is the macro definitions in call
chain order starting with atomic_xchg()


#define  atomic_xchg(...)						\
	__atomic_op_fence(atomic_xchg, __VA_ARGS__)

#define __atomic_op_fence(op, args...)					\
({									\
	typeof(op##_relaxed(args)) __ret;				\
	smp_mb__before_atomic();					\
	__ret = op##_relaxed(args);					\
	smp_mb__after_atomic();						\
	__ret;								\
})

#define  atomic_xchg_release(...)					\
	__atomic_op_release(atomic_xchg, __VA_ARGS__)

#define __atomic_op_release(op, args...)				\
({									\
	smp_mb__before_atomic();					\
	op##_relaxed(args);						\
})

#define  atomic_xchg_relaxed		atomic_xchg

thanks,
Tobin.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* atomic test and set
  2017-10-18  0:40 atomic test and set Tobin C. Harding
@ 2017-10-18  1:33 ` Anish Kumar
  2017-10-20 14:22 ` Yubin Ruan
  1 sibling, 0 replies; 6+ messages in thread
From: Anish Kumar @ 2017-10-18  1:33 UTC (permalink / raw)
  To: kernelnewbies



> On Oct 17, 2017, at 5:40 PM, Tobin C. Harding <me@tobin.cc> wrote:
> 
> Hi,
> 
> Do we have an atomic test and set function in the kernel. I have tried

Yes and it is used very much inside the kernel.
> 
> const int KEY_FLAG_BIT = 1;
> 
> ...
> 
>    static siphash_key_t ptr_secret __read_mostly;
>    static unsigned long have_key = 0;
> 
>    if (test_and_set_bit(KEY_FLAG_BIT, &have_key))
>        get_random_bytes(&ptr_secret, sizeof(ptr_secret));
> 
> 
> But that doesn't work.

What doesn't work? What you expected and how is the result different?
> 
> I looked in include/linux/atomic.h and thought about using
> 
>    static atomic_t have_key = ATOMIC_INIT(0);
> 
>        if (atomic_xchg(&have_key, 1) == 0)
>            get_random_bytes(&ptr_secret, sizeof(ptr_secret));
> 
> 
> This works. My question is; does this code LOAD the value at have_key and STORE the argument on
> every call? Or does it LOAD the value, check if it is the same as the argument, and STORE _only_ if
> it is different? 

Did you read this https://www.mjmwired.net/kernel/Documentation/atomic_ops.txt ?
> (Is this whole discussion just premature optimization?)

This has nothing to do with optimization but rather the requirement. In kernel atomic variables are used to avoid race conditions generally.
> 
> I cannot grok the macros in atomic.h, they seem circular. Here is the macro definitions in call
> chain order starting with atomic_xchg()

Implementation would differ based on the architecture. In some architectures it would be a single instruction and in others it may not be the case.
> 
> 
> #define  atomic_xchg(...)                        \
>    __atomic_op_fence(atomic_xchg, __VA_ARGS__)
> 
> #define __atomic_op_fence(op, args...)                    \
> ({                                    \
>    typeof(op##_relaxed(args)) __ret;                \
>    smp_mb__before_atomic();                    \
>    __ret = op##_relaxed(args);                    \
>    smp_mb__after_atomic();                        \
>    __ret;                                \
> })
> 
> #define  atomic_xchg_release(...)                    \
>    __atomic_op_release(atomic_xchg, __VA_ARGS__)
> 
> #define __atomic_op_release(op, args...)                \
> ({                                    \
>    smp_mb__before_atomic();                    \
>    op##_relaxed(args);                        \
> })
> 
> #define  atomic_xchg_relaxed        atomic_xchg
> 
> thanks,
> Tobin.
> 
> _______________________________________________
> Kernelnewbies mailing list
> Kernelnewbies at kernelnewbies.org
> https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20171017/0fc28498/attachment.html 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* atomic test and set
  2017-10-20 14:22 ` Yubin Ruan
@ 2017-10-20  8:55   ` Tobin C. Harding
  2017-10-20  9:33     ` Yubin Ruan
  0 siblings, 1 reply; 6+ messages in thread
From: Tobin C. Harding @ 2017-10-20  8:55 UTC (permalink / raw)
  To: kernelnewbies

On Fri, Oct 20, 2017 at 10:22:01PM +0800, Yubin Ruan wrote:
> On Wed, Oct 18, 2017 at 11:40:44AM +1100, Tobin C. Harding wrote:
> > Hi,
> > 
> > Do we have an atomic test and set function in the kernel. I have tried
> > 
> > const int KEY_FLAG_BIT = 1;
> > 
> > ...
> > 
> > 	static siphash_key_t ptr_secret __read_mostly;
> > 	static unsigned long have_key = 0;
> > 
> > 	if (test_and_set_bit(KEY_FLAG_BIT, &have_key))
> > 		get_random_bytes(&ptr_secret, sizeof(ptr_secret));
> > 
> > 
> > But that doesn't work.
> > 
> > I looked in include/linux/atomic.h and thought about using
> > 
> > 	static atomic_t have_key = ATOMIC_INIT(0);
> > 
> >         if (atomic_xchg(&have_key, 1) == 0)
> >         	get_random_bytes(&ptr_secret, sizeof(ptr_secret));
> > 
> > 
> > This works. My question is; does this code LOAD the value at have_key and STORE the argument on
> > every call? Or does it LOAD the value, check if it is the same as the argument, and STORE _only_ if
> > it is different? 
> 
> Yes it stores at every call. To check before load, you should use cmpxchg
>  
> > (Is this whole discussion just premature optimization?)
> > 
> > I cannot grok the macros in atomic.h, they seem circular. Here is the macro definitions in call
> > chain order starting with atomic_xchg()
> 
> I think those macros look loverly ;-)

Thanks Yubin, guess I need to work on my macro-foo

> Maybe you should look into `Documentation/atomic_ops.txt' for help. Also, I
> think see how xchg() and cmpxchg() is implemented in the kernel, e.g., at here:
> 
>     https://elixir.free-electrons.com/linux/latest/source/arch/x86/include/asm/cmpxchg.h

thanks,
Tobin.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* atomic test and set
  2017-10-20  8:55   ` Tobin C. Harding
@ 2017-10-20  9:33     ` Yubin Ruan
  2017-10-22  1:50       ` Tobin C. Harding
  0 siblings, 1 reply; 6+ messages in thread
From: Yubin Ruan @ 2017-10-20  9:33 UTC (permalink / raw)
  To: kernelnewbies

2017-10-20 16:55 GMT+08:00 Tobin C. Harding <me@tobin.cc>:
> On Fri, Oct 20, 2017 at 10:22:01PM +0800, Yubin Ruan wrote:
>> On Wed, Oct 18, 2017 at 11:40:44AM +1100, Tobin C. Harding wrote:
>> > Hi,
>> >
>> > Do we have an atomic test and set function in the kernel. I have tried
>> >
>> > const int KEY_FLAG_BIT = 1;
>> >
>> > ...
>> >
>> >     static siphash_key_t ptr_secret __read_mostly;
>> >     static unsigned long have_key = 0;
>> >
>> >     if (test_and_set_bit(KEY_FLAG_BIT, &have_key))
>> >             get_random_bytes(&ptr_secret, sizeof(ptr_secret));
>> >
>> >
>> > But that doesn't work.
>> >
>> > I looked in include/linux/atomic.h and thought about using
>> >
>> >     static atomic_t have_key = ATOMIC_INIT(0);
>> >
>> >         if (atomic_xchg(&have_key, 1) == 0)
>> >             get_random_bytes(&ptr_secret, sizeof(ptr_secret));
>> >
>> >
>> > This works. My question is; does this code LOAD the value at have_key and STORE the argument on
>> > every call? Or does it LOAD the value, check if it is the same as the argument, and STORE _only_ if
>> > it is different?
>>
>> Yes it stores at every call. To check before load, you should use cmpxchg

sorry this should be "check before store" ...

>>
>> > (Is this whole discussion just premature optimization?)
>> >
>> > I cannot grok the macros in atomic.h, they seem circular. Here is the macro definitions in call
>> > chain order starting with atomic_xchg()
>>
>> I think those macros look loverly ;-)
>
> Thanks Yubin, guess I need to work on my macro-foo
>
>> Maybe you should look into `Documentation/atomic_ops.txt' for help. Also, I
>> think see how xchg() and cmpxchg() is implemented in the kernel, e.g., at here:
>>
>>     https://elixir.free-electrons.com/linux/latest/source/arch/x86/include/asm/cmpxchg.h

I really think the `cmpxchg.h' is valuable for learning. Please do
take a look at it.

And sorry again for so many typos.

Yubin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* atomic test and set
  2017-10-18  0:40 atomic test and set Tobin C. Harding
  2017-10-18  1:33 ` Anish Kumar
@ 2017-10-20 14:22 ` Yubin Ruan
  2017-10-20  8:55   ` Tobin C. Harding
  1 sibling, 1 reply; 6+ messages in thread
From: Yubin Ruan @ 2017-10-20 14:22 UTC (permalink / raw)
  To: kernelnewbies

On Wed, Oct 18, 2017 at 11:40:44AM +1100, Tobin C. Harding wrote:
> Hi,
> 
> Do we have an atomic test and set function in the kernel. I have tried
> 
> const int KEY_FLAG_BIT = 1;
> 
> ...
> 
> 	static siphash_key_t ptr_secret __read_mostly;
> 	static unsigned long have_key = 0;
> 
> 	if (test_and_set_bit(KEY_FLAG_BIT, &have_key))
> 		get_random_bytes(&ptr_secret, sizeof(ptr_secret));
> 
> 
> But that doesn't work.
> 
> I looked in include/linux/atomic.h and thought about using
> 
> 	static atomic_t have_key = ATOMIC_INIT(0);
> 
>         if (atomic_xchg(&have_key, 1) == 0)
>         	get_random_bytes(&ptr_secret, sizeof(ptr_secret));
> 
> 
> This works. My question is; does this code LOAD the value at have_key and STORE the argument on
> every call? Or does it LOAD the value, check if it is the same as the argument, and STORE _only_ if
> it is different? 

Yes it stores at every call. To check before load, you should use cmpxchg
 
> (Is this whole discussion just premature optimization?)
> 
> I cannot grok the macros in atomic.h, they seem circular. Here is the macro definitions in call
> chain order starting with atomic_xchg()

I think those macros look loverly ;-)
Maybe you should look into `Documentation/atomic_ops.txt' for help. Also, I
think see how xchg() and cmpxchg() is implemented in the kernel, e.g., at here:

    https://elixir.free-electrons.com/linux/latest/source/arch/x86/include/asm/cmpxchg.h

Yubin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* atomic test and set
  2017-10-20  9:33     ` Yubin Ruan
@ 2017-10-22  1:50       ` Tobin C. Harding
  0 siblings, 0 replies; 6+ messages in thread
From: Tobin C. Harding @ 2017-10-22  1:50 UTC (permalink / raw)
  To: kernelnewbies

On Fri, Oct 20, 2017 at 05:33:19PM +0800, Yubin Ruan wrote:
> 2017-10-20 16:55 GMT+08:00 Tobin C. Harding <me@tobin.cc>:
> > On Fri, Oct 20, 2017 at 10:22:01PM +0800, Yubin Ruan wrote:
> >> On Wed, Oct 18, 2017 at 11:40:44AM +1100, Tobin C. Harding wrote:
> >> > Hi,
> >> >
> >> > Do we have an atomic test and set function in the kernel. I have tried
> >> >
> >> > const int KEY_FLAG_BIT = 1;
> >> >
> >> > ...
> >> >
> >> >     static siphash_key_t ptr_secret __read_mostly;
> >> >     static unsigned long have_key = 0;
> >> >
> >> >     if (test_and_set_bit(KEY_FLAG_BIT, &have_key))
> >> >             get_random_bytes(&ptr_secret, sizeof(ptr_secret));
> >> >
> >> >
> >> > But that doesn't work.
> >> >
> >> > I looked in include/linux/atomic.h and thought about using
> >> >
> >> >     static atomic_t have_key = ATOMIC_INIT(0);
> >> >
> >> >         if (atomic_xchg(&have_key, 1) == 0)
> >> >             get_random_bytes(&ptr_secret, sizeof(ptr_secret));
> >> >
> >> >
> >> > This works. My question is; does this code LOAD the value at have_key and STORE the argument on
> >> > every call? Or does it LOAD the value, check if it is the same as the argument, and STORE _only_ if
> >> > it is different?
> >>
> >> Yes it stores at every call. To check before load, you should use cmpxchg
> 
> sorry this should be "check before store" ...
> 
> >>
> >> > (Is this whole discussion just premature optimization?)
> >> >
> >> > I cannot grok the macros in atomic.h, they seem circular. Here is the macro definitions in call
> >> > chain order starting with atomic_xchg()
> >>
> >> I think those macros look loverly ;-)
> >
> > Thanks Yubin, guess I need to work on my macro-foo
> >
> >> Maybe you should look into `Documentation/atomic_ops.txt' for help. Also, I
> >> think see how xchg() and cmpxchg() is implemented in the kernel, e.g., at here:
> >>
> >>     https://elixir.free-electrons.com/linux/latest/source/arch/x86/include/asm/cmpxchg.h
> 
> I really think the `cmpxchg.h' is valuable for learning. Please do
> take a look at it.

I read atomic_ops.txt, thanks. Also I used cmpxchg() like this 

static atomic_t foo = ATOMIC_INIT(0);

...
        if (atomic_cmpxchg(&foo, 0, 1) == 0) {
                do_this();
        }

With the intent that only a single caller will ever call `do_this()`.

thanks,
Tobin.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-10-22  1:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-18  0:40 atomic test and set Tobin C. Harding
2017-10-18  1:33 ` Anish Kumar
2017-10-20 14:22 ` Yubin Ruan
2017-10-20  8:55   ` Tobin C. Harding
2017-10-20  9:33     ` Yubin Ruan
2017-10-22  1:50       ` Tobin C. Harding

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).