Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 25 Jun 2018 14:51:05 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= 
        <u.kleine-koenig@pengutronix.de>
Cc:     linux-kernel@vger.kernel.org, Oleg Nesterov <oleg@redhat.com>,
        "Eric W . Biederman" <ebiederm@xmission.com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Gavin Schenk <g.schenk@eckelmann.de>, kernel@pengutronix.de
Subject: Re: [PATCH] RFC: siox: don't create a thread without starting it
Message-ID: <20180625125105.GZ2494@hirez.programming.kicks-ass.net>
References: <20180625102056.28468-1-u.kleine-koenig@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20180625102056.28468-1-u.kleine-koenig@pengutronix.de>
User-Agent: Mutt/1.10.0 (2018-05-17)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Jun 25, 2018 at 12:20:56PM +0200, Uwe Kleine-K?nig wrote:
> when I just boot without any other siox-related action. So the kthread (created
> in drivers/siox/siox-core.c:siox_master_register()) is never started.
> 
> While you could argue that there is little reason to not start the
> thread there also is little reason to actually do it.

Well, you really _should_ wake up the thread. That first wakeup really
is part of the whole 'create/setup' kthread pattern.

> peterz in #kernelnewbies said "[...] kernel/kthread.c:kthread() should
> really be using __set_current_state(TASK_IDLE), I suppose". This however
> seems to interfere with problems fixed in a076e4bca2fd ("freezer: fix
> kthread_create vs freezer theoretical race").

I don't think so, that patch has an issue with INTERRUPTIBLE, but IDLE
very much doesn't allow signals like INTERRUPTIBLE does.

> So I wonder where the real problem is and how it can be fixed.

Without the first wakeup, the kthread will not run the provided function
and we can therefore argue the creation is incomplete. I really feel you
should just wake the thing up to land in your own wait-condition-loop.

That said, irrespective of the whole UNINTERRUPTIBLE/IDLE thing, I find
this construct fairly fragile. We rely on not getting any spurious
wakeups without a 'special' state. The only reason this doesn't normally
happen is because it's a new task, but since it is already hashed, it
might well be possible to trick someone into sending a wakeup.