On Fri, Feb 05, 2021 at 01:03:18PM -0700, Shuah Khan wrote:
On 2/5/21 2:58 AM, Greg KH wrote:
On Wed, Feb 03, 2021 at 11:11:57AM -0700, Shuah Khan wrote:
+static inline u32 seqnum32_inc(struct seqnum32 *seq) +{
- atomic_t val = ATOMIC_INIT(seq->seqnum);
- seq->seqnum = (u32) atomic_inc_return(&val);
- if (seq->seqnum >= UINT_MAX)
pr_info("Sequence Number overflow %u detected\n",seq->seqnum);- return seq->seqnum;
As Peter points out, this is doing doing what you think it is doing :(
Why do you not just have seq->seqnum be a real atomic variable? Trying to switch to/from one like this does not work as there is no "atomic-ness" happening here at all.
Yes. This is sloppy on my part. As Peter and Rafael also pointed. I have to start paying more attention to my inner voice.
What's the end goal with this sequence number type?
Apart from the functional issues with this code already pointed out, it doesn't seem overly useful to just print the *value* of the sequence number when it hits the max value. It might be more helpful if each instance had a seq->name field that is used here to tell the user *which* user of sequence numbers had seen the wrap arounnd.
But that just begs the question of what should users of sequence numbers do when wrap around occurs? Any user that can run through sequence numbers at greater than 10 Hz is going to cause a problem for systems that stay running for years.
Maybe you should force users to code for wraparound by initializing to something like 0xffffff00 so that they all get a wraparound event quickly, rather than slowly, (same as was done with jiffies)?