Re: [PATCH v1] kthread/smpboot: Serialize kthread parking against wakeup

From: Andrea Parri
Date: Thu Apr 26 2018 - 12:02:44 EST


On Thu, Apr 26, 2018 at 10:41:31AM +0200, Peter Zijlstra wrote:

[...]

> +/*
> + * Special states are those that do not use the normal wait-loop pattern. See
> + * the comment with set_special_state().
> + */
> +#define is_special_state(state) \
> + ((state) == TASK_DEAD || \
> + (state) == TASK_STOPPED)
> +
> #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
>
> +/*
> + * Assert we don't use the regular *set_current_state() helpers for special
> + * states. See the comment with set_special_state().
> + */
> +#define assert_special_state(state) WARN_ON_ONCE(is_special_state(state))

Nitpicking, this name suggests "Shout if the state is NOT special" to me:
maybe,

#define assert_special_state(state) WARN_ON_ONCE(!is_special_state(state))
#define assert_regular_state(state) WARN_ON_ONCE(is_special_state(state))

or just do with the WARN_ON_ONCE()s ?

Andrea


> +
> #define __set_current_state(state_value) \
> do { \
> + assert_special_state(state_value); \
> current->task_state_change = _THIS_IP_; \
> current->state = (state_value); \
> } while (0)
> +
> #define set_current_state(state_value) \
> do { \
> + assert_special_state(state_value); \
> current->task_state_change = _THIS_IP_; \
> smp_store_mb(current->state, (state_value)); \
> } while (0)
>
> +#define set_special_state(state_value) \
> + do { \
> + unsigned long flags; /* may shadow */ \
> + WARN_ON_ONCE(!is_special_state(state_value)); \
> + raw_spin_lock_irqsave(&current->pi_lock, flags); \
> + current->task_state_change = _THIS_IP_; \
> + current->state = (state_value); \
> + raw_spin_unlock_irqrestore(&current->pi_lock, flags); \
> + } while (0)