lkml.org 
[lkml]   [2020]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
From
Date
On 11/16/20 7:53 AM, Peter Zijlstra wrote:
> On Fri, Nov 13, 2020 at 10:46:03AM -0700, Shuah Khan wrote:
>
>> +Increment interface
>> +-------------------
>> +
>> +Increments sequence number and returns the new value. ::
>> +
>> + seqnum32_inc_return() --> (u32) atomic_inc_return(seqnum)
>> + seqnum64_inc_return() --> (u64) atomic64_inc_return(seqnum)
>
> Did you think about the ordering?
>

Looking at atomic_t.txt _inc_return() can be fully ordered without
loosing or making the intermediate state visible. This is good for
this sequence number use-case. Is there something I am overlooking?

>> +Fetch interface
>> +---------------
>> +
>> +Fetched and returns current sequence number value. ::
>> +
>> + seqnum32_fetch() --> (u32) atomic_add_return(0, seqnum)
>> + seqnum64_fetch() --> (u64) atomic64_add_return(0, seqnum)
>
> That's horrible. Please explain how that is not broken garbage.
>
> Per the fact that it is atomic, nothing prevents the counter being
> incremented concurrently. There is no such thing as a 'current' sequence
> number.
>

Correct. Some usages of this _fecth() in this patch series are for
printing sequence numbers in debug message and others are stats.

I will review the patches in this series and drop the ones that use
read/fetch - the reason being sequence numbers are strictly up counters
and don't need read/fetch.

thanks,
-- Shuah





\
 
 \ /
  Last update: 2020-11-17 17:17    [W:0.191 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site