Kamil,
# Sorry for late response.
On Fri, Dec 13, 2013 at 05:00:04PM -0600, Kamil Iskra wrote:
> On Fri, Dec 13, 2013 at 14:59:02 -0500, Naoya Horiguchi wrote:
>
> Hi Naoya,
>
> > On Thu, Dec 12, 2013 at 04:25:27PM -0600, Kamil Iskra wrote:
> > > Please find below a trivial patch that changes the sending of BUS_MCEERR_AO
> > > SIGBUS signals so that they can be handled by an arbitrary thread of the
> > > target process. The current implementation makes it impossible to create a
> > > separate, dedicated thread to handle such errors, as the signal is always
> > > sent to the main thread.
> > This can be done in application side by letting the main thread create a
> > dedicated thread for error handling, or by waking up existing/sleeping one.
> > It might not be optimal in overhead, but note that an action optional error
> > does not require to be handled ASAP. And we need only one process to handle
> > an action optional error, so no need to send SIGBUS(BUS_MCEERR_AO) for every
> > processes/threads.
>
> I'm not sure if I understand. "letting the main thread create a dedicated
> thread for error handling" is exactly what I was trying to do -- the
> problem is that SIGBUS(BUS_MCEERR_AO) signals are never sent to that
> thread, which is contrary to common expectations. The signals are sent to
> the main thread only, even if SIGBUS is masked there.
I think that what your patch suggests is that "letting the dedicated thread
get SIGBUS(BUS_MCEERR_AO) directly (not via the main thread) from kernel."
It's a bit different from what I meant in the previous email.
> Just to make sure that we're on the same page, here's a testcase that
> demonstrates the problem I'm trying to fix (I should've sent it the first
> time; sorry for being lazy):
Thanks. And I see your problem.
>
> #include <pthread.h>
> #include <signal.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <sys/prctl.h>
> #include <unistd.h>
>
> void sigbus_handler(int sig, siginfo_t* si, void* ucontext)
> {
> printf("SIGBUS caught by thread %ld, code %d, addr %p\n",
> (long)pthread_self(), si->si_code, si->si_addr);
> }
>
> void* sigbus_thread(void* arg)
> {
> printf("sigbus thread: %ld\n", (long)pthread_self());
> for (;;)
> pause();
> }
>
> int main(void)
> {
> struct sigaction sa;
> sigset_t mask;
> char* buf;
> pthread_t thread_id;
>
> prctl(PR_MCE_KILL, PR_MCE_KILL_SET, PR_MCE_KILL_EARLY, 0, 0);
>
> posix_memalign((void*)&buf, 4096, 4096);
> buf[0] = 0;
> printf("convenient address to hard offline: %p\n", buf);
>
> sa.sa_sigaction = sigbus_handler;
> sigemptyset(&sa.sa_mask);
> sa.sa_flags = SA_SIGINFO;
> sigaction(SIGBUS, &sa, NULL);
>
> pthread_create(&thread_id, NULL, sigbus_thread, NULL);
>
> sigemptyset(&mask);
> sigaddset(&mask, SIGBUS);
> pthread_sigmask(SIG_BLOCK, &mask, NULL);
>
> printf("main thread: %ld\n", (long)pthread_self());
>
> for (;;)
> pause();
>
> return 0;
> }
>
>
> This testcase uses a very common signal handling strategy in multithreaded
> programs: masking signals in all threads but one, created specifically for
> signal handling. It works just fine if I send it SIGBUS from another
> terminal using "kill". It does not work if I offline the page: the signal
> is routed to the main thread, where it's marked as pending; nothing gets
> printed out.
>
> As you were so kind to point out, SIGBUS(BUS_MCEERR_AO) does not need to be
> handled ASAP, so why should the kernel handle it differently to other
> non-critical signals? The current behavior seems inconsistent, and there
> is no convenient workaround (as a library writer, I have no control over
> the actions of the main thread).
I'm not sure if current implementation is intentional or not,
but I understand about the inconsistency.
> > And another concern is if this change can affect/break existing applications.
> > If it can, maybe you need to add (for example) a prctl attribute to show that
> > the process expects kernel to send SIGBUS(BUS_MCEERR_AO) only to the main
> > thread, or to all threads belonging to the process.
>
> I understand your concern. However, I believe that having
> SIGBUS(BUS_MCEERR_AO) behave consistently with established POSIX standards
> for signal handling outhweighs the concerns over potential
> incompatibilities, especially with a feature that is currently used by a
> very small subset of applications.
OK, and in this case the effect on existing multi-threaded applications seems
to be small (just small degradation of availability, but no kernel panic nor
data lost,) so I think it's acceptable.
I want to agree with your patch, so could you repost the patch with patch
description? git-format-patch will help you.
Thanks,
Naoya Horiguchi
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>