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/

Reply via email to