From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758987AbcIWXsn (ORCPT ); Fri, 23 Sep 2016 19:48:43 -0400 Received: from foss.arm.com ([217.140.101.70]:41698 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752728AbcIWXsm (ORCPT ); Fri, 23 Sep 2016 19:48:42 -0400 Date: Sat, 24 Sep 2016 00:48:34 +0100 From: Mark Rutland To: Andy Lutomirski , Ingo Molnar Cc: "linux-kernel@vger.kernel.org" , Andrew Morton , Andy Lutomirski , Kees Cook Subject: Re: [PATCH] thread_info: use unsigned long for flags Message-ID: <20160923234833.GA26666@remoulade> References: <1474651447-30447-1-git-send-email-mark.rutland@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org [Adding Ingo, so this doesn't get lost -- please see the end of the mail] On Fri, Sep 23, 2016 at 11:08:46AM -0700, Andy Lutomirski wrote: > On Fri, Sep 23, 2016 at 10:24 AM, Mark Rutland wrote: > > The generic THREAD_INFO_IN_TASK definition of thread_info::flags is a > > u32, matching x86 prior to the introduction of THREAD_INFO_IN_TASK. > > > > However, common helpers like test_ti_thread_flag() implicitly assume > > that thread_info::flags has at least the size and alignment of unsigned > > long, and relying on padding and alignment provided by other elements of > > task_struct is somewhat fragile. Additionally, some architectures use > > more that 32 bits for thread_info::flags, and others may need to in > > future. > > > > With THREAD_INFO_IN_TASK, task struct follows thread_info with a long > > field, and thus we no longer save any space as we did back in commit > > affa219b60a11b32 ("x86: change thread_info's flag field back to 32 > > bits"). > > > > Given all this, it makes more sense for the generic thread_info::flags > > to be an unsigned long. Make it so. > > I have only one problem with this, and it's a general objection that's > mostly off topic: why the [expletive] do the arch-independent bitfield > helpers think in units of variable size? It's *absurd*, especially on > big-endian architectures. > > Now that that's out of my system, I think this patch is fine. > Big-endian arches that opt in will have to deal with it somehow, but I > don't see why making it 'unsigned long' is worse than anything else. > x86 is fine with this change. FWIW, given contains/uses the helpers mentioned above, BE arches *must* use unsigned long (or something of the same size) today, or they wouldn't work. In v4.8-rc7 that is the case: $ ls -l arch | wc -l 33 # Note the above includes Kconfig, so there are 32 to consider $ git grep -W 'struct thread_info {' -- arch/*/include | \ grep flags | grep 'unsigned long\s\+flags' | wc -l 29 $ git grep -W 'struct thread_info {' -- arch/*/include | \ grep flags | grep 'int\s\+flags' arch/alpha/include/asm/thread_info.h- unsigned int flags; /* low level flags */ $ git grep -W 'struct thread_info {' -- arch/*/include | \ grep flags | grep 'u32\s\+flags' arch/ia64/include/asm/thread_info.h- __u32 flags; /* thread_info flags (see TIF_*) */ arch/x86/include/asm/thread_info.h- __u32 flags; /* low level flags */ > Acked-by: Andy Lutomirski > > Ingo, can you apply this for 4.9 so that we can make this change > before other arches might start depending on the field being u32? > > --Andy Thanks, Mark.