linux-kernel - Re: [RFC PATCH] posix timers: allocate timer id per task

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <507D15EE.3020305@parallels.com>
Date:	Tue, 16 Oct 2012 12:08:14 +0400
From:	Stanislav Kinsbursky <skinsbursky@...allels.com>
To:	Thomas Gleixner <tglx@...utronix.de>
CC:	"peterz@...radead.org" <peterz@...radead.org>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devel@...nvz.org" <devel@...nvz.org>
Subject: Re: [RFC PATCH] posix timers: allocate timer id per task

15.10.2012 23:08, Thomas Gleixner пишет:
> 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.
>

This is probably a misunderstanding.
I meant process process.

> 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 ?
>

The reason is that CRIU have to support single processes regardless to namespaces.

>> 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?
>

I don't really want it.
I just tried to preserve existent logic. I.e. currently one process can create 
2^31 posix timers (if no other processes tried to do the same).
But the amount can  be decreased, of course.

>> +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
>

Yep, Eric already pointed to it.
I'll try to fix this problem, if the idea with hash table suits in general.
Thanks.

> Thanks,
>
> 	tglx
>


-- 
Best regards,
Stanislav Kinsbursky
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/