<div dir="ltr"><div><div>Hi Björn,<br><br></div>I will take a look at the code and investigate atomic operations on ARM.<br><br></div><div>Thanks,<br><br></div>Namhoon<br><div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 8, 2017 at 6:39 AM, Björn Brandenburg <span dir="ltr"><<a href="mailto:bbb@mpi-sws.org" target="_blank">bbb@mpi-sws.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-"><br>
> On 8 Mar 2017, at 10:16, Andrea Bastoni <<a href="mailto:bastoni@sprg.uniroma2.it">bastoni@sprg.uniroma2.it</a>> wrote:<br>
> So, yes, above you need a release<br>
> semantics (but that should be coupled with an acquire one somewhere,<br>
> perhaps where the free count is checked(?) -- why is the free_count<br>
> incremented atomically, but not read with at least READ_ONCE? -- but<br>
> I'm no longer familiar with the code, sorry.)<br>
<br>
</span>Hi Andrea,<br>
<br>
thanks for your quick reply. The corresponding atomic access is in ft_buffer_start_write() in litmus/feather_buffer.h.<br>
<br>
static inline int ft_buffer_start_write(struct ft_buffer* buf, void **ptr)<br>
{<br>
<br>
—->     int free = fetch_and_dec(&buf->free_<wbr>count);<br>
<br>
        unsigned int idx;<br>
        if (free <= 0) {<br>
                fetch_and_inc(&buf->free_<wbr>count);<br>
                *ptr = 0;<br>
                fetch_and_inc(&buf->failed_<wbr>writes);<br>
                return 0;<br>
        } else {<br>
                idx  = fetch_and_inc((int*) &buf->write_idx) % buf->slot_count;<br>
                buf->slots[idx] = SLOT_BUSY;<br>
                *ptr = ((char*) buf->buffer_mem) + idx * buf->slot_size;<br>
                return 1;<br>
        }<br>
}<br>
<br>
If both instances are fully serializing (i.e., the “old” semantics of atomic_add_return()), then this should work fine, I think.<br>
<br>
There are also a few places where the variable is read without atomics (e.g., the single-writer case in the same file). This “relies” (i.e., it seemingly has worked to date) on the fact that word-aligned loads on x86 are actually atomic, and the fact that the variable is not used again (without a cast to atomic) in the same scope. I don’t think anyone has reasoned about portability to other platforms much to date, or taken into account what future hyper-agressive compiler optimizations might result in.<br>
<br>
Hence my suggestion to convert everything to proper atomic_t variables… Any volunteers (not Andrea)?<br>
<span class="gmail-"><br>
> In the end, yes, there's the need of a barrier after the<br>
> fetch_and_inc(). Probably this _untested_ patch may do the trick:<br>
> diff --git a/include/litmus/feather_<wbr>trace.h b/include/litmus/feather_<wbr>trace.h<br>
> index dbeca46..24dbc16 100644<br>
> --- a/include/litmus/feather_<wbr>trace.h<br>
> +++ b/include/litmus/feather_<wbr>trace.h<br>
> @@ -1,7 +1,7 @@<br>
> #ifndef _FEATHER_TRACE_H_<br>
> #define _FEATHER_TRACE_H_<br>
><br>
> -#include <asm/atomic.h><br>
> +#include <linux/atomic.h><br>
<br>
</span>Namhoon, can you confirm that this works?<br>
<br>
Thanks,<br>
Björn<br>
<br>
</blockquote></div><br></div></div></div></div></div>