Re: [PATCH 2/2] signals: eventpoll: set ->saved_sigmask at thestart

[Oleg Nesterov]

On 06/25, Oleg Nesterov wrote:
>
> But if we remove this WARN_ON() we can probably change
> set_restore_sigmask() to set TS_RESTORE_SIGMASK and
> do saved_mask = blocked.
>
> Perhaps it can even acccept "sigset_t *newmask" and do
> set_current_blocked().

So, Al, what do you think if we do something like

	--- x/arch/x86/include/asm/thread_info.h
	+++ x/arch/x86/include/asm/thread_info.h
	@@ -247,7 +247,6 @@ static inline void set_restore_sigmask(v
	 {
		struct thread_info *ti = current_thread_info();
		ti->status |= TS_RESTORE_SIGMASK;
	-	WARN_ON(!test_bit(TIF_SIGPENDING, (unsigned long *)&ti->flags));
	 }
	 static inline void clear_restore_sigmask(void)
	 {
	--- x/include/linux/signal.h
	+++ x/include/linux/signal.h
	@@ -249,6 +249,13 @@ extern void __set_current_blocked(const 
	 extern int show_unhandled_signals;
	 extern int sigsuspend(sigset_t *);
	 
	+static inline set_restore_xxx(sigset_t *mask)
	+{
	+	set_restore_sigmask();
	+	current->saved_sigmask = current->blocked;
	+	set_current_blocked(mask);
	+}
	+
	 struct sigaction {
	 #ifndef __ARCH_HAS_IRIX_SIGACTION
		__sighandler_t	sa_handler;

Then sys_epoll_pwait() (and other users) can simply do

	SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct epoll_event __user *, events,
			int, maxevents, int, timeout, const sigset_t __user *, sigmask,
			size_t, sigsetsize)
	{
		int error;
		/*
		 * If the caller wants a certain signal mask to be set during the wait,
		 * we apply it here.
		 */
		if (sigmask) {
			sigset_t ksigmask;

			if (sigsetsize != sizeof(sigset_t))
				return -EINVAL;
			if (copy_from_user(&ksigmask, sigmask, sizeof(ksigmask)))
				return -EFAULT;

			set_restore_xxx(&ksigmask);
		}

		error = sys_epoll_wait(epfd, events, maxevents, timeout);

		if (error != -EINTR)
			restore_saved_sigmask();

		return error;
	}

Hmm... and when I re-read your original email I am starting to think
that perhaps you proposed exactly this...

But I still think it would be better to do this change on top of the
cleanups I sent (fs/compat.c and fs/select.c should be updated too).

But, perhaps, it also makes sense to add

	void restore_saved_sigmask_if(bool cond)
	{
		if (cond)
			restore_saved_sigmask();
		else
			WARN_ON(TS_RESTORE_SIGMASK && !TIF_SIGPENDING);
	}

so that epoll_pwait() could do

	restore_saved_sigmask_if(error != -EINTR);

What do you think?

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/