# Re: \[PATCH 2/3\] signalfd: add ability to return siginfo in a raw format (v2) - List: linux-api - 4 messages, 4 authors - Page: https://ratatoskr.run/linux-api/2012/12/70222/t ## Re: \[PATCH 2/3\] signalfd: add ability to return siginfo in a raw format (v2) - From: Oleg Nesterov \ - Date: 2012-12-28 16:14:36 - Message: https://ratatoskr.run/linux-api/2012/12/70222.md ```text 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. ``` ## Re: \[PATCH 2/3\] signalfd: add ability to return siginfo in a raw format (v2) - From: Andrey Wagin \ - Date: 2013-01-10 09:47:40 - Message: https://ratatoskr.run/linux-api/2013/01/70434.md ```text 2012/12/28 Oleg Nesterov [ref]: > 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? I won't argue too:), but I think the flag is more flexiable. The flag allows to get raw siginfo from read(), it may be useful not only for checkpoint/restore. pread with offset = 0 is equal to read(), so the interface with the flag is more clear. > >> @@ -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. > ``` ## Re: \[PATCH 2/3\] signalfd: add ability to return siginfo in a raw format (v2) - From: Michael Kerrisk (man-pages) \ - Date: 2013-01-10 22:45:17 - Message: https://ratatoskr.run/linux-api/2013/01/70445.md ```text On Thu, Jan 10, 2013 at 10:47 AM, Andrey Wagin [ref] wrote: > 2012/12/28 Oleg Nesterov [ref]: >> 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? > > I won't argue too:), but I think the flag is more flexiable. The flag > allows to get raw siginfo from read(), it may be useful not only for > checkpoint/restore. pread with offset = 0 is equal to read(), so the > interface with the flag is more clear. Special casing pread() behavior for signalfd() so that SFD_RAW is not required seems only to make the ugliness worse... While I appreciate the need for this API, multiplexing a queue selector and a positional value into what is normally a byte offset in pread() is really bizarre. This is ugly and I wonder if something better is possible, though so far I have no suggestions beyond adding a new system call. -- Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Author of "The Linux Programming Interface"; http://man7.org/tlpi/ ``` ## Re: \[PATCH 2/3\] signalfd: add ability to return siginfo in a raw format (v2) - From: Oleg Nesterov \ - Date: 2013-01-12 18:56:09 - Message: https://ratatoskr.run/linux-api/2013/01/70490.md ```text On 01/10, Andrey Wagin wrote: > > 2012/12/28 Oleg Nesterov [ref]: > >> > >> 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? > > I won't argue too:), but I think the flag is more flexiable. The flag > allows to get raw siginfo from read(), it may be useful not only for > checkpoint/restore. pread with offset = 0 is equal to read(), so the > interface with the flag is more clear. OK, I agree. Oleg. ```