[CRIU] [PATCH v7 06/15] files: new "used" files list introduced

Stanislav Kinsburskiy skinsbursky at virtuozzo.com
Wed Mar 16 05:21:03 PDT 2016



16.03.2016 13:19, Pavel Emelyanov пишет:
> On 03/16/2016 03:08 PM, Stanislav Kinsburskiy wrote:
>>
>> 16.03.2016 11:18, Pavel Emelyanov пишет:
>>> On 03/14/2016 05:59 PM, Stanislav Kinsburskiy wrote:
>>>> This list contains all per-process used file fdinfo's, sorted by fd number.
>>>> Will be used to safely create new artificial file descriptors and also allow
>>>> to recreate temporary descriptors with original number, if possible, like
>>>> AutoFS tries to preserve original pipe write end descriptor, when it was
>>>> closed.
>>>> This patch also adds simple helper to find unused file descriptor.
>>>> Return "hint" if unused or last used descriptor plus one.
>>> I finally got what's bad with this patch -- the fact that you put collect_used_fd
>>> into all the ->collect_fd callbacks. This is dangerous as one more such callback
>>> and the whole logic gets broken.
>>>
>>> Plz, call the collect_used_fd in the core fd collecting routine, in the files.c's
>>> collect_fd() one.
>>>
>>>> +static inline void collect_used_fd(struct fdinfo_list_entry *new_fle, struct rst_info *ri)
>>>> +{
>>>> +	struct fdinfo_list_entry *fle;
>>>> +
>>>> +	list_for_each_entry(fle, &ri->used, used_list) {
>>> You scan this list FULLY in find_unused_fd anyway, why keeping it sorted?
>> The reason is to find an unused fd, if hint_fd == -1 (or hint_fd != -1
>> but it's busy)
>> If we have unsorted list like: { 1, 10, 2 9, 8} and searching for unused
>> fd, how we will define one?
> That's OK, but if hint_fd == -1 you can break the list_for_each upon first
> item met, can't you?

You probably mean, that I don't need to iterate over the list if hint_fd 
== -1, but take prev item of the first item in the list?
Yes, I can do so. It's a nice optimisation.
But the list have to be sorted anyway.

>>>> +		if (new_fle->fe->fd < fle->fe->fd)
>>>> +			break;
>>>> +	}
>>>> +
>>>> +	list_add_tail(&new_fle->used_list, &fle->used_list);
>>>> +}
>>>> +
>> .
>>



More information about the CRIU mailing list