Atomic Limit Counter Add and Subtract, and QQ about goto

Atomic Limit Counter Add and Subtract, and QQ about goto

[Leonardo Brás]

Hi Paul,

I was reading the Counting Chapter in detail, and the QQ about avoiding a Goto
got my attention:

Listing 5.13: Atomic Limit Counter Add and Subtract
Quick Quiz 5.43: Yecch! Why the ugly goto on line 11 of
Listing 5.13? Haven’t you heard of the break statement???

And I was thinking on an alternate solution to those, by calling slowpath as a
function in this simple change:

```
inline int add_count_slowpath(unsigned long delta)
{
	spin_lock(&gblcnt_mutex);
	globalize_count();

	if (globalcountmax - globalcount - globalreserve < delta) {
		flush_local_count();
		if (globalcountmax - globalcount - globalreserve < delta) {
			spin_unlock(&gblcnt_mutex);
			return 0;
		}
	}

	globalcount += delta;
	balance_count();
	spin_unlock(&gblcnt_mutex);
	return 1;
}


int add_count(unsigned long delta)
{
	int c;
	int cm;
	int old;
	int new;

	do {
		split_counterandmax(&counterandmax, &old, &c, &cm);
		if (delta > MAX_COUNTERMAX || c + delta > cm)
			return add_count_slowpath(delta);
		new = merge_counterandmax(c + delta, cm);	
	 while (atomic_cmpxchg(&counterandmax, old, new) != old);

	return 1;

}
```

Does it make sense?
Does it introduce any overhead I couldn't see?

Thanks!
Leo

Re: Atomic Limit Counter Add and Subtract, and QQ about goto

[Leonardo Brás]

On Sat, 2025-01-18 at 17:49 -0300, Leonardo Brás wrote:
> Hi Paul,
> 
> I was reading the Counting Chapter in detail, and the QQ about avoiding a Goto
> got my attention:
> 
> Listing 5.13: Atomic Limit Counter Add and Subtract
> Quick Quiz 5.43: Yecch! Why the ugly goto on line 11 of
> Listing 5.13? Haven’t you heard of the break statement???
> 
> And I was thinking on an alternate solution to those, by calling slowpath as a
> function in this simple change:
> 
> ```
> inline int add_count_slowpath(unsigned long delta)
> {
> 	spin_lock(&gblcnt_mutex);
> 	globalize_count();
> 
> 	if (globalcountmax - globalcount - globalreserve < delta) {
> 		flush_local_count();
> 		if (globalcountmax - globalcount - globalreserve < delta) {
> 			spin_unlock(&gblcnt_mutex);
> 			return 0;
> 		}
> 	}
> 
> 	globalcount += delta;
> 	balance_count();
> 	spin_unlock(&gblcnt_mutex);
> 	return 1;
> }
> 
> 
> int add_count(unsigned long delta)
> {
> 	int c;
> 	int cm;
> 	int old;
> 	int new;
> 
> 	do {
> 		split_counterandmax(&counterandmax, &old, &c, &cm);
> 		if (delta > MAX_COUNTERMAX || c + delta > cm)
> 			return add_count_slowpath(delta);
> 		new = merge_counterandmax(c + delta, cm);	
> 	 while (atomic_cmpxchg(&counterandmax, old, new) != old);
> 
> 	return 1;
> 
> }
> ```
> 
> Does it make sense?
> Does it introduce any overhead I couldn't see?
> 
> Thanks!
> Leo
> 
> 

Oh, just to be clear, I like goto, and IMO the above is a perfectly acceptable
way of using goto, and I would not change it. Just thinking about the QQ and
added complexity.

Re: Atomic Limit Counter Add and Subtract, and QQ about goto

[Paul E. McKenney]

On Sat, Jan 18, 2025 at 05:52:00PM -0300, Leonardo Brás wrote:
> On Sat, 2025-01-18 at 17:49 -0300, Leonardo Brás wrote:
> > Hi Paul,
> > 
> > I was reading the Counting Chapter in detail, and the QQ about avoiding a Goto
> > got my attention:
> > 
> > Listing 5.13: Atomic Limit Counter Add and Subtract
> > Quick Quiz 5.43: Yecch! Why the ugly goto on line 11 of
> > Listing 5.13? Haven’t you heard of the break statement???
> > 
> > And I was thinking on an alternate solution to those, by calling slowpath as a
> > function in this simple change:
> > 
> > ```
> > inline int add_count_slowpath(unsigned long delta)
> > {
> > 	spin_lock(&gblcnt_mutex);
> > 	globalize_count();
> > 
> > 	if (globalcountmax - globalcount - globalreserve < delta) {
> > 		flush_local_count();
> > 		if (globalcountmax - globalcount - globalreserve < delta) {
> > 			spin_unlock(&gblcnt_mutex);
> > 			return 0;
> > 		}
> > 	}
> > 
> > 	globalcount += delta;
> > 	balance_count();
> > 	spin_unlock(&gblcnt_mutex);
> > 	return 1;
> > }
> > 
> > 
> > int add_count(unsigned long delta)
> > {
> > 	int c;
> > 	int cm;
> > 	int old;
> > 	int new;
> > 
> > 	do {
> > 		split_counterandmax(&counterandmax, &old, &c, &cm);
> > 		if (delta > MAX_COUNTERMAX || c + delta > cm)
> > 			return add_count_slowpath(delta);
> > 		new = merge_counterandmax(c + delta, cm);	
> > 	 while (atomic_cmpxchg(&counterandmax, old, new) != old);
> > 
> > 	return 1;
> > 
> > }
> > ```
> > 
> > Does it make sense?
> > Does it introduce any overhead I couldn't see?
> > 
> > Thanks!
> > Leo
> 
> Oh, just to be clear, I like goto, and IMO the above is a perfectly acceptable
> way of using goto, and I would not change it. Just thinking about the QQ and
> added complexity.

Congratulations!  You are the first to have responded to the implicit
challenge in the last sentence of the answer to that Quick Quiz:

	If you really hate the goto that much, your best bet would be
	to pull the fastpath into a separate function that returned
	success or failure, with “failure” indicating a need for
	the slowpath. This is left as an exercise for goto-hating readers.

Very good!  ;-)

So why didn't I do it that way in the book?  Because that would add
several lines for the function declaration, and Listing 5.13 is long
enough as it is.

But if enough people prefer the separate slowpath function, and someone
is willing to create the patch, adjust the text, and test the result,
why not?

							Thanx, Paul