All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: "Sebastian Andrzej Siewior" <bigeasy@linutronix.de>,
	linux-kernel@vger.kernel.org,
	"André Almeida" <andrealmeid@igalia.com>,
	"Darren Hart" <dvhart@infradead.org>,
	"Davidlohr Bueso" <dave@stgolabs.net>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Juri Lelli" <juri.lelli@redhat.com>,
	"Valentin Schneider" <vschneid@redhat.com>,
	"Waiman Long" <longman@redhat.com>
Subject: Re: [RFC PATCH 2/3] futex: Add basic infrastructure for local task local hash.
Date: Wed, 30 Oct 2024 22:08:19 +0100	[thread overview]
Message-ID: <20241030210819.GS9767@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <87r080306d.ffs@tglx>

On Mon, Oct 28, 2024 at 01:02:34PM +0100, Thomas Gleixner wrote:

> That's what we did with the original series, but with this model it's
> daft. What we maybe could do there is:

Not sure what's daft -- a single JVM running on 400+ CPUs with 4
hashbuckets sounds awesome.

> 
> private_hash()
>    scoped_guard(rcu) {
>       hash = rcu_dereference(current->signal->futex_hash);

So I really do think mm_struct is a better place for this than signal
struct -- CLONE_SIGHAND is not mandatory when CLONE_VM.

I've long forgotten which JVM used the naked CLONE_VM, but there is some
creative code out there.

And futexes fundamentally live in the memory address space.

>       if (hash && rcuref_get(&hash->ref))
>          return hash;
>    }
> 
>    guard(spinlock_irq)(&task->sighand->siglock);
>    hash = current->signal->futex_hash;
>    if (hash && rcuref_get(&hash->ref))
>        return hash;
>    // Let alloc scale according to signal->nr_threads

  mm->mm_users

>    // alloc acquires a reference count
>    ....

It might make sense to have a prctl() setting that inhibits the hash
allocation entirely, reverting back to the global hash tables.

> And on fork do the following:
> 
>    scoped_guard(spinlock_irq, &task->sighand->siglock) {
>       hash = current->signal->futex_hash;
>       if (!hash || hash_size_ok())
>    	return hash;
> 
>       // Drop the initial reference, which forces the last
>       // user and subsequent new users into the respective
>       // slow paths, where they get stuck on sighand lock.
>       if (!rcuref_put(&hash->ref))
>         return;
> 
>       // rcuref_put() dropped the last reference
>       old_hash = realloc_hash(hash);
>       hash = current->signal->futex_hash;
>    }
>    kfree_rcu(old_hash);
>    return hash;
> 
> A similar logic is required when putting the last reference
> 
> futex_hash_put()
> {
>    if (!rcuref_put(&hash->ref))
>       return;
> 
>    scoped_guard(spinlock_irq, &task->sighand->siglock) {
>       // Fork might have raced with this
>       if (hash != current->signal->futex_hash)
>       	 return;
>       old_hash = realloc_hash(hash);
>    }
>    kfree_rcu(old_hash);  
> }

I'm not sure having that rehash under siglock is a fine idea. It's
convenient, no doubt, but urgh, could get expensive.

Another scheme would be to have 2 concurrent hash-tables for a little
while.

  reply	other threads:[~2024-10-30 21:08 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-26 22:34 [RFC PATCH 0/3] futex: Add support task local hash maps Sebastian Andrzej Siewior
2024-10-26 22:34 ` [RFC PATCH 1/3] futex: Create helper function to initialize a hash slot Sebastian Andrzej Siewior
2024-10-26 22:34 ` [RFC PATCH 2/3] futex: Add basic infrastructure for local task local hash Sebastian Andrzej Siewior
2024-10-27 12:19   ` Thomas Gleixner
2024-10-28 10:30     ` Sebastian Andrzej Siewior
2024-10-28 10:58       ` Thomas Gleixner
2024-10-28 11:00         ` Peter Zijlstra
2024-10-28 12:02           ` Thomas Gleixner
2024-10-30 21:08             ` Peter Zijlstra [this message]
2024-10-30 23:14               ` Thomas Gleixner
2024-10-31  9:13                 ` Peter Zijlstra
2024-10-28 10:16   ` Peter Zijlstra
2024-10-28 10:24     ` Sebastian Andrzej Siewior
2024-10-28 10:46       ` Peter Zijlstra
2024-10-28 13:10         ` Peter Zijlstra
2024-10-26 22:34 ` [RFC PATCH 3/3] futex: Use the task local hashmap Sebastian Andrzej Siewior
2024-10-28 10:22   ` Peter Zijlstra
2024-10-28 10:24     ` Sebastian Andrzej Siewior
2024-10-27 10:01 ` [RFC PATCH 0/3] futex: Add support task local hash maps Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241030210819.GS9767@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=andrealmeid@igalia.com \
    --cc=bigeasy@linutronix.de \
    --cc=dave@stgolabs.net \
    --cc=dvhart@infradead.org \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.