MINOR: server: ensure max_events_at_once > 0 in server_atomic_sync()

In 8f1fd96 ("BUG/MEDIUM: server/addr: fix tune.events.max-events-at-once
event miss and leak"), we added a comment saying that
tune.events.max-events-at-once is assumed to be strictly positive.

It is so because the keyword parser forces values between 1 and 10000:
we don't want less than 1 because it wouldn't make any sense, and 10k
max because beyond that we could create contention in server_atomic_sync()

Now as the above commit implements a do..while it heavily relies on the
fact that the budget is at least 1. Upon soft-stop, we break away from
the loop without decrementing the budget. With all that in mind, it is
safe to assume that the 'remain' counter will only fall to 0 if the task
runs out of budget while doing work, in which case the task still exists
and must be rescheduled.

As seen in GH #2667 this assumption was ambiguous, so let's make it
official by adding a pair of BUG_ON() that make it explicit that it
works because remain 'cannot' be 0 unless the entire budget was
consumed.

No backport needed.
This commit is contained in:
Aurelien DARRAGON 2024-08-07 18:04:08 +02:00
parent 3ef1ee477d
commit a6d1eb8f5d

View File

@ -227,6 +227,8 @@ static struct task *server_atomic_sync(struct task *task, void *context, unsigne
unsigned int remain = event_hdl_tune.max_events_at_once; // to limit max number of events per batch
struct event_hdl_async_event *event;
BUG_ON(remain == 0); // event_hdl_tune.max_events_at_once is expected to be > 0
/* check for new server events that we care about */
do {
event = event_hdl_async_equeue_pop(&server_atomic_sync_queue);
@ -317,7 +319,7 @@ static struct task *server_atomic_sync(struct task *task, void *context, unsigne
srv_set_addr_desc(srv, 1);
}
event_hdl_async_free_event(event);
} while (--remain); // event_hdl_tune.max_events_at_once is expected to be > 0
} while (--remain);
/* some events possibly required thread_isolation:
* now that we are done, we must leave thread isolation before
@ -334,6 +336,7 @@ static struct task *server_atomic_sync(struct task *task, void *context, unsigne
* Reschedule the task to finish where we left off if
* there are remaining events in the queue.
*/
BUG_ON(task == NULL); // ending event doesn't decrement remain
if (!event_hdl_async_equeue_isempty(&server_atomic_sync_queue))
task_wakeup(task, TASK_WOKEN_OTHER);
}