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

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

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