[CRIU] Re: [RFC PATCH] posix timers: allocate timer id per task

[Stanislav Kinsbursky]

15.10.2012 19:26, Pavel Emelyanov пишет:
> On 10/15/2012 07:24 PM, Stanislav Kinsbursky wrote:
>> 15.10.2012 19:16, Pavel Emelyanov пишет:
>>> On 10/15/2012 06:01 PM, Stanislav Kinsbursky wrote:
>>>> This 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.
>>>>
>>>> Hash table is size of page (4KB).
>>>>
>>>> Key is constructed as follows:
>>>> key = hash_ptr(current->signal) ^ hash_32(posix_timer_id);
>>>>
>>>> ---
>>>>    include/linux/posix-timers.h |    1
>>>>    include/linux/sched.h        |    4 +-
>>>>    kernel/posix-timers.c        |  112 ++++++++++++++++++++++++++++--------------
>>>>    3 files changed, 78 insertions(+), 39 deletions(-)
>>>>
>>>
>>>
>>>> +{
>>>> +	struct signal_struct *sig = current->signal;
>>>> +	struct hlist_head *head;
>>>> +	int ret = -ENOENT;
>>>> +
>>>> +	do {
>>>> +		spin_lock(&hash_lock);
>>>> +		head = &posix_timers_hashtable[hash(sig, sig->posix_timer_id)];
>>>> +		if (__posix_timers_find(head, sig, sig->posix_timer_id) == NULL) {
>>>> +			hlist_add_head(&timer->t_hash, head);
>>>> +			ret = sig->posix_timer_id++;
>>>> +		} else if (++sig->posix_timer_id < 0) {
>>>> +			sig->posix_timer_id = 0;
>>>> +			ret = -EAGAIN;
>>>
>>> What for? Overflow is OK, no need in restart.
>>>
>>
>> I was thinking, that we need some way to stop here in case of corner case, when
>> all id's are busy.
>> I'm don't think that anyone can reach even INT_MAX timers in one task, so
>> INT_MAX looks enough. This "EGAIN" will be returned to user.
>
> while (1) {
> 	add_timer()
> 	del_timer()
> }
>
> will overflow it very fast.
>

Sure. And add_timer() caller will receive -EAGAIN.
This is already a part of timer_create() API.
There should be some way to return from syscall in case of all id's are busy.
Actually, I can check for 0 after add and return -EAGAIN in this case.
Is it better from your pow?

>>>> +		}
>>>> +		spin_unlock(&hash_lock);
>>>> +	} while (ret == -ENOENT);
>>>> +	return ret;
>>>> +}
>>>
>>
>>
>
>


-- 
Best regards,
Stanislav Kinsbursky