Re: [RFC PATCH] membarrier: handle nohz_full with expedited thread registration

From: Linus Torvalds
Date: Mon Jan 16 2017 - 15:15:36 EST


Excuse my french, but this looks like incredible shit to me.

On Mon, Jan 16, 2017 at 11:51 AM, Mathieu Desnoyers
<mathieu.desnoyers@xxxxxxxxxxxx> wrote:
> +
> +static int membarrier_register_expedited(struct task_struct *t)
> +{
> + struct rq *rq;
> +
> + if (t->membarrier_expedited == UINT_MAX)
> + return -EOVERFLOW;
> + rq = this_rq();
> + raw_spin_lock(&rq->lock);
> + t->membarrier_expedited++;
> + raw_spin_unlock(&rq->lock);
> + return 0;
> +}

Yeah, taking the rq lock with

(a) a random "rq" that isn't stable

(b) without disabling interrupts

(c) using an internal scheduler helper that isn't supposed to be used
externally

(d) when it doesn't even make any sense in the first place for a
per-thread value that is never modified by any other threads!

(e) .. and you expose this ABSOLUTELY SHIT as a random system call.

Oh, and the clone semantics make no sense either.

In fact, it just makes me doubt everything about the whole membarrier
concept, because it appears *so* terminally broken.

So unless I'm seriously missing something, this is just about the
worst piece of code I have seen this year.

No.

NO NO NO.

It really smells so broken that I'm wondering if I'm missing anything.
But I don't think I am. I think the code is just pure garbage.

Linus