[CRIU] [PATCH 4/5] timerfd: Implement c/r procedure, v2

[Pavel Emelyanov]

On 06/27/2014 01:57 PM, Cyrill Gorcunov wrote:
> On Fri, Jun 27, 2014 at 01:24:30AM +0400, Pavel Emelyanov wrote:
>> On 06/27/2014 01:11 AM, Cyrill Gorcunov wrote:
>>> On Fri, Jun 27, 2014 at 01:07:48AM +0400, Pavel Emelyanov wrote:
>>>>> +
>>>>> +		t = rst_mem_alloc(sizeof(*t), RM_PRIVATE);
>>>>
>>>> Why not in post_open callback of timerfd ops?
>>>
>>> It must be sequential slab of restorer memory.
>>
>> Make it the same as in tcp sockets: post_open_inet_sk->rst_tcp_socks_add.
>> It copies memory, but a) there are quite a few of timerfd-s and b) we'll
>> have more reasons to optimize this.
> 
> Attached.

> +static int timerfd_post_open(struct file_desc *d, int fd)
> +{
> +	struct timerfd_info *info = container_of(d, struct timerfd_info, d);
> +	TimerfdEntry *tfe = info->tfe;
> +	struct restore_timerfd *t;
> +
> +	rst_timerfd_nr++;
> +	rst_timerfd = xrealloc(rst_timerfd, rst_timerfd_len());
> +	if (!rst_timerfd)
> +		return -ENOMEM;
> +
> +	t = &rst_timerfd[rst_timerfd_nr - 1];
> +	t->fd				= fd;
> +	t->ticks			= (unsigned long)tfe->ticks;
> +	t->settime_flags		= tfe->settime_flags;
> +	t->val.it_interval.tv_sec	= (time_t)tfe->isec;
> +	t->val.it_interval.tv_nsec	= (long)tfe->insec;
> +	t->val.it_value.tv_sec		= (time_t)tfe->vsec;
> +	t->val.it_value.tv_nsec		= (long)tfe->vnsec;

How about TFD_TIMER_ABSTIME adjustments like in timerfd_open?

> +
> +	return 0;
> +}



> 
> +static int timerfd_open(struct file_desc *d)
> +{
> +	struct itimerspec v = { };

This v is write-only variable here.

> +	struct timerfd_info *info;
> +	TimerfdEntry *tfe;
> +	int tmp = -1;
> +
> +	info = container_of(d, struct timerfd_info, d);
> +	tfe = info->tfe;
> +	pr_info("Restoring id %#x clockid %d settime_flags %x ticks %llu "
> +		"it_value(%llu, %llu) it_interval(%llu, %llu)\n",
> +		tfe->id, tfe->clockid, tfe->settime_flags, (unsigned long long)tfe->ticks,
> +		(unsigned long long)tfe->vsec, (unsigned long long)tfe->vnsec,
> +		(unsigned long long)tfe->isec, (unsigned long long)tfe->insec);
> +
> +	tmp = timerfd_create(tfe->clockid, 0);
> +	if (tmp < 0) {
> +		pr_perror("Can't create for %#x\n", tfe->id);
> +		return -1;
> +	}
> +
> +	v.it_interval.tv_sec	= (time_t)tfe->isec;
> +	v.it_interval.tv_nsec	= (long)tfe->insec;
> +
> +	v.it_value.tv_sec	= (time_t)tfe->vsec;
> +	v.it_value.tv_nsec	= (long)tfe->vnsec;
> +
> +	if (tfe->settime_flags & TFD_TIMER_ABSTIME) {
> +		struct timespec ts = { };

So is ts.

> +
> +		/*
> +		 * We might need to adjust value because the checkpoint
> +		 * and restore procedure takes some time itself. Note
> +		 * we don't adjust nanoseconds, since the result may
> +		 * overflow the limit NSEC_PER_SEC FIXME
> +		 */
> +		if (clock_gettime(tfe->clockid, &ts)) {
> +			pr_perror("Can't get current time");
> +			goto err_close;
> +		}
> +
> +		v.it_value.tv_sec += (time_t)ts.tv_sec;
> +
> +		pr_debug("Ajust id %#x it_value(%llu, %llu) -> it_value(%llu, %llu)\n",
> +			 tfe->id, (unsigned long long)ts.tv_sec,
> +			 (unsigned long long)ts.tv_nsec,
> +			 (unsigned long long)v.it_value.tv_sec,
> +			 (unsigned long long)v.it_value.tv_nsec);
> +	}
> +
> +	if (rst_file_params(tmp, tfe->fown, tfe->flags)) {
> +		pr_perror("Can't restore params for %#x", tfe->id);
> +		goto err_close;
> +	}
> +
> +	return tmp;
> +
> +err_close:
> +	close_safe(&tmp);
> +	return -1;
> +}
> +

> +static struct file_desc_ops timerfd_desc_ops = {
> +	.type		= FD_TYPES__TIMERFD,
> +	.open		= timerfd_open,

Please write comment why we need post_open for timerfd-s.

> +	.post_open	= timerfd_post_open,
> +};
> +

> +static int timerfd_arm(struct task_restore_args *args)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < args->timerfd_n; i++) {
> +		struct restore_timerfd *t = &args->timerfd[i];
> +
> +		pr_debug("timerfd: arm for fd %d (%d)\n", t->fd, i);
> +
> +		ret  = sys_ioctl(t->fd, TFD_IOC_SET_TICKS, (unsigned long)&t->ticks);

Ticks can be restored in timerfd_open, can't they?

> +		ret |= sys_timerfd_settime(t->fd, t->settime_flags, &t->val, NULL);
> +		if (ret) {
> +			pr_err("Can't restore ticks/time for timerfd - %d\n", i);
> +			return ret;
> +		}
> +	}
> +	return 0;
> +}
> +