Re: [PATCH] BUILD: common: Add __ha_cas_dw fallback for single threaded builds

[Chris Packham]

On 12/05/19 5:29 AM, Willy Tarreau wrote:
> On Fri, May 10, 2019 at 11:52:31AM +0200, Willy Tarreau wrote:
>>> Actually I think there's an additional change needed in my patch. By
>>> passing the parameters to HA_ATOMIC_CAS we end up attempting to
>>> dereference a void *. So this should needs to cast to a proper type. For
>>> what it's worth I'll send a v2 that does this.
>>
>> OK, but since it's already merged, please send an incremental patch.
> 
> Don't waste your time, finally I fixed it definitely.
> 
> Willy
> 

Thanks Willy.

Re: [PATCH] BUILD: common: Add __ha_cas_dw fallback for single threaded builds

[Willy Tarreau]

On Fri, May 10, 2019 at 11:52:31AM +0200, Willy Tarreau wrote:
> > Actually I think there's an additional change needed in my patch. By 
> > passing the parameters to HA_ATOMIC_CAS we end up attempting to 
> > dereference a void *. So this should needs to cast to a proper type. For 
> > what it's worth I'll send a v2 that does this.
> 
> OK, but since it's already merged, please send an incremental patch.

Don't waste your time, finally I fixed it definitely.

Willy

Re: [PATCH] BUILD: common: Add __ha_cas_dw fallback for single threaded builds

[Willy Tarreau]

On Fri, May 10, 2019 at 09:38:08AM +, Chris Packham wrote:
> On 10/05/19 8:57 PM, Willy Tarreau wrote:
> > On Thu, May 09, 2019 at 05:07:40PM +1200, Chris Packham wrote:
> >> __ha_cas_dw() is used in fd_rm_from_fd_list() and when built without
> >> USE_THREADS=1 the linker fails to find __ha_cas_dw(). Add a definition
> >> of __ha_cas_dw() for the #ifndef USE_THREADS case.
> > 
> > Just found your patch, I think it's indeed OK to fall back to
> > HA_ATOMIC_CAS in this case since we won't use atomic instructions.
> > I'd like that we do a bit of tidying in this area so that it's
> > clearer which functions are always atomic and which ones possibly
> > are not, but for now that's OK. I've merged it now.
> 
> Actually I think there's an additional change needed in my patch. By 
> passing the parameters to HA_ATOMIC_CAS we end up attempting to 
> dereference a void *. So this should needs to cast to a proper type. For 
> what it's worth I'll send a v2 that does this.

OK, but since it's already merged, please send an incremental patch.

Thanks,
Willy

Re: [PATCH] BUILD: common: Add __ha_cas_dw fallback for single threaded builds

[Chris Packham]

On 10/05/19 8:57 PM, Willy Tarreau wrote:
> On Thu, May 09, 2019 at 05:07:40PM +1200, Chris Packham wrote:
>> __ha_cas_dw() is used in fd_rm_from_fd_list() and when built without
>> USE_THREADS=1 the linker fails to find __ha_cas_dw(). Add a definition
>> of __ha_cas_dw() for the #ifndef USE_THREADS case.
> 
> Just found your patch, I think it's indeed OK to fall back to
> HA_ATOMIC_CAS in this case since we won't use atomic instructions.
> I'd like that we do a bit of tidying in this area so that it's
> clearer which functions are always atomic and which ones possibly
> are not, but for now that's OK. I've merged it now.

Actually I think there's an additional change needed in my patch. By 
passing the parameters to HA_ATOMIC_CAS we end up attempting to 
dereference a void *. So this should needs to cast to a proper type. For 
what it's worth I'll send a v2 that does this.

Re: [PATCH] BUILD: common: Add __ha_cas_dw fallback for single threaded builds

[Willy Tarreau]

On Thu, May 09, 2019 at 05:07:40PM +1200, Chris Packham wrote:
> __ha_cas_dw() is used in fd_rm_from_fd_list() and when built without
> USE_THREADS=1 the linker fails to find __ha_cas_dw(). Add a definition
> of __ha_cas_dw() for the #ifndef USE_THREADS case.

Just found your patch, I think it's indeed OK to fall back to
HA_ATOMIC_CAS in this case since we won't use atomic instructions.
I'd like that we do a bit of tidying in this area so that it's
clearer which functions are always atomic and which ones possibly
are not, but for now that's OK. I've merged it now.

Thanks!
Willy

[PATCH] BUILD: common: Add __ha_cas_dw fallback for single threaded builds

[Chris Packham]

__ha_cas_dw() is used in fd_rm_from_fd_list() and when built without
USE_THREADS=1 the linker fails to find __ha_cas_dw(). Add a definition
of __ha_cas_dw() for the #ifndef USE_THREADS case.

Signed-off-by: Chris Packham 
---
 include/common/hathreads.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/common/hathreads.h b/include/common/hathreads.h
index cae6eabe..516a66d0 100644
--- a/include/common/hathreads.h
+++ b/include/common/hathreads.h
@@ -140,6 +140,11 @@ static inline void __ha_barrier_full(void)
 {
 }
 
+static inline int __ha_cas_dw(void *target, void *compare, void *set)
+{
+   return HA_ATOMIC_CAS(target, compare, set);
+}
+
 static inline void thread_harmless_now()
 {
 }
-- 
2.21.0