Re: [PATCH 8/9] psi: pressure stall information for CPU, memory, and IO

Peter Zijlstra
2018-08-06T15:25:28Z

On Mon, Aug 06, 2018 at 11:05:50AM -0400, Johannes Weiner wrote:
> Argh, that's right. This needs an explicit count if we want to access
> it locklessly. And you already said you didn't like that this is the
> only state not derived purely from the task counters, so maybe this is
> the way to go after all.
>
> How about something like this (untested)?

> +static inline void psi_switch(struct rq *rq, struct task_struct *prev,
> + struct task_struct *next)
> +{
> + if (psi_disabled)
> + return;
> +
> + if (unlikely(prev->flags & PF_MEMSTALL))
> + psi_task_change(prev, rq_clock(rq), TSK_RECLAIMING, 0);
> + if (unlikely(next->flags & PF_MEMSTALL))
> + psi_task_change(next, rq_clock(rq), 0, TSK_RECLAIMING);
> +}

Urgh... can't say I really like that.

I would really rather do that scheduler_tick() thing to avoid the remote
update. The tick is a lot less hot than the switch path and esp.
next->flags might be a cold line (prev->flags is typically the same line
as prev->state so we already have that, but I don't think anybody now
looks at next->flags or its line, so that'd be cold load).