On Wed, Jun 14, 2017 at 04:43:45PM -0700, Paul E. McKenney wrote: > On Wed, Jun 14, 2017 at 04:06:39PM -0700, Luis R. Rodriguez wrote: > > These waits don't even check for the return value for interruption, > > using the non-killable variants means we could be killed by other > > signals than SIGKILL, this is fragile. > > A number of people asserted that kthreads could never catch signals. > I checked this at the time, and it seemed like this was the case, and > the call to ignore_signals() seems to confirm this. > > So it looks to me like RCU doesn't care about this change (give or > take any affect on the load average, anyway), but there is much that I > don't know about Linux-kernel signal handling, so please feel free to > educate me.
Thanks, I had seen the kthread but figured best to ask, just got into parnaoia mode. If we were to do a sanity check for usage we'd then have to white list when kthreads are used, however since we don't care to be interrupted why not use a wait which is also explicit about our current uninterruptible state? > > Signed-off-by: Luis R. Rodriguez <[email protected]> > > --- > > > > The killable swaits were just posted [1] as part of a series where SIGCHLD > > was detected as interrupting and killing kernel calls waiting using > > non-killable swaits [1]. The fragility here made curious about other callers > > and seeing if they really meant to use such broad wait which captures a lot > > of signals. > > > > I can't see why we'd want to have these killed by other signals, specialy > > since it seems we don't even check for the return value... Granted to abort > > properly we'd have to check for the return value for -ERESTARTSYS, but yeah, > > none of this is done, so it would seem we don't want fragile signals > > interrupting these ? > > The later WARN_ON(signal_pending(current)) complains if a signal somehow > makes it to this task. Assuming that the signal is nonfatal, anyway. I see, how about just using swait_event_timeout() and removing the WARN_ON()? Is there a reason for having the interruptible ? > > Also can someone confirm if the original change of to swait_event_timeout() > > from wait_event_interruptible_timeout() was actually intentional on > > synchronize_sched_expedited_wait() on commit abedf8e2419fb ("rcu: Use simple > > wait queues where possible in rcutree") ? I can't easily confirm. > > This is also called from a workqueue (at least once the core_initcall()s > get done), which again should mean no signals. A WARN_ON(ret < 0) > complains if this ever changes. So it should be OK that this is > swait_event_timeout(). And expedited grace periods are supposed to > get done quickly, so effect on the load average should be negligible. Great thanks. > Or am I missing something? No, just got into parnoia mode and better to ask than be sorry later! Luis
