<div dir="ltr">Thanks Kaleb for your reply, I will try it and will share the result.<div><br></div><div><br></div><div>Regards</div><div>Mohit Agrawal</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 27, 2017 at 5:33 PM, Kaleb S. KEITHLEY <span dir="ltr"><<a href="mailto:kkeithle@redhat.com" target="_blank">kkeithle@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 09/27/2017 04:17 AM, Mohit Agrawal wrote:<br>
> Niels,<br>
><br>
> Thanks for your reply, I think these built-in function provides by<br>
> gcc and it should support most of the architecture.<br>
> In your view what could be the archietecure that does not support<br>
> these builtin function ??<br>
</span>see<br>
<br>
<br>
<a href="https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/_005f_005fsync-Builtins.html#g_t_005f_005fsync-Builtins" rel="noreferrer" target="_blank">https://gcc.gnu.org/<wbr>onlinedocs/gcc-7.2.0/gcc/_<wbr>005f_005fsync-Builtins.html#g_<wbr>t_005f_005fsync-Builtins</a>,<br>
<br>
<br>
<a href="https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/_005f_005fatomic-Builtins.html#g_t_005f_005fatomic-Builtins" rel="noreferrer" target="_blank">https://gcc.gnu.org/<wbr>onlinedocs/gcc-7.2.0/gcc/_<wbr>005f_005fatomic-Builtins.html#<wbr>g_t_005f_005fatomic-Builtins</a>,<br>
and<br>
<br>
<a href="https://llvm.org/docs/Atomics.html" rel="noreferrer" target="_blank">https://llvm.org/docs/Atomics.<wbr>html</a><br>
<br>
The _legacy_ __sync*() functions have been superceded by the __atomic*()<br>
functions.<br>
<br>
A quick search seems to suggest that ARM has atomic insns since armv6.<br>
Fedora supports armv7hl and aarch64 and IMO ARM should be okay in this<br>
regard.<br>
<br>
A ten year old gcc bug report<br>
(<a href="https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34115" rel="noreferrer" target="_blank">https://gcc.gnu.org/bugzilla/<wbr>show_bug.cgi?id=34115</a>) speaks to __sync*<br>
(and now __atomic* ?) not being supported on the i386 and by extension<br>
to i686 because of how glibc was built for most distributions back then.<br>
We (gluster) are conservative in this regard, but frankly, since hardly<br>
anyone runs 32-bit these days, the perf hit of not using atomic is not a<br>
serious concern.<br>
<br>
It's easy to fall into the trap of saying "it works on my box." Where<br>
"my box" is a x86_64 machine running latest {Fedora,Debian,whatever} but<br>
please keep in mind that even Fedora runs on i686, x86_64, armv7hl,<br>
aarch64, ppc64, ppc64le, and s390x these days. IMO we don't want to fill<br>
our source with lots of #ifdef glop if we can avoid it.<br>
<br>
Clang/LLVM supports __sync*() and __atomic*(), and between Clang and gcc<br>
I'd say that covers pretty much all the distributions we care about,<br>
e.g. Linux and *BSD, and even many we don't care so much about any more.<br>
<br>
I'd say proceed with caution, but proceed. ;-)<br>
<span class=""><br>
<br>
> <br>
><br>
> Regards<br>
> Mohit Agrawal<br>
><br>
> On Wed, Sep 27, 2017 at 1:20 PM, Niels de Vos <<a href="mailto:ndevos@redhat.com">ndevos@redhat.com</a><br>
</span><div><div class="h5">> <mailto:<a href="mailto:ndevos@redhat.com">ndevos@redhat.com</a>>> wrote:<br>
><br>
> On Wed, Sep 27, 2017 at 12:55:37PM +0530, Mohit Agrawal wrote:<br>
> > Hi,<br>
> ><br>
> > I was checking code of internal data structures (dict,fd,rpc_clnt etc.)<br>
> > those we use in glusterfs to store data.<br>
> > Usually we use common pattern to take reference of data structure in<br>
> > xlator level, in ref function we do take lock(mutex_lock)<br>
> > and update(increase) reference counter and in unref function we do take<br>
> > lock and decrease reference counter and<br>
> > check if ref counter is become 0 after decrease then destroy object.<br>
> ><br>
> > I think to update reference counter we don't need to take a lock, we can<br>
> > use atomic in-built function those<br>
> > can improve performance<br>
><br>
> The below is not portable for all architectures. However we have<br>
> refcount.h in libglusterfs/src/ which hides the portability things. One<br>
> of the big advantages to use this, is that the code for reference<br>
> counting is the same everywhere. Some structures have been updated with<br>
> GF_REF_* macros, more can surely be done.<br>
><br>
> For other more basic counters that do not function as reference counter,<br>
> the libglusterfs/src/atomic.h macros can be used. The number of lock<br>
> instructions on modern architectures can be reduced considerably this<br>
> way. It will likely come with a performance increase, but the usage of a<br>
> standard API makes the code simpler to understand and that is my main<br>
> interest :)<br>
><br>
> Obviously I'm all for replacing the lock+count+unlock sequences for many<br>
> structures!<br>
><br>
> Thanks,<br>
> Niels<br>
><br>
><br>
> ><br>
> > For ex: Below is a example for specific to dict_ref/unref<br>
> > To increase refCount we can use below built-in function<br>
> > dict_ref<br>
> > {<br>
> > __atomic_add_fetch (&this->refcount, 1, __ATOMIC_SEQ_CST);<br>
> ><br>
> > }<br>
> ><br>
> > dict_unref<br>
> > {<br>
> > __atomic_sub_fetch (&this->refcount, 1, __ATOMIC_SEQ_CST);<br>
> > __atomic_load (&this->refcount, &ref, __ATOMIC_SEQ_CST);<br>
> > }<br>
> ><br>
> > In the same way we can use for all other shared data-structure also in<br>
> > case of take/release reference.<br>
> ><br>
> > I have not tested yet how much performance improvement we can gain but i<br>
> > think there should be some improvement.<br>
> > Please share your input on this, appreciate your input.<br>
> ><br>
> > Regards<br>
> > Mohit Agrawal<br>
><br>
> > ______________________________<wbr>_________________<br>
> > Gluster-devel mailing list<br>
</div></div>> > <a href="mailto:Gluster-devel@gluster.org">Gluster-devel@gluster.org</a> <mailto:<a href="mailto:Gluster-devel@gluster.org">Gluster-devel@gluster.<wbr>org</a>><br>
> > <a href="http://lists.gluster.org/mailman/listinfo/gluster-devel" rel="noreferrer" target="_blank">http://lists.gluster.org/<wbr>mailman/listinfo/gluster-devel</a><br>
> <<a href="http://lists.gluster.org/mailman/listinfo/gluster-devel" rel="noreferrer" target="_blank">http://lists.gluster.org/<wbr>mailman/listinfo/gluster-devel</a><wbr>><br>
<div class="HOEnZb"><div class="h5">><br>
><br>
><br>
><br>
> ______________________________<wbr>_________________<br>
> Gluster-devel mailing list<br>
> <a href="mailto:Gluster-devel@gluster.org">Gluster-devel@gluster.org</a><br>
> <a href="http://lists.gluster.org/mailman/listinfo/gluster-devel" rel="noreferrer" target="_blank">http://lists.gluster.org/<wbr>mailman/listinfo/gluster-devel</a><br>
><br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
<br>
Kaleb<br>
</font></span></blockquote></div><br></div>