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

[Stanislav Kinsbursky]

15.10.2012 19:33, Pavel Emelyanov пишет:
> On 10/15/2012 07:30 PM, Stanislav Kinsbursky wrote:
>> 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.
>
> It didn't behave _that_ _before_ this patch.
>

Yes, it did.

# man timer_create

ERRORS
        EAGAIN Temporary error during kernel allocation of timer structures.


>> This is already a part of timer_create() API.
>
> It doesn't mean you can report "I'm confused, try one more time please".
>

O_o

>> Actually, I can check for 0 after add and return -EAGAIN in this case.
>
> -1
>

Please, share the better idea, if you have it.




-- 
Best regards,
Stanislav Kinsbursky