All of lore.kernel.org
 help / color / mirror / Atom feed
From: Manfred Spraul <manfred@colorfullife.com>
To: Pavel Machek <pavel@ucw.cz>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>,
	linux-kernel@vger.kernel.org, "Agarwal,
	Lomesh" <lomesh.agarwal@intel.com>,
	Nigel Cunningham <nigel@nigel.suspend2.net>
Subject: Re: which signal is sent to freeze process?
Date: Tue, 24 Jul 2007 20:48:36 +0200	[thread overview]
Message-ID: <46A64984.1070808@colorfullife.com> (raw)
In-Reply-To: <46A50AF0.70305@colorfullife.com>

[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]

> Hi!
>
> Can you generate small testcase that demonstrates the problem?
>
> > Then what would be the correct way to handle resume process. The other
> > way of course is to make all the applications check the errno in case of
> > failure. But that seems more more problematic then system call checking.
> > What do you say?
>
> Hmm, does that testcase behave correctly over SIGSTOP/SIGCONT? I'm not
> saying kernel behaves nicely here, but perhaps fixing the apps to
> check errno properly is the right thing to do? :-)
Perhaps the kernel should use ERESTARTNOHAND instead of EINTR?
The current code is more than odd:
- select() and sys_ppoll() both use ERESTARTNOHAND (i.e.: 
the functions do not return to user space with SIGSTOP/SIGCONT or freezer())

- sys_poll() uses EINTR (i.e.: SIGSTOP/SIGCONT/freezer() return to user space)

Attached is a patch that switches sys_poll to ERESTARTNOHAND and a poll test app.
Boot tested with FC6.

What do you think? With ERESTARTNOHAND, poll would only return to user space if the app has a SIGCONT handler installed.

--
	Manfred


[-- Attachment #2: patch-poll-ERESTARTNOHAND --]
[-- Type: text/plain, Size: 783 bytes --]

--- 2.6/fs/select.c	2007-05-20 09:52:32.000000000 +0200
+++ build-2.6/fs/select.c	2007-07-23 22:10:21.000000000 +0200
@@ -723,7 +723,7 @@
   	}
 	err = fdcount;
 	if (!fdcount && signal_pending(current))
-		err = -EINTR;
+		err = -ERESTARTNOHAND;
 out_fds:
 	walk = head;
 	while(walk!=NULL) {
@@ -794,7 +794,7 @@
 	ret = do_sys_poll(ufds, nfds, &timeout);
 
 	/* We can restart this syscall, usually */
-	if (ret == -EINTR) {
+	if (ret == -ERESTARTNOHAND) {
 		/*
 		 * Don't restore the signal mask yet. Let do_signal() deliver
 		 * the signal on the way back to userspace, before the signal
@@ -805,7 +805,6 @@
 					sizeof(sigsaved));
 			set_thread_flag(TIF_RESTORE_SIGMASK);
 		}
-		ret = -ERESTARTNOHAND;
 	} else if (sigmask)
 		sigprocmask(SIG_SETMASK, &sigsaved, NULL);
 

[-- Attachment #3: sp.cpp --]
[-- Type: text/x-c++src, Size: 789 bytes --]

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/poll.h>
#include <sys/time.h>
#include <sys/resource.h>
#include <errno.h>

int main(int argc, char **argv)
{
	int delay, ret;
	struct pollfd pfd;
	int pipesfd[2];

	printf("sp <delay>\n");

	if (argc != 2)
		return 1;
	delay = atoi(argv[1]);
	printf("delay %d.\n", delay);
	if (delay < 0 || delay > 1000)
		return 2;

	if (pipe(pipesfd) != 0) {
		fprintf(stderr, "pipe failed with errno %d.\n", errno);
		return 3;
	}
	pfd.fd = pipesfd[0];
	pfd.events = POLLIN;

	if (!fork()) {
		sleep(delay);
		write(pipesfd[1], "", 1);;
		return 0;
	}
	sleep(1);
	ret = poll(&pfd, 1, -1);
	printf("poll returned %d.\n", ret);

	if (pfd.revents)
		printf("   events %8xh revents %8xh.\n", pfd.events, pfd.revents);

	return 0;
}

  reply	other threads:[~2007-07-24 18:51 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-23 19:52 which signal is sent to freeze process? Manfred Spraul
2007-07-23 20:11 ` Rafael J. Wysocki
2007-07-23 20:09   ` Manfred Spraul
2007-07-24 18:48     ` Manfred Spraul [this message]
2007-07-25 19:19       ` Rafael J. Wysocki
2007-07-26 21:12       ` Agarwal, Lomesh
2007-07-31  7:55       ` Pavel Machek
  -- strict thread matches above, loose matches on Subject: below --
2007-07-18 23:42 Agarwal, Lomesh
2007-07-19  2:18 ` Nigel Cunningham
2007-07-19  4:09   ` Agarwal, Lomesh
2007-07-19  4:59     ` Nigel Cunningham
2007-07-19 21:06       ` Agarwal, Lomesh
2007-07-19 22:02         ` Rafael J. Wysocki
2007-07-19 22:19         ` Nigel Cunningham
2007-07-19 23:22           ` Agarwal, Lomesh
2007-07-20 11:24             ` Rafael J. Wysocki
2007-07-20 18:07               ` Agarwal, Lomesh
2007-07-20 22:10                 ` Rafael J. Wysocki
2007-07-23 18:38                   ` Agarwal, Lomesh
2007-07-23 19:25                     ` Rafael J. Wysocki
2007-07-23 19:31                       ` Agarwal, Lomesh
2007-07-24 16:54                         ` Pavel Machek
2007-07-23 20:57                   ` Agarwal, Lomesh
2007-07-23 21:50                     ` Rafael J. Wysocki
2007-07-23 22:18                       ` Agarwal, Lomesh
2007-07-24  9:44                         ` Rafael J. Wysocki
2007-07-19 22:02       ` Rafael J. Wysocki
2007-07-25 13:41       ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=46A64984.1070808@colorfullife.com \
    --to=manfred@colorfullife.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lomesh.agarwal@intel.com \
    --cc=nigel@nigel.suspend2.net \
    --cc=pavel@ucw.cz \
    --cc=rjw@sisk.pl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.