Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1364204 > unrolled thread

[PATCH 0/3] nohz: Convert tick dependency mask to atomic_t

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Frederic Weisbecker <fweisbec@gmail.com> - 2016-03-24 15:40 +0100
    [PATCH 1/3] atomic: Introduce atomic_fetch_or Frederic Weisbecker <fweisbec@gmail.com> - 2016-03-24 15:50 +0100
      [tip:core/urgent] locking/atomic: Introduce atomic_fetch_or() tip-bot for Frederic Weisbecker <tipbot@zytor.com> - 2016-03-29 12:40 +0200
    Re: [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Ingo Molnar <mingo@kernel.org> - 2016-03-25 09:50 +0100
      Re: [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Frederic Weisbecker <fweisbec@gmail.com> - 2016-03-25 14:20 +0100
        Re: [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Ingo Molnar <mingo@kernel.org> - 2016-03-29 11:50 +0200
          Re: [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-29 14:30 +0200
            Re: [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Frederic Weisbecker <fweisbec@gmail.com> - 2016-03-29 15:10 +0200
              Re: [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Ingo Molnar <mingo@kernel.org> - 2016-03-31 09:00 +0200
                Re: [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Peter Zijlstra <peterz@infradead.org> - 2016-03-31 11:30 +0200
                  Re: [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Ingo Molnar <mingo@kernel.org> - 2016-03-31 15:20 +0200
            Re: [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Ingo Molnar <mingo@kernel.org> - 2016-03-29 15:10 +0200
              Re: [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Ingo Molnar <mingo@kernel.org> - 2016-03-29 15:10 +0200
                Re: [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t Ingo Molnar <mingo@kernel.org> - 2016-03-29 15:20 +0200

#1364204 — [PATCH 0/3] nohz: Convert tick dependency mask to atomic_t

As per Linus suggestion, lets convert the tick dependency mask to
atomic_t. Introduce atomic_fetch_or() and confine fetch_or() back to
scheduler guts.

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	timers/nohz

HEAD: 7b7e5da5733f58668181077ec394a718e08c392c

Thanks,
	Frederic
---

Frederic Weisbecker (3):
      atomic: Introduce atomic_fetch_or
      nohz: Convert tick dependency mask to atomic_t
      Revert "atomic: Export fetch_or()"


 include/linux/atomic.h   | 34 +++++++++++++--------------
 include/linux/sched.h    |  4 ++--
 kernel/sched/core.c      | 18 ++++++++++++++
 kernel/time/tick-sched.c | 61 ++++++++++++++++++++++++------------------------
 kernel/time/tick-sched.h |  2 +-
 5 files changed, 68 insertions(+), 51 deletions(-)

[toc] | [next] | [standalone]

#1364228 — [PATCH 1/3] atomic: Introduce atomic_fetch_or

This is deemed to replace the type generic fetch_or() which brings a lot
of issues such as macro induced block variable aliasing and sloppy types.
Not to mention fetch_or() doesn't refer to any namespace, adding even
more confusion.

So lets provide an atomic_t version. Current and next users of fetch_or
are thus encouraged to use atomic_t.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/linux/atomic.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index df4f369..3d64c08 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -559,6 +559,27 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 #endif
 
 /**
+ * atomic_fetch_or - perform *p |= mask and return old value of *p
+ * @p: pointer to atomic_t
+ * @mask: mask to OR on the atomic_t
+ */
+#ifndef atomic_fetch_or
+static inline int atomic_fetch_or(atomic_t *p, int mask)
+{
+	int old, val = atomic_read(p);
+
+	for (;;) {
+		old = atomic_cmpxchg(p, val, val | mask);
+		if (old == val)
+			break;
+		val = old;
+	}
+
+	return old;
+}
+#endif
+
+/**
  * fetch_or - perform *ptr |= mask and return old value of *ptr
  * @ptr: pointer to value
  * @mask: mask to OR on the value
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1366126 — [tip:core/urgent] locking/atomic: Introduce atomic_fetch_or()

Commit-ID:  5acba71e18833b9d06686b3751598bfa263a3ac3
Gitweb:     http://git.kernel.org/tip/5acba71e18833b9d06686b3751598bfa263a3ac3
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Thu, 24 Mar 2016 15:37:59 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 29 Mar 2016 11:52:11 +0200

locking/atomic: Introduce atomic_fetch_or()

This is deemed to replace the type generic fetch_or() which brings a lot
of issues such as macro induced block variable aliasing and sloppy types.
Not to mention fetch_or() doesn't refer to any namespace, adding even
more confusion.

So lets provide an atomic_t version. Current and next users of fetch_or()
are thus encouraged to use atomic_t.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1458830281-4255-2-git-send-email-fweisbec@gmail.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/atomic.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index df4f369..3d64c08 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -559,6 +559,27 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 #endif
 
 /**
+ * atomic_fetch_or - perform *p |= mask and return old value of *p
+ * @p: pointer to atomic_t
+ * @mask: mask to OR on the atomic_t
+ */
+#ifndef atomic_fetch_or
+static inline int atomic_fetch_or(atomic_t *p, int mask)
+{
+	int old, val = atomic_read(p);
+
+	for (;;) {
+		old = atomic_cmpxchg(p, val, val | mask);
+		if (old == val)
+			break;
+		val = old;
+	}
+
+	return old;
+}
+#endif
+
+/**
  * fetch_or - perform *ptr |= mask and return old value of *ptr
  * @ptr: pointer to value
  * @mask: mask to OR on the value

[toc] | [prev] | [next] | [standalone]

#1364603

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> As per Linus suggestion, lets convert the tick dependency mask to
> atomic_t. Introduce atomic_fetch_or() and confine fetch_or() back to
> scheduler guts.
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> 	timers/nohz
> 
> HEAD: 7b7e5da5733f58668181077ec394a718e08c392c
> 
> Thanks,
> 	Frederic
> ---
> 
> Frederic Weisbecker (3):
>       atomic: Introduce atomic_fetch_or
>       nohz: Convert tick dependency mask to atomic_t
>       Revert "atomic: Export fetch_or()"
> 
> 
>  include/linux/atomic.h   | 34 +++++++++++++--------------
>  include/linux/sched.h    |  4 ++--
>  kernel/sched/core.c      | 18 ++++++++++++++
>  kernel/time/tick-sched.c | 61 ++++++++++++++++++++++++------------------------
>  kernel/time/tick-sched.h |  2 +-
>  5 files changed, 68 insertions(+), 51 deletions(-)

Could you please also convert the sched/core.c usage, so that we can get rid of 
the private fetch_or() definition? Please also double check that it does not 
result in worse code generation.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1364660

On Fri, Mar 25, 2016 at 09:48:47AM +0100, Ingo Molnar wrote:
> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > As per Linus suggestion, lets convert the tick dependency mask to
> > atomic_t. Introduce atomic_fetch_or() and confine fetch_or() back to
> > scheduler guts.
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> > 	timers/nohz
> > 
> > HEAD: 7b7e5da5733f58668181077ec394a718e08c392c
> > 
> > Thanks,
> > 	Frederic
> > ---
> > 
> > Frederic Weisbecker (3):
> >       atomic: Introduce atomic_fetch_or
> >       nohz: Convert tick dependency mask to atomic_t
> >       Revert "atomic: Export fetch_or()"
> > 
> > 
> >  include/linux/atomic.h   | 34 +++++++++++++--------------
> >  include/linux/sched.h    |  4 ++--
> >  kernel/sched/core.c      | 18 ++++++++++++++
> >  kernel/time/tick-sched.c | 61 ++++++++++++++++++++++++------------------------
> >  kernel/time/tick-sched.h |  2 +-
> >  5 files changed, 68 insertions(+), 51 deletions(-)
> 
> Could you please also convert the sched/core.c usage, so that we can get rid of 
> the private fetch_or() definition? Please also double check that it does not 
> result in worse code generation.

That involve converting thread_info::flags to atomic_t and given how much the type varies
across architectures:

       $ grep 'flags;' $(ls arch/*/include/asm/thread_info.h)
       arch/alpha/include/asm/thread_info.h:	unsigned int  flags;		/* low level flags */
       arch/arc/include/asm/thread_info.h:	unsigned long flags;			/* low level flags */
       arch/arm64/include/asm/thread_info.h:	unsigned long flags; 	     /* low level flags */
       arch/arm/include/asm/thread_info.h:	unsigned long flags; 	  /* low level flags */
       arch/avr32/include/asm/thread_info.h:	unsigned long flags;       /* low level flags */
       arch/blackfin/include/asm/thread_info.h:	 unsigned long flags;					  /* low level flags */
       arch/c6x/include/asm/thread_info.h:		 unsigned long flags;       /* low level flags */
       arch/cris/include/asm/thread_info.h:		 unsigned long flags; 	    /* low level flags */
       arch/frv/include/asm/thread_info.h:		 unsigned long flags; 	 /* low level flags */
       arch/h8300/include/asm/thread_info.h:		 unsigned long flags;	    /* low level flags */
       arch/hexagon/include/asm/thread_info.h:		 unsigned long flags;          /* low level flags */
       arch/ia64/include/asm/thread_info.h:		 __u32 flags;  		       /* thread_info flags (see TIF_*) */
       arch/m32r/include/asm/thread_info.h:		 unsigned long flags;      	    /* low level flags */
       arch/m68k/include/asm/thread_info.h:		 unsigned long flags;
       arch/metag/include/asm/thread_info.h:		 unsigned long flags;	    /* low level flags */
       arch/microblaze/include/asm/thread_info.h:	 unsigned long flags; /* low level flags */
       arch/mips/include/asm/thread_info.h:		 unsigned long flags; 	/* low level flags */
       arch/mn10300/include/asm/thread_info.h:		 unsigned long flags; 	     /* low level flags */
       arch/nios2/include/asm/thread_info.h:		 unsigned long flags; 	  /* low level flags */
       arch/openrisc/include/asm/thread_info.h:	 unsigned long flags;       /* low level flags */
       arch/parisc/include/asm/thread_info.h:		 unsigned long flags; 	       /* thread_info flags (see TIF_*) */
       arch/powerpc/include/asm/thread_info.h:		 unsigned long local_flags;    	  /* private flags for thread */
       arch/s390/include/asm/thread_info.h:		 unsigned long flags;     	   /* low level flags */
       arch/score/include/asm/thread_info.h:		 unsigned long flags; 	/* low level flags */
       arch/sh/include/asm/thread_info.h:		 unsigned long flags; 	     /* low level flags */
       arch/tile/include/asm/thread_info.h:		 unsigned long flags; 	  /* low level flags */
       arch/um/include/asm/thread_info.h:		 unsigned long flags;       /* low level flags */
       arch/unicore32/include/asm/thread_info.h:	 unsigned long flags; 	    /* low level flags */
       arch/x86/include/asm/thread_info.h:		 __u32	       flags; 	 /* low level flags */
       arch/xtensa/include/asm/thread_info.h:		 unsigned long flags;	      /* low level flags */

also given how much it is accessed (and that happens a lot in ASM as well). This conversion deserves quite a
whole project on its own.

It might be possible to do it incrementally though.

[toc] | [prev] | [next] | [standalone]

#1366080

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Fri, Mar 25, 2016 at 09:48:47AM +0100, Ingo Molnar wrote:
> > 
> > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > 
> > > As per Linus suggestion, lets convert the tick dependency mask to
> > > atomic_t. Introduce atomic_fetch_or() and confine fetch_or() back to
> > > scheduler guts.
> > > 
> > > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> > > 	timers/nohz
> > > 
> > > HEAD: 7b7e5da5733f58668181077ec394a718e08c392c
> > > 
> > > Thanks,
> > > 	Frederic
> > > ---
> > > 
> > > Frederic Weisbecker (3):
> > >       atomic: Introduce atomic_fetch_or
> > >       nohz: Convert tick dependency mask to atomic_t
> > >       Revert "atomic: Export fetch_or()"
> > > 
> > > 
> > >  include/linux/atomic.h   | 34 +++++++++++++--------------
> > >  include/linux/sched.h    |  4 ++--
> > >  kernel/sched/core.c      | 18 ++++++++++++++
> > >  kernel/time/tick-sched.c | 61 ++++++++++++++++++++++++------------------------
> > >  kernel/time/tick-sched.h |  2 +-
> > >  5 files changed, 68 insertions(+), 51 deletions(-)
> > 
> > Could you please also convert the sched/core.c usage, so that we can get rid of 
> > the private fetch_or() definition? Please also double check that it does not 
> > result in worse code generation.
> 
> That involve converting thread_info::flags to atomic_t and given how much the type varies
> across architectures:

Ah, yes - I did a similar analysis originally and then promptly forgot about it!

Harmonizing thread_info::flags does not look easy, given how much assembly code 
accesses this field.

So I suspect your original series of introducing the atomic_t interface while 
reverting back to the scheduler-specific auto-typing hack is fine after all.

> also given how much it is accessed (and that happens a lot in ASM as well). This 
> conversion deserves quite a whole project on its own.
> 
> It might be possible to do it incrementally though.

So I don't even know where to begin with that:

 - some 64-bit architectures want 32-bit flags
 - some 64-bit architectures want 64-bit flags
 - some 64-bit architectures may genuinely want more than 32 flags
 - some 64-bit architectures may want 64-bit word just because it's the fastest

... there's not a single natural data type on the C side that I can see this could 
be converted to :-/

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1366211

On Tue, Mar 29, 2016 at 4:44 AM, Ingo Molnar <mingo@kernel.org> 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;

so you could do that fairly mindlessly. You *could* even use a nasty
macro from hell to do it automatically, with the above rules in some .

Then each architecture could clean itself up and get rid of the
padding field if they want to.

              Linus

[toc] | [prev] | [next] | [standalone]

#1366252

On Tue, Mar 29, 2016 at 03:05:14PM +0200, Ingo Molnar wrote:
> 
> * Ingo Molnar <mingo@kernel.org> wrote:
> 
> > 
> > * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > 
> > > On Tue, Mar 29, 2016 at 4:44 AM, Ingo Molnar <mingo@kernel.org> 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
> 
> Argh, my reading comprehension skills suck today.
> 
> That's a totally useless analysis of task_struct::flags, while we want to convert 
> thread_info::flags...

Actually we want to convert that one too :-)
In fact I planned to start there.

[toc] | [prev] | [next] | [standalone]

#1367888

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Tue, Mar 29, 2016 at 03:05:14PM +0200, Ingo Molnar wrote:
> > 
> > * Ingo Molnar <mingo@kernel.org> wrote:
> > 
> > > 
> > > * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > > 
> > > > On Tue, Mar 29, 2016 at 4:44 AM, Ingo Molnar <mingo@kernel.org> 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
> > 
> > Argh, my reading comprehension skills suck today.
> > 
> > That's a totally useless analysis of task_struct::flags, while we want to convert 
> > thread_info::flags...
> 
> Actually we want to convert that one too :-)
> In fact I planned to start there.

Sounds good to me! I also volunteer the x86 architecture to be the guinea pig to 
convert thread_info::flags to atomic_t ;-) [*]

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1368071

On Thu, Mar 31, 2016 at 08:54:19AM +0200, Ingo Molnar wrote:
> Sounds good to me! I also volunteer the x86 architecture to be the guinea pig to 
> convert thread_info::flags to atomic_t ;-) [*]

So I'm not sure we can do this one arch at a time; all the TIF
manipulators live in include/linux/thread_info.h and are shared across
all archs.

Another thing to look out for is that set_bit() uses LOCK BTS when the
bit is not a compile time constant, we do not have an atomic_*() version
of that. Now I'm not sure if this matters, we might never end up
triggering that code path for TIF flags.

[toc] | [prev] | [next] | [standalone]

#1368292

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, Mar 31, 2016 at 08:54:19AM +0200, Ingo Molnar wrote:
> > Sounds good to me! I also volunteer the x86 architecture to be the guinea pig to 
> > convert thread_info::flags to atomic_t ;-) [*]
> 
> So I'm not sure we can do this one arch at a time; all the TIF
> manipulators live in include/linux/thread_info.h and are shared across
> all archs.

So my thinking was that we'd #ifdef the variants.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1366254

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, Mar 29, 2016 at 4:44 AM, Ingo Molnar <mingo@kernel.org> 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(&current->__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

[toc] | [prev] | [next] | [standalone]

#1366260

* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > On Tue, Mar 29, 2016 at 4:44 AM, Ingo Molnar <mingo@kernel.org> 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

Argh, my reading comprehension skills suck today.

That's a totally useless analysis of task_struct::flags, while we want to convert 
thread_info::flags...

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1366265

* Ingo Molnar <mingo@kernel.org> wrote:

> That's a totally useless analysis of task_struct::flags, while we want to 
> convert thread_info::flags...

So going over to arguing about thread_info::flags:

1)

We already have ti::flags accessors for most of the generic code. There's a few 
outliers (in the scheduler code...) which can be fixed.

2)

We could introduce a ARCH_HAS_ATOMIC_TIF flag to do per arch conversion: this 
would trigger #ifdefs in the accessors. When an architecture switches ti::flags to 
atomic_t, it also sets ARCH_HAS_ATOMIC_TIF.

I'd still do a flags => __flags rename of the field, to make the conversion easy 
and safe. So any arch that has ARCH_HAS_ATOMIC_TIF set provides an atomic_t 
thread_info::__flags field.

3)

The new accessors under ARCH_HAS_ATOMIC_TIF would use atomic.h functions to 
shuffle the thread-info flags.

4)

After one kernel release we could add:

	WARN_ONCE("please convert thread_info::flags to atomic_t!", 1);

to the old accessors to accelerate conversion.

5)

Eventually, once every architecture is converted, we could eliminate the old 
sched.c fetch_or() macro.

Thanks,

	Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web