public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Stanislav Kinsbursky <skinsbursky@parallels.com>
Cc: peterz@infradead.org, mingo@redhat.com,
	linux-kernel@vger.kernel.org, devel@openvz.org
Subject: Re: [RFC PATCH] posix timers: allocate timer id per task
Date: Mon, 15 Oct 2012 21:08:18 +0200 (CEST)	[thread overview]
Message-ID: <alpine.LFD.2.02.1210152105090.5541@ionos> (raw)
In-Reply-To: <20121015161559.7806.72762.stgit@localhost.localdomain>

On Mon, 15 Oct 2012, Stanislav Kinsbursky wrote:

> This patch is required CRIU project (www.criu.org).
> To migrate processes with posix timers we have to make sure, that we can
> restore posix timer with proper id.
> Currently, this is not true, because timer ids are allocated globally.
> So, this is precursor patch and it's purpose is make posix timer id to be
> allocated per task.

You can't allocate them per task. posix timers are process wide.

What's the reason why you did not make the posix timer ids per name
space instead of going down to the per process level ?

> Patch replaces global idr with global hash table for posix timers and
> makes timer ids unique not globally, but per task. Next free timer id is type
> of integer and stored on signal struct (posix_timer_id). If free timer id
> reaches negative value on timer creation, it will be dropped to zero and
> -EAGAIN will be returned to user.

So you want to allow 2^31 posix timers created for a single process?

> +static struct k_itimer *__posix_timers_find(struct hlist_head *head, struct signal_struct *sig, timer_t id)
> +{
> +	struct hlist_node *node;
> +	struct k_itimer *timer;
> +
> +	hlist_for_each_entry(timer, node, head, t_hash) {
> +		if ((timer->it_signal == sig) && (timer->it_id == id))
> +			return timer;
> +	}
> +	return NULL;
> +}
> +
> +static struct k_itimer *posix_timer_find(timer_t id, unsigned long *flags)
> +{
> +	struct k_itimer *timer;
> +	struct signal_struct *sig = current->signal;
> +	struct hlist_head *head = &posix_timers_hashtable[hash(sig, id)];
> +
> +	spin_lock_irqsave(&hash_lock, *flags);

This is not going to fly. You just reintroduced a massive scalability
problem. See commit 8af08871

Thanks,

	tglx

  parent reply	other threads:[~2012-10-15 19:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-15 16:17 [RFC PATCH] posix timers: allocate timer id per task Stanislav Kinsbursky
2012-10-15 16:34 ` Eric Dumazet
2012-10-16  7:57   ` Stanislav Kinsbursky
2012-10-15 17:04 ` Peter Zijlstra
2012-10-16  8:00   ` Stanislav Kinsbursky
2012-10-15 19:08 ` Thomas Gleixner [this message]
2012-10-16  8:08   ` Stanislav Kinsbursky

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=alpine.LFD.2.02.1210152105090.5541@ionos \
    --to=tglx@linutronix.de \
    --cc=devel@openvz.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=skinsbursky@parallels.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox