Re: [PATCH 2/3] signalfd: add ability to return siginfo in a raw format (v2)

[Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On 12/28, Andrey Vagin wrote:
>
> signalfd should be called with the flag SFD_RAW for that.
>
> signalfd_siginfo is not full for siginfo with a negative si_code.
> copy_siginfo_to_user() is copied a full siginfo to user-space, if
> si_code is negative.  signalfd_copyinfo() doesn't do that and can't be
> expanded, because it has not compatible format with siginfo_t.
>
> Another problem is that a constant __SI_* is removed from si_code.
> It's not a problem for usual applications, because they expect
> a defined type of siginfo (internal logic).
> When we want to dump pending signals, we can't predict a type of
> siginfo, so we should get it from kernel.
>
> The main idea of the raw format is that it should be enough for
> restoring exactly the same siginfo for the current process.
>
> This functionality is required for checkpointing pending signals.

And this should be used along with pread().

I won't argue, but perhaps we should simply postulate that pread()
always dumps siginfo in raw format and avoid the new flag?

> @@ -272,20 +311,35 @@ SYSCALL_DEFINE4(signalfd4, int, ufd, sigset_t __user *, user_mask,
>  	signotset(&sigmask);
>  
>  	if (ufd == -1) {
> +		struct file *file;
>  		ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
>  		if (!ctx)
>  			return -ENOMEM;
>  
>  		ctx->sigmask = sigmask;
>  
> +		ufd = get_unused_fd_flags(flags);
> +		if (ufd < 0) {
> +			kfree(ctx);
> +			goto out;
> +		}
> +
>  		/*
>  		 * When we call this, the initialization must be complete, since
>  		 * anon_inode_getfd() will install the fd.
>  		 */
> -		ufd = anon_inode_getfd("[signalfd]", &signalfd_fops, ctx,
> +		file = anon_inode_getfile("[signalfd]", &signalfd_fops, ctx,
>  				       O_RDWR | (flags & (O_CLOEXEC | O_NONBLOCK)));
> -		if (ufd < 0)
> +		if (IS_ERR(file)) {
> +			put_unused_fd(ufd);
> +			ufd = PTR_ERR(file);
>  			kfree(ctx);
> +			goto out;
> +		}
> +
> +		file->f_flags |= flags & SFD_RAW;
> +
> +		fd_install(ufd, file);

And this is needed because we want to set more bits in f_flags/f_mode.

I am wondering if we can change anon_inode_getfd/getfile somehow so
that the user can pass more flags...

Or. Currently fops->open() is never used if this file_operations is used
by anon_inode_get*. So perhaps we can change anon_inode_getfile() to call
fops->open() if it is not zero before return?

OK, this is almost off-topic, please ignore.

Oleg.