Paul E. McKenney wrote: > > On Wed, Aug 17, 2005 at 06:35:03PM +0400, Oleg Nesterov wrote: > > > > Sorry, I don't understand you. CLONE_THREAD implies CLONE_SIGHAND, > > so we always need to lock one ->sighand. Could you please clarify? > > On the #3 and #4 code paths, the code assumes that the task-list lock > is held. So I was thinking of something (very roughly) as follows: > > #define SIGLOCK_HOLD_RCU (1 << 0) > #define SIGLOCK_HOLD_TASKLIST (1 << 1) > #define SIGLOCK_HOLD_SIGLOCK (1 << 2)
Oh, no, sorry for confusion. I meant this function should only lock ->sighand, nothing more, something like this: // must be called with preemtion disabled !!! struct sighand_struct *lock_task_sighand(struct task_struct *tsk, unsigned long *flags) { struct sighand_struct *sighand; // sighand = NULL; // if (!get_task_struct_rcu(tsk)) // goto out; for (;;) { sighand = tsk->sighand; if (unlikely(sighand == NULL)) break; spin_lock_irqsave(sighand->siglock, *flags); if (likely(sighand == tsk->sighand) goto out; spin_unlock_irqrestore(sighand->siglock, *flags); } // put_task_struct(tsk); out: return sighand; } static inline void unlock_task_sighand(struct task_struct *tsk, unsigned long *flags) { spin_unlock_irqrestore(tsk->sighand->siglock, flags); // put_task_struct(tsk); } int group_send_sig_info(int sig, struct siginfo *info, struct task_struct *p) { unsigned long flags; int ret; ret = check_kill_permission(sig, info, p); if (!ret && sig) { ret = -ESRCH; if (lock_task_sighand(p, &flags)) { ret = __group_send_sig_info(sig, info, p); unlock_task_sighand(p, &flags); } } return ret; } Currently the only user of it will be group_send_sig_info(), but I hope you have devil plans to kill the "tasklist_lock guards the very rare ->sighand change" finally :) Oleg. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/