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

[Stanislav Kinsburskiy]

26.05.2016 15:36, Kirill Tkhai пишет:
> В Чт, 26/05/2016 в 15:21 +0200, Stanislav Kinsburskiy пишет:
>> 26.05.2016 15:20, Kirill Tkhai пишет:
>>> В Ср, 16/03/2016 в 16:17 +0300, Stanislav Kinsburskiy пишет:
>>>> 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 a couple of simple helpers to find unused
>>>> file
>>>> descriptor:
>>>> 1) fd_is_used() does exactly what it is named
>>>> 2) find_unused_fd() returns "hint_fd" if unused or last used
>>>> descriptor plus
>>>> one.
>>>>
>>>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
>>>> ---
>>>>    criu/files.c            |    3 +++
>>>>    criu/include/files.h    |   33
>>>> +++++++++++++++++++++++++++++++++
>>>>    criu/include/rst_info.h |    1 +
>>>>    3 files changed, 37 insertions(+)
>>>>
>>>> diff --git a/criu/files.c b/criu/files.c
>>>> index 8a7d85f..40bb13a 100644
>>>> --- a/criu/files.c
>>>> +++ b/criu/files.c
>>>> @@ -646,6 +646,8 @@ static int collect_fd(int pid, FdinfoEntry
>>>> *e,
>>>> struct rst_info *rst_info)
>>>>    	else
>>>>    		collect_gen_fd(new_le, rst_info);
>>>>    
>>>> +	collect_used_fd(new_le, rst_info);
>>>> +
>>>>    	list_add_tail(&new_le->desc_list, &le->desc_list);
>>>>    	new_le->desc = fdesc;
>>>>    
>>>> @@ -686,6 +688,7 @@ int prepare_fd_pid(struct pstree_item *item)
>>>>    	pid_t pid = item->pid.virt;
>>>>    	struct rst_info *rst_info = rsti(item);
>>>>    
>>>> +	INIT_LIST_HEAD(&rst_info->used);
>>>>    	INIT_LIST_HEAD(&rst_info->fds);
>>>>    	INIT_LIST_HEAD(&rst_info->eventpoll);
>>>>    	INIT_LIST_HEAD(&rst_info->tty_slaves);
>>>> diff --git a/criu/include/files.h b/criu/include/files.h
>>>> index 5db6ab1..d38dbfd 100644
>>>> --- a/criu/include/files.h
>>>> +++ b/criu/include/files.h
>>>> @@ -67,6 +67,7 @@ struct fdinfo_list_entry {
>>>>    	struct list_head	desc_list;	/* To chain
>>>> on  @fd_info_head */
>>>>    	struct file_desc	*desc;		/*
>>>> Associated
>>>> file descriptor */
>>>>    	struct list_head	ps_list;	/* To
>>>> chain  per-
>>>> task files */
>>>> +	struct list_head	used_list;	/* To chain
>>>> per-
>>>> task used fds */
>>>>    	int			pid;
>>>>    	futex_t			real_pid;
>>>>    	FdinfoEntry		*fe;
>>>> @@ -108,11 +109,43 @@ struct file_desc_ops {
>>>>    	char *			(*name)(struct file_desc
>>>> *,
>>>> char *b, size_t s);
>>>>    };
>>>>    
>>>> +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) {
>>>> +		if (new_fle->fe->fd < fle->fe->fd)
>>>> +			break;
>>>> +	}
>>>> +
>>>> +	list_add_tail(&new_fle->used_list, &fle->used_list);
>>>> +}
>>>> +
>>>>    static inline void collect_gen_fd(struct fdinfo_list_entry
>>>> *fle,
>>>> struct rst_info *ri)
>>>>    {
>>>>    	list_add_tail(&fle->ps_list, &ri->fds);
>>>>    }
>>>>    
>>>> +static inline bool fd_is_used(struct list_head *head, int fd)
>>>> +{
>>>> +	struct fdinfo_list_entry *fle;
>>>> +
>>>> +	list_for_each_entry(fle, head, used_list) {
>>>> +		if (fle->fe->fd == fd)
>>>> +			return true;
>>>> +	}
>>>> +
>>>> +	return false;
>>>> +}
>>>> +
>>>> +static inline unsigned int find_unused_fd(struct list_head
>>>> *head,
>>>> int hint_fd)
>>>> +{
>>>> +	if ((hint_fd >= 0) && (!fd_is_used(head, hint_fd)))
>>>> +		return hint_fd;
>>>> +	/* Return last used fd +1 */
>>>> +	return list_entry(head->prev, typeof(struct
>>>> fdinfo_list_entry), used_list)->fe->fd + 1;
>>> What is guarantee that we have a space between the last fd and
>>> INT/UINT_MAX?
>>>
>> Frankly speaking, there is no garantee.
> Ok. Is this behavior expected for autofs (because of true hint_fd etc)?
> I try to understand if it's need just for me or it's common question.

Frankly, I just missed/didn't pay enough attention to this point.
It should be fixed (although this is an exotic case).
The best way, from my POW, would be to fall back to loop over all 
existent descriptors in attempt to find an unused one in case of last_fd 
== INT_MAX.

>>>> +}
>>>> +
>>>>    struct file_desc {
>>>>    	u32			id;		/* File
>>>> id,
>>>> unique */
>>>>    	struct hlist_node	hash;		/*
>>>> Descriptor
>>>> hashing and lookup */
>>>> diff --git a/criu/include/rst_info.h b/criu/include/rst_info.h
>>>> index b6d378e..562e2f0 100644
>>>> --- a/criu/include/rst_info.h
>>>> +++ b/criu/include/rst_info.h
>>>> @@ -27,6 +27,7 @@ struct fdt {
>>>>    struct _MmEntry;
>>>>    
>>>>    struct rst_info {
>>>> +	struct list_head	used;
>>>>    	struct list_head	fds;
>>>>    	struct list_head	eventpoll;
>>>>    	struct list_head	tty_slaves;
>>>>
>>>> _______________________________________________
>>>> CRIU mailing list
>>>> CRIU at openvz.org
>>>> https://lists.openvz.org/mailman/listinfo/criu