From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S966018AbcJ1QLN (ORCPT <rfc822;w@1wt.eu>);
        Fri, 28 Oct 2016 12:11:13 -0400
Received: from mx1.redhat.com ([209.132.183.28]:41942 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S937878AbcJ1QLK (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 28 Oct 2016 12:11:10 -0400
Date: Fri, 28 Oct 2016 18:11:06 +0200
From: Oleg Nesterov <oleg@redhat.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@amacapital.net>,
        Roman Pen <roman.penyaev@profitbricks.com>,
        Andy Lutomirski <luto@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>,
        Tejun Heo <tj@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [PATCH 0/2] kthread: make struct kthread kmalloc'ed
Message-ID: <20161028161106.GA8933@redhat.com>
References: <20161025110508.9052-1-roman.penyaev@profitbricks.com>
 <20161025140333.GB4326@redhat.com>
 <20161025154301.GA12015@redhat.com>
 <CALCETrVmczve=knhYGrcXkeAXRQR_oY4ac2BSc7V0TEnx6-GFA@mail.gmail.com>
 <20161026141359.GA6893@redhat.com>
 <alpine.DEB.2.20.1610261650260.4983@nanos>
 <20161026155155.GA28832@redhat.com>
 <alpine.DEB.2.20.1610262027110.5013@nanos>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.DEB.2.20.1610262027110.5013@nanos>
User-Agent: Mutt/1.5.24 (2015-08-30)
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 28 Oct 2016 16:11:09 +0000 (UTC)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Sorry for delay, I was distracted...

On 10/26, Thomas Gleixner wrote:
>
> On Wed, 26 Oct 2016, Oleg Nesterov wrote:
> > On 10/26, Thomas Gleixner wrote:
> > >
> > > On Wed, 26 Oct 2016, Oleg Nesterov wrote:
> > > > +static inline void set_kthread_struct(void *kthread)
> > > > +{
> > > > +	/*
> > > > +	 * We abuse ->set_child_tid to avoid the new member and because it
> > > > +	 * can't be wrongly copied by copy_process(). We also rely on fact
> > > > +	 * that the caller can't exec, so PF_KTHREAD can't be cleared.
> > > > +	 */
> > > > +	current->set_child_tid = (__force void __user *)kthread;
> > >
> > > Can we pretty please avoid this type casting? We only have 5 places using
> > > set_child_tid. So we can really make it a proper union
> >
> > Yes, I thought about anonymous union too, the only problem is that
> > it will need more comments ;)
>
> Be careful with anonymous unions. There are a few pitfalls with older
> compilers. That's why I said make it a proper union and fixup the 5 usage
> sites.

Ah. Then I'd prefer to do this later or in a separate change, unless you
feel strongly. I certainly do not want to update other users at least right
now.

Yes, these 2 type casts do not look nice, but they are hidden in the trivial
helpers. And, for example, if something goes wrong we can trivially change
this code to use, say, sas_ss_sp. Just we need to update the comments to
explain why it is safe too.

Finally. I still hope we will kill struct kthread (I mean, unbloat it and
embed into task_struct), and this means that the proper union should touch
more members. Say, sas_* and/or vfork_done+set/clear_child_tid. I'd like
to do this only once if possible.


I'll try to kill to_live_kthread() tomorrow, didn't have time to do this
today.

Oleg.