Instead of relying on a global and shared hash_lock
to protect sig->next_posix_timer_id, make it atomic.

This allows the following patch to use RCU.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/sched/signal.h | 2 +-
 kernel/time/posix-timers.c   | 9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index d5d03d919df8..72649d7fce2a 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -136,7 +136,7 @@ struct signal_struct {
 #ifdef CONFIG_POSIX_TIMERS
 
 	/* POSIX.1b Interval Timers */
-	unsigned int		next_posix_timer_id;
+	atomic_t		next_posix_timer_id;
 	struct hlist_head	posix_timers;
 	struct hlist_head	ignored_posix_timers;
 
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index 1b675aee99a9..204a351a2fd3 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -105,13 +105,14 @@ static int posix_timer_add(struct k_itimer *timer)
 	 * a plan to handle the resulting CRIU regression gracefully.
 	 */
 	for (cnt = 0; cnt <= INT_MAX; cnt++) {
-		spin_lock(&hash_lock);
-		id = sig->next_posix_timer_id;
+		id = atomic_inc_return(&sig->next_posix_timer_id) - 1;
 
-		/* Write the next ID back. Clamp it to the positive space */
-		sig->next_posix_timer_id = (id + 1) & INT_MAX;
+		/* Clamp id to the positive space */
+		id &= INT_MAX;
 
 		head = &posix_timers_hashtable[hash(sig, id)];
+
+		spin_lock(&hash_lock);
 		if (!__posix_timers_find(head, sig, id)) {
 			hlist_add_head_rcu(&timer->t_hash, head);
 			spin_unlock(&hash_lock);
-- 
2.48.1.601.g30ceb7b040-goog

On Wed, Feb 19 2025 at 12:55, Eric Dumazet wrote:
> Instead of relying on a global and shared hash_lock
> to protect sig->next_posix_timer_id, make it atomic.
>
> This allows the following patch to use RCU.

Your patch ordering is slightly off by two :)

And it fails to explain for what RCU can be used....

Thanks,

        tglx

On Thu, Feb 20, 2025 at 9:09 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Wed, Feb 19 2025 at 12:55, Eric Dumazet wrote:
> > Instead of relying on a global and shared hash_lock
> > to protect sig->next_posix_timer_id, make it atomic.
> >
> > This allows the following patch to use RCU.
>
> Your patch ordering is slightly off by two :)
>
> And it fails to explain for what RCU can be used....

This is explained in the following patches.

If I add nothing in the changelog, you complain the changelog is not
explaining anything.

I suggest you write the patches. because I feel a huge resistance,
which I do not understand.

Thank you.

On Thu, Feb 20 2025 at 09:49, Eric Dumazet wrote:
> On Thu, Feb 20, 2025 at 9:09 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>> > This allows the following patch to use RCU.
>>
>> Your patch ordering is slightly off by two :)
>>
>> And it fails to explain for what RCU can be used....
>
> This is explained in the following patches.

The changelog of a patch has to be self contained. The 'following patch'
has no meaning when the patch is merged.

> If I add nothing in the changelog, you complain the changelog is not
> explaining anything.
>
> I suggest you write the patches. because I feel a huge resistance,
> which I do not understand.

I'm just asking that I get properly written change logs which adhere to
the documented change log requirements.

How does that qualify as resistance?

Thanks,

        tglx

On Thu, Feb 20 2025 at 15:04, Thomas Gleixner wrote:
> On Thu, Feb 20 2025 at 09:49, Eric Dumazet wrote:
>> On Thu, Feb 20, 2025 at 9:09 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>>> > This allows the following patch to use RCU.
>>>
>>> Your patch ordering is slightly off by two :)
>>>
>>> And it fails to explain for what RCU can be used....
>>
>> This is explained in the following patches.
>
> The changelog of a patch has to be self contained. The 'following patch'
> has no meaning when the patch is merged.

That said, please just fold this into the patch which actually does this RCU
lookup upfront. The change is trivial enough that it does not really
require to be seperate. If the lockless increment would cause issues,
then the subsequent RCU lookup is the least of the worries :)

Thanks,

	tglx

On Thu, Feb 20, 2025 at 3:32 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Thu, Feb 20 2025 at 15:04, Thomas Gleixner wrote:
> > On Thu, Feb 20 2025 at 09:49, Eric Dumazet wrote:
> >> On Thu, Feb 20, 2025 at 9:09 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> >>> > This allows the following patch to use RCU.
> >>>
> >>> Your patch ordering is slightly off by two :)
> >>>
> >>> And it fails to explain for what RCU can be used....
> >>
> >> This is explained in the following patches.
> >
> > The changelog of a patch has to be self contained. The 'following patch'
> > has no meaning when the patch is merged.
>
> That said, please just fold this into the patch which actually does this RCU
> lookup upfront. The change is trivial enough that it does not really
> require to be seperate. If the lockless increment would cause issues,
> then the subsequent RCU lookup is the least of the worries :)

I can squash all patches into a single one if you prefer.

On Thu, Feb 20 2025 at 16:55, Eric Dumazet wrote:
> On Thu, Feb 20, 2025 at 3:32 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> On Thu, Feb 20 2025 at 15:04, Thomas Gleixner wrote:
>> > On Thu, Feb 20 2025 at 09:49, Eric Dumazet wrote:
>> >> On Thu, Feb 20, 2025 at 9:09 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>> >>> > This allows the following patch to use RCU.
>> >>>
>> >>> Your patch ordering is slightly off by two :)
>> >>>
>> >>> And it fails to explain for what RCU can be used....
>> >>
>> >> This is explained in the following patches.
>> >
>> > The changelog of a patch has to be self contained. The 'following patch'
>> > has no meaning when the patch is merged.
>>
>> That said, please just fold this into the patch which actually does this RCU
>> lookup upfront. The change is trivial enough that it does not really
>> require to be seperate. If the lockless increment would cause issues,
>> then the subsequent RCU lookup is the least of the worries :)
>
> I can squash all patches into a single one if you prefer.

Please don't. The wraparound race prevention has nothing to do with the
lookup optimization.

Thanks,

        tglx