Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756116AbcC2M76 (ORCPT ); Tue, 29 Mar 2016 08:59:58 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:35845 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751986AbcC2M7z (ORCPT ); Tue, 29 Mar 2016 08:59:55 -0400 Date: Tue, 29 Mar 2016 14:59:51 +0200 From: Ingo Molnar To: Linus Torvalds Cc: Frederic Weisbecker , LKML , Peter Zijlstra , Thomas Gleixner , Andrew Morton Subject: Re: [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Message-ID: <20160329125950.GA2768@gmail.com> References: <1458830281-4255-1-git-send-email-fweisbec@gmail.com> <20160325084847.GA15235@gmail.com> <20160325131732.GA16488@lerouge> <20160329094454.GA4715@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3900 Lines: 133 * Linus Torvalds wrote: > On Tue, Mar 29, 2016 at 4:44 AM, Ingo Molnar wrote: > > > > Harmonizing thread_info::flags does not look easy, given how much assembly code > > accesses this field. > > It might not be too bad. > > For 32-bit architectures (which is still most of them), it's just a > > unsigned int/long -> atomic_t > > and for 64-bit architectures you end up with three choices: > > - it's already 32-bit (alpha, ia64, x86): > > unsigned int -> atomic_t > > - little-endian long: > > atomic_t flags > unsigned int padding; > > - big-endian long (only powerpc? Maybe there's a big-endian MIPS still?) > > unsigned int padding; > atomic_t flags; Hm, that indeed sounds fairly nice and doable - I thought some architectures do have a task flag above bit 31, but that does not appear to be so ... Right now we seem to have 27 bits defined in include/linux/sched.h, with 5 more bits left for the future. Here's their current usage histogram in the kernel source: PF_KTHREAD : 68 PF_MEMALLOC : 65 PF_EXITING : 49 PF_RANDOMIZE : 20 PF_VCPU : 18 PF_FREEZER_SKIP : 15 PF_SUPERPRIV : 14 PF_FSTRANS : 14 PF_NOFREEZE : 13 PF_WQ_WORKER : 12 PF_SWAPWRITE : 12 PF_MEMALLOC_NOIO : 11 PF_FROZEN : 11 PF_NO_SETAFFINITY : 9 PF_LESS_THROTTLE : 8 PF_USED_MATH : 7 PF_SUSPEND_TASK : 7 PF_KSWAPD : 7 PF_FORKNOEXEC : 7 PF_NPROC_EXCEEDED : 6 PF_MCE_PROCESS : 6 PF_MCE_EARLY : 6 PF_USED_ASYNC : 5 PF_SIGNALED : 5 PF_EXITPIDONE : 5 PF_DUMPCORE : 5 PF_MUTEX_TESTER : 1 1) PF_MUTEX_TESTER could be gotten rid of straight away as it appears to be unused. 2) I'd also rename the lot while touching every usage site: the PF_ 'process flag' namespace currently collides with: - the PF_ 'page flag' namespace - the PF_ 'protocol family' constants in the networking code ... all of which makes grepping and code reading a bit harder than it should be, IMHO. Calling them 'process' flags is a misnomer anyway, these are fundamentally per task flags. All in one, having them named TF_ would work for me. TF_ is a mostly unused namespace in generic code right now, and it would rhyme well with the existing TIF_ (thread_info flag) namespace. ( I guess ATF_ for 'atomic task flag' would work as well, except that the acronym sounds too much like a well-known government agency. Plus I guess the ASS acronym principle applies as well. ) 3) We could also rename the flag itself to __flags, for the following five purposes: - to make sure there's no lingering unconverted usage, especially in assembly code that tends to drop types and go by names only. - and to push people towards using accessors (task_flag(), set_task_flag(), etc.), not the raw field. - accessor conversion could precede the type conversion. I.e. the new accessors could work on the old type as well. - accessors would also make it easier to extend the type to atomic64_t in the future, should we ever run out of 32 task flags. - accessors would make it easier to do per arch conversion as well. So this: if (current->flags & PF_KTHREAD) would look like this: if (atomic_read(¤t->__task_flags) & TF_KTHREAD) Or rather, we'd use obviously named accessors: if (task_flag(current, TF_KTHREAD)) plus: set_task_flag(current, TF_KTHREAD); et al. How does this sound? Thanks, Ingo