From: Linus Torvalds > Sent: 04 June 2019 22:27 > Ugh. I htink this is correct, but I wish we had a better and more > intuitive interface. > > In particular, since restore_user_sigmask() basically wants to check > for "signal_pending()" anyway (to decide if the mask should be > restored by signal handling or by that function), I really get the > feeling that a lot of these patterns like > > > - restore_user_sigmask(ksig.sigmask, &sigsaved); > > - if (signal_pending(current) && !ret) > > + > > + interrupted = signal_pending(current); > > + restore_user_sigmask(ksig.sigmask, &sigsaved, interrupted); > > + if (interrupted && !ret) > > ret = -ERESTARTNOHAND; > > are wrong to begin with, and we really should aim for an interface > which says "tell me whether you completed the system call, and I'll > give you an error return if not". > > How about we make restore_user_sigmask() take two return codes: the > 'ret' we already have, and the return we would get if there is a > signal pending and w're currently returning zero. > > IOW, I think the above could become > > ret = restore_user_sigmask(ksig.sigmask, &sigsaved, ret, -ERESTARTHAND); > > instead if we just made the right interface decision. I think we should tell restore_user_sigmask() whether it should cause any signal handles to be run (using the current mask) and it should tell us whether it would run any (ie if it deferred restoring the mask to syscall exit). So the above would (probably) be: if (restore_user_sigmask(ksig.sigmask, &sigsaved, true) && !ret) ret = -ERESTARTNOHAND; epoll() would have: if (restore_user_sigmask(xxx.sigmask, &sigsaved, !ret || ret == -EINTR)) ret = -EINTR; I also think it could be simplified if code that loaded the 'user sigmask' saved the old one in 'current->saved_sigmask' (and saved that it had done it). You'd not need 'sigsaved' nor pass the user sigmask address into the restore function. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
