public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] x86: Cleanup TIF value gaps in shift range
@ 2010-10-18 20:35 Frederic Weisbecker
  2010-10-18 20:35 ` [PATCH 2/2] x86: Remove stale TIF_DEBUG Frederic Weisbecker
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Frederic Weisbecker @ 2010-10-18 20:35 UTC (permalink / raw)
  To: LKML
  Cc: LKML, Frederic Weisbecker, Ingo Molnar, H. Peter Anvin,
	Thomas Gleixner

9, 19 and 26 values are missing from the TIF shift range, probably
due to flags that were removed by the past. Now repack the range
so that we can quickly retrieve the remaining free shift slots.

But take care of keeping the seperation between high and low bits
as some masks are created on top of this boundary.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/thread_info.h |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index f0b6e5d..0bbf6f9 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -82,19 +82,19 @@ struct thread_info {
 #define TIF_SYSCALL_EMU		6	/* syscall emulation active */
 #define TIF_SYSCALL_AUDIT	7	/* syscall auditing active */
 #define TIF_SECCOMP		8	/* secure computing */
-#define TIF_MCE_NOTIFY		10	/* notify userspace of an MCE */
-#define TIF_USER_RETURN_NOTIFY	11	/* notify kernel of userspace return */
+#define TIF_MCE_NOTIFY		9	/* notify userspace of an MCE */
+#define TIF_USER_RETURN_NOTIFY	10	/* notify kernel of userspace return */
 #define TIF_NOTSC		16	/* TSC is not accessible in userland */
 #define TIF_IA32		17	/* 32bit process */
 #define TIF_FORK		18	/* ret_from_fork */
-#define TIF_MEMDIE		20	/* is terminating due to OOM killer */
-#define TIF_DEBUG		21	/* uses debug registers */
-#define TIF_IO_BITMAP		22	/* uses I/O bitmap */
-#define TIF_FREEZE		23	/* is freezing for suspend */
-#define TIF_FORCED_TF		24	/* true if TF in eflags artificially */
-#define TIF_BLOCKSTEP		25	/* set when we want DEBUGCTLMSR_BTF */
-#define TIF_LAZY_MMU_UPDATES	27	/* task is updating the mmu lazily */
-#define TIF_SYSCALL_TRACEPOINT	28	/* syscall tracepoint instrumentation */
+#define TIF_MEMDIE		19	/* is terminating due to OOM killer */
+#define TIF_DEBUG		20	/* uses debug registers */
+#define TIF_IO_BITMAP		21	/* uses I/O bitmap */
+#define TIF_FREEZE		22	/* is freezing for suspend */
+#define TIF_FORCED_TF		23	/* true if TF in eflags artificially */
+#define TIF_BLOCKSTEP		24	/* set when we want DEBUGCTLMSR_BTF */
+#define TIF_LAZY_MMU_UPDATES	25	/* task is updating the mmu lazily */
+#define TIF_SYSCALL_TRACEPOINT	26	/* syscall tracepoint instrumentation */
 
 #define _TIF_SYSCALL_TRACE	(1 << TIF_SYSCALL_TRACE)
 #define _TIF_NOTIFY_RESUME	(1 << TIF_NOTIFY_RESUME)
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] x86: Remove stale TIF_DEBUG
  2010-10-18 20:35 [PATCH 1/2] x86: Cleanup TIF value gaps in shift range Frederic Weisbecker
@ 2010-10-18 20:35 ` Frederic Weisbecker
  2010-10-18 22:12   ` [PATCH v2] " Frederic Weisbecker
  2010-10-18 20:47 ` [PATCH 1/2] x86: Cleanup TIF value gaps in shift range David Rientjes
  2010-10-18 20:48 ` H. Peter Anvin
  2 siblings, 1 reply; 13+ messages in thread
From: Frederic Weisbecker @ 2010-10-18 20:35 UTC (permalink / raw)
  To: LKML
  Cc: LKML, Frederic Weisbecker, Ingo Molnar, H. Peter Anvin,
	Thomas Gleixner

The last user was ptrace breakpoints, which doesn't use that
since 2.6.33.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/thread_info.h |   15 ++++++---------
 arch/x86/kernel/process_32.c       |    2 +-
 arch/x86/kernel/process_64.c       |    2 +-
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 0bbf6f9..445cf3c 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -88,13 +88,12 @@ struct thread_info {
 #define TIF_IA32		17	/* 32bit process */
 #define TIF_FORK		18	/* ret_from_fork */
 #define TIF_MEMDIE		19	/* is terminating due to OOM killer */
-#define TIF_DEBUG		20	/* uses debug registers */
-#define TIF_IO_BITMAP		21	/* uses I/O bitmap */
-#define TIF_FREEZE		22	/* is freezing for suspend */
-#define TIF_FORCED_TF		23	/* true if TF in eflags artificially */
-#define TIF_BLOCKSTEP		24	/* set when we want DEBUGCTLMSR_BTF */
-#define TIF_LAZY_MMU_UPDATES	25	/* task is updating the mmu lazily */
-#define TIF_SYSCALL_TRACEPOINT	26	/* syscall tracepoint instrumentation */
+#define TIF_IO_BITMAP		20	/* uses I/O bitmap */
+#define TIF_FREEZE		21	/* is freezing for suspend */
+#define TIF_FORCED_TF		22	/* true if TF in eflags artificially */
+#define TIF_BLOCKSTEP		23	/* set when we want DEBUGCTLMSR_BTF */
+#define TIF_LAZY_MMU_UPDATES	24	/* task is updating the mmu lazily */
+#define TIF_SYSCALL_TRACEPOINT	25	/* syscall tracepoint instrumentation */
 
 #define _TIF_SYSCALL_TRACE	(1 << TIF_SYSCALL_TRACE)
 #define _TIF_NOTIFY_RESUME	(1 << TIF_NOTIFY_RESUME)
@@ -110,7 +109,6 @@ struct thread_info {
 #define _TIF_NOTSC		(1 << TIF_NOTSC)
 #define _TIF_IA32		(1 << TIF_IA32)
 #define _TIF_FORK		(1 << TIF_FORK)
-#define _TIF_DEBUG		(1 << TIF_DEBUG)
 #define _TIF_IO_BITMAP		(1 << TIF_IO_BITMAP)
 #define _TIF_FREEZE		(1 << TIF_FREEZE)
 #define _TIF_FORCED_TF		(1 << TIF_FORCED_TF)
@@ -148,7 +146,6 @@ struct thread_info {
 	(_TIF_IO_BITMAP|_TIF_NOTSC|_TIF_BLOCKSTEP)
 
 #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)
-#define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW|_TIF_DEBUG)
 
 #define PREEMPT_ACTIVE		0x10000000
 
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index 96586c3..44726fb 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -350,7 +350,7 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 	 * Now maybe handle debug registers and/or IO bitmaps
 	 */
 	if (unlikely(task_thread_info(prev_p)->flags & _TIF_WORK_CTXSW_PREV ||
-		     task_thread_info(next_p)->flags & _TIF_WORK_CTXSW_NEXT))
+		     task_thread_info(next_p)->flags & _TIF_WORK_CTXSW))
 		__switch_to_xtra(prev_p, next_p, tss);
 
 	/* If we're going to preload the fpu context, make sure clts
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index b3d7a3a..399760d 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -484,7 +484,7 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 	/*
 	 * Now maybe reload the debug registers and handle I/O bitmaps
 	 */
-	if (unlikely(task_thread_info(next_p)->flags & _TIF_WORK_CTXSW_NEXT ||
+	if (unlikely(task_thread_info(next_p)->flags & _TIF_WORK_CTXSW ||
 		     task_thread_info(prev_p)->flags & _TIF_WORK_CTXSW_PREV))
 		__switch_to_xtra(prev_p, next_p, tss);
 
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] x86: Cleanup TIF value gaps in shift range
  2010-10-18 20:35 [PATCH 1/2] x86: Cleanup TIF value gaps in shift range Frederic Weisbecker
  2010-10-18 20:35 ` [PATCH 2/2] x86: Remove stale TIF_DEBUG Frederic Weisbecker
@ 2010-10-18 20:47 ` David Rientjes
  2010-10-18 21:23   ` Thomas Gleixner
  2010-10-18 20:48 ` H. Peter Anvin
  2 siblings, 1 reply; 13+ messages in thread
From: David Rientjes @ 2010-10-18 20:47 UTC (permalink / raw)
  To: Frederic Weisbecker; +Cc: LKML, Ingo Molnar, H. Peter Anvin, Thomas Gleixner

On Mon, 18 Oct 2010, Frederic Weisbecker wrote:

> 9, 19 and 26 values are missing from the TIF shift range, probably
> due to flags that were removed by the past. Now repack the range
> so that we can quickly retrieve the remaining free shift slots.
> 
> But take care of keeping the seperation between high and low bits
> as some masks are created on top of this boundary.
> 

What's the benefit of doing this?

These flags are exported to userspace through SysRq-T, SysRq+W, the hung 
task detector, and the rcu stall detector, so there may be external 
dependencies testing for these bits.

We use this to look for TIF_MEMDIE to determine whether an oom killed task 
has failed to exit and becomes hung after having access to memory 
reserves, and that's one of the bits you've changed here.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] x86: Cleanup TIF value gaps in shift range
  2010-10-18 20:35 [PATCH 1/2] x86: Cleanup TIF value gaps in shift range Frederic Weisbecker
  2010-10-18 20:35 ` [PATCH 2/2] x86: Remove stale TIF_DEBUG Frederic Weisbecker
  2010-10-18 20:47 ` [PATCH 1/2] x86: Cleanup TIF value gaps in shift range David Rientjes
@ 2010-10-18 20:48 ` H. Peter Anvin
  2010-10-18 21:10   ` Frederic Weisbecker
  2 siblings, 1 reply; 13+ messages in thread
From: H. Peter Anvin @ 2010-10-18 20:48 UTC (permalink / raw)
  To: Frederic Weisbecker; +Cc: LKML, Ingo Molnar, Thomas Gleixner

On 10/18/2010 01:35 PM, Frederic Weisbecker wrote:
> 9, 19 and 26 values are missing from the TIF shift range, probably
> due to flags that were removed by the past. Now repack the range
> so that we can quickly retrieve the remaining free shift slots.
> 
> But take care of keeping the seperation between high and low bits
> as some masks are created on top of this boundary.
> 

What is the point of this?  All it seems is that it would make stupid
debugging mistakes possible because someone looks at the values from the
wrong kernel.

Feel free to put comments in:

/* 9 - unused - was TIF_xxx */

... but I think gratuitous compaction is a really bad idea.

Nacked-by: H. Peter Anvin <hpa@zytor.com>

	-hpa


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] x86: Cleanup TIF value gaps in shift range
  2010-10-18 20:48 ` H. Peter Anvin
@ 2010-10-18 21:10   ` Frederic Weisbecker
  0 siblings, 0 replies; 13+ messages in thread
From: Frederic Weisbecker @ 2010-10-18 21:10 UTC (permalink / raw)
  To: H. Peter Anvin, David Rientjes; +Cc: LKML, Ingo Molnar, Thomas Gleixner

On Mon, Oct 18, 2010 at 01:48:10PM -0700, H. Peter Anvin wrote:
> On 10/18/2010 01:35 PM, Frederic Weisbecker wrote:
> > 9, 19 and 26 values are missing from the TIF shift range, probably
> > due to flags that were removed by the past. Now repack the range
> > so that we can quickly retrieve the remaining free shift slots.
> > 
> > But take care of keeping the seperation between high and low bits
> > as some masks are created on top of this boundary.
> > 
> 
> What is the point of this?  All it seems is that it would make stupid
> debugging mistakes possible because someone looks at the values from the
> wrong kernel.
> 
> Feel free to put comments in:
> 
> /* 9 - unused - was TIF_xxx */
> 
> ... but I think gratuitous compaction is a really bad idea.
> 
> Nacked-by: H. Peter Anvin <hpa@zytor.com>


I just felt the current state was messy. But I understand your reasons,
just forget those patches.

Thanks.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] x86: Cleanup TIF value gaps in shift range
  2010-10-18 20:47 ` [PATCH 1/2] x86: Cleanup TIF value gaps in shift range David Rientjes
@ 2010-10-18 21:23   ` Thomas Gleixner
  2010-10-18 21:28     ` H. Peter Anvin
  2010-10-18 21:30     ` David Rientjes
  0 siblings, 2 replies; 13+ messages in thread
From: Thomas Gleixner @ 2010-10-18 21:23 UTC (permalink / raw)
  To: David Rientjes; +Cc: Frederic Weisbecker, LKML, Ingo Molnar, H. Peter Anvin

On Mon, 18 Oct 2010, David Rientjes wrote:

> On Mon, 18 Oct 2010, Frederic Weisbecker wrote:
> 
> > 9, 19 and 26 values are missing from the TIF shift range, probably
> > due to flags that were removed by the past. Now repack the range
> > so that we can quickly retrieve the remaining free shift slots.
> > 
> > But take care of keeping the seperation between high and low bits
> > as some masks are created on top of this boundary.
> > 
> 
> What's the benefit of doing this?
> 
> These flags are exported to userspace through SysRq-T, SysRq+W, the hung 
> task detector, and the rcu stall detector, so there may be external 
> dependencies testing for these bits.
> 
> We use this to look for TIF_MEMDIE to determine whether an oom killed task 
> has failed to exit and becomes hung after having access to memory 
> reserves, and that's one of the bits you've changed here.

I agree in general, but this is stupid as hell. No fcking interface
should exposed kernel internal flag bits just as hex values and no
fcking luser space should rely on it to be a subject of no change.

Seriously, if we can't even change TIF_* bits anymore then we are
doing something wrong. That's a pure kernel internal affair and
subject to change.

Thanks,

	tglx


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] x86: Cleanup TIF value gaps in shift range
  2010-10-18 21:23   ` Thomas Gleixner
@ 2010-10-18 21:28     ` H. Peter Anvin
  2010-10-18 21:36       ` David Rientjes
  2010-10-18 21:30     ` David Rientjes
  1 sibling, 1 reply; 13+ messages in thread
From: H. Peter Anvin @ 2010-10-18 21:28 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: David Rientjes, Frederic Weisbecker, LKML, Ingo Molnar

On 10/18/2010 02:23 PM, Thomas Gleixner wrote:
> On Mon, 18 Oct 2010, David Rientjes wrote:
> 
>> On Mon, 18 Oct 2010, Frederic Weisbecker wrote:
>>
>>> 9, 19 and 26 values are missing from the TIF shift range, probably
>>> due to flags that were removed by the past. Now repack the range
>>> so that we can quickly retrieve the remaining free shift slots.
>>>
>>> But take care of keeping the seperation between high and low bits
>>> as some masks are created on top of this boundary.
>>>
>>
>> What's the benefit of doing this?
>>
>> These flags are exported to userspace through SysRq-T, SysRq+W, the hung 
>> task detector, and the rcu stall detector, so there may be external 
>> dependencies testing for these bits.
>>
>> We use this to look for TIF_MEMDIE to determine whether an oom killed task 
>> has failed to exit and becomes hung after having access to memory 
>> reserves, and that's one of the bits you've changed here.
> 
> I agree in general, but this is stupid as hell. No fcking interface
> should exposed kernel internal flag bits just as hex values and no
> fcking luser space should rely on it to be a subject of no change.
> 
> Seriously, if we can't even change TIF_* bits anymore then we are
> doing something wrong. That's a pure kernel internal affair and
> subject to change.
> 

The problem is that someone exports something as debugging information,
then someone else suddenly thinks it's an ABI.  I believe the same
complainant in the past has objected to changing of formatting in dmesg,
which is equally insane.

	-hpa

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] x86: Cleanup TIF value gaps in shift range
  2010-10-18 21:23   ` Thomas Gleixner
  2010-10-18 21:28     ` H. Peter Anvin
@ 2010-10-18 21:30     ` David Rientjes
  1 sibling, 0 replies; 13+ messages in thread
From: David Rientjes @ 2010-10-18 21:30 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Frederic Weisbecker, LKML, Ingo Molnar, H. Peter Anvin

On Mon, 18 Oct 2010, Thomas Gleixner wrote:

> I agree in general, but this is stupid as hell. No fcking interface
> should exposed kernel internal flag bits just as hex values and no
> fcking luser space should rely on it to be a subject of no change.
> 
> Seriously, if we can't even change TIF_* bits anymore then we are
> doing something wrong. That's a pure kernel internal affair and
> subject to change.
> 

I agree, and when I added them to the show_state() dumps, I should have 
probably enumerated the bits of interest to userspace and printed them 
much like taint flags: use 'M' for my bit of interest, TIF_MEMDIE, which 
gives access to memory reserves.  We care a _lot_ about detecting hung 
tasks that have that bit set because it usually indicates a VM problem 
where we've allowed complete depletion of memory reserves.  There's no 
other way to detect that, so we need to export TIF_MEMDIE somehow and if 
that's by doing (ti->flags & TIF_MEMDIE) ? "M" : "" then that's fine by 
me.  I hope we can do that before changing its value, though.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] x86: Cleanup TIF value gaps in shift range
  2010-10-18 21:28     ` H. Peter Anvin
@ 2010-10-18 21:36       ` David Rientjes
  2010-10-18 21:43         ` H. Peter Anvin
  0 siblings, 1 reply; 13+ messages in thread
From: David Rientjes @ 2010-10-18 21:36 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Thomas Gleixner, Frederic Weisbecker, LKML, Ingo Molnar

On Mon, 18 Oct 2010, H. Peter Anvin wrote:

> The problem is that someone exports something as debugging information,
> then someone else suddenly thinks it's an ABI.  I believe the same
> complainant in the past has objected to changing of formatting in dmesg,
> which is equally insane.
> 

It's not insane if there is no other way to ascertain that information.  
If it's available through sysfs or debugfs (and, even better, documented 
as part of the API in Documentation/ABI), then I don't think anyone would 
object to changing a log message.  But I don't think all log messages 
should be fair game under some general principle if they are being changed 
(instead of just extending it) without a compelling reason, such as 
technically being incorrect in its present form.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] x86: Cleanup TIF value gaps in shift range
  2010-10-18 21:36       ` David Rientjes
@ 2010-10-18 21:43         ` H. Peter Anvin
  2010-10-18 22:00           ` David Rientjes
  0 siblings, 1 reply; 13+ messages in thread
From: H. Peter Anvin @ 2010-10-18 21:43 UTC (permalink / raw)
  To: David Rientjes; +Cc: Thomas Gleixner, Frederic Weisbecker, LKML, Ingo Molnar

On 10/18/2010 02:36 PM, David Rientjes wrote:
> On Mon, 18 Oct 2010, H. Peter Anvin wrote:
> 
>> The problem is that someone exports something as debugging information,
>> then someone else suddenly thinks it's an ABI.  I believe the same
>> complainant in the past has objected to changing of formatting in dmesg,
>> which is equally insane.
>>
> 
> It's not insane if there is no other way to ascertain that information.  
> If it's available through sysfs or debugfs (and, even better, documented 
> as part of the API in Documentation/ABI), then I don't think anyone would 
> object to changing a log message.  But I don't think all log messages 
> should be fair game under some general principle if they are being changed 
> (instead of just extending it) without a compelling reason, such as 
> technically being incorrect in its present form.

YES IT IS.  In fact, it is completely and totally bananas bonkers.

By not pushing for a proper maintainable ABI, you will have an
indefinite forward compatibility problem, and when predictably it
breaks, you'll complain.  This is, however, backwards -- the right thing
would have been to say "I need this, this isn't available, I should add
a maintainable API and push it upstream", and perhaps add log parsing as
a backwards-compatibility solution.

	-hpa


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] x86: Cleanup TIF value gaps in shift range
  2010-10-18 21:43         ` H. Peter Anvin
@ 2010-10-18 22:00           ` David Rientjes
  2010-10-18 23:23             ` H. Peter Anvin
  0 siblings, 1 reply; 13+ messages in thread
From: David Rientjes @ 2010-10-18 22:00 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Thomas Gleixner, Frederic Weisbecker, LKML, Ingo Molnar

On Mon, 18 Oct 2010, H. Peter Anvin wrote:

> > It's not insane if there is no other way to ascertain that information.  
> > If it's available through sysfs or debugfs (and, even better, documented 
> > as part of the API in Documentation/ABI), then I don't think anyone would 
> > object to changing a log message.  But I don't think all log messages 
> > should be fair game under some general principle if they are being changed 
> > (instead of just extending it) without a compelling reason, such as 
> > technically being incorrect in its present form.
> 
> YES IT IS.  In fact, it is completely and totally bananas bonkers.
> 
> By not pushing for a proper maintainable ABI, you will have an
> indefinite forward compatibility problem, and when predictably it
> breaks, you'll complain.  This is, however, backwards -- the right thing
> would have been to say "I need this, this isn't available, I should add
> a maintainable API and push it upstream", and perhaps add log parsing as
> a backwards-compatibility solution.
> 

The problem is deciding what should be an ABI and what shouldn't an ABI 
when it isn't clear.  It would be great if everything that is logged or 
shown to userspace would be part of an ABI and would be available no 
matter how much information is emitted to the log.  That's not scalable, 
so we have to decide what userspace may depend on and design ABIs that 
provide that information in an extendable way that won't break or become 
obsoleted in the foreseeable future.

Since we can't do that for everything and we have no idea what users will 
find to parse from the dmesg, I'm advocating that if a change is proposed, 
like was in this case with ti->flags, and someone has a usecase where the 
information isn't available in any other way, that they speak up and come 
up with a maintainable solution so that we've identified the parties 
involved and can change that log message if necessary.  I only think that 
should be done, though, when there is a compelling reason for the change.

I think that was done in this case by suggesting an alternative (printing, 
at minimum, "M" when a thread has TIF_MEMDIE set instead of the raw flag 
bits), but I don't think the change itself was compelling enough that it 
has to be done.  That doesn't mean doing the change I suggested wasn't 
still appropriate, but at least it was known as a prerequisite before 
something like this should be merged.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2] x86: Remove stale TIF_DEBUG
  2010-10-18 20:35 ` [PATCH 2/2] x86: Remove stale TIF_DEBUG Frederic Weisbecker
@ 2010-10-18 22:12   ` Frederic Weisbecker
  0 siblings, 0 replies; 13+ messages in thread
From: Frederic Weisbecker @ 2010-10-18 22:12 UTC (permalink / raw)
  To: LKML
  Cc: LKML, Frederic Weisbecker, Ingo Molnar, H. Peter Anvin,
	Thomas Gleixner

The last user was ptrace breakpoints, which doesn't use that
since 2.6.33.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/thread_info.h |    3 ---
 arch/x86/kernel/process_32.c       |    2 +-
 arch/x86/kernel/process_64.c       |    2 +-
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index f0b6e5d..7d0fed5 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -88,7 +88,6 @@ struct thread_info {
 #define TIF_IA32		17	/* 32bit process */
 #define TIF_FORK		18	/* ret_from_fork */
 #define TIF_MEMDIE		20	/* is terminating due to OOM killer */
-#define TIF_DEBUG		21	/* uses debug registers */
 #define TIF_IO_BITMAP		22	/* uses I/O bitmap */
 #define TIF_FREEZE		23	/* is freezing for suspend */
 #define TIF_FORCED_TF		24	/* true if TF in eflags artificially */
@@ -110,7 +109,6 @@ struct thread_info {
 #define _TIF_NOTSC		(1 << TIF_NOTSC)
 #define _TIF_IA32		(1 << TIF_IA32)
 #define _TIF_FORK		(1 << TIF_FORK)
-#define _TIF_DEBUG		(1 << TIF_DEBUG)
 #define _TIF_IO_BITMAP		(1 << TIF_IO_BITMAP)
 #define _TIF_FREEZE		(1 << TIF_FREEZE)
 #define _TIF_FORCED_TF		(1 << TIF_FORCED_TF)
@@ -148,7 +146,6 @@ struct thread_info {
 	(_TIF_IO_BITMAP|_TIF_NOTSC|_TIF_BLOCKSTEP)
 
 #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)
-#define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW|_TIF_DEBUG)
 
 #define PREEMPT_ACTIVE		0x10000000
 
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index 96586c3..44726fb 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -350,7 +350,7 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 	 * Now maybe handle debug registers and/or IO bitmaps
 	 */
 	if (unlikely(task_thread_info(prev_p)->flags & _TIF_WORK_CTXSW_PREV ||
-		     task_thread_info(next_p)->flags & _TIF_WORK_CTXSW_NEXT))
+		     task_thread_info(next_p)->flags & _TIF_WORK_CTXSW))
 		__switch_to_xtra(prev_p, next_p, tss);
 
 	/* If we're going to preload the fpu context, make sure clts
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index b3d7a3a..399760d 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -484,7 +484,7 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 	/*
 	 * Now maybe reload the debug registers and handle I/O bitmaps
 	 */
-	if (unlikely(task_thread_info(next_p)->flags & _TIF_WORK_CTXSW_NEXT ||
+	if (unlikely(task_thread_info(next_p)->flags & _TIF_WORK_CTXSW ||
 		     task_thread_info(prev_p)->flags & _TIF_WORK_CTXSW_PREV))
 		__switch_to_xtra(prev_p, next_p, tss);
 
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] x86: Cleanup TIF value gaps in shift range
  2010-10-18 22:00           ` David Rientjes
@ 2010-10-18 23:23             ` H. Peter Anvin
  0 siblings, 0 replies; 13+ messages in thread
From: H. Peter Anvin @ 2010-10-18 23:23 UTC (permalink / raw)
  To: David Rientjes; +Cc: Thomas Gleixner, Frederic Weisbecker, LKML, Ingo Molnar

On 10/18/2010 03:00 PM, David Rientjes wrote:
>>
>> YES IT IS.  In fact, it is completely and totally bananas bonkers.
>>
>> By not pushing for a proper maintainable ABI, you will have an
>> indefinite forward compatibility problem, and when predictably it
>> breaks, you'll complain.  This is, however, backwards -- the right thing
>> would have been to say "I need this, this isn't available, I should add
>> a maintainable API and push it upstream", and perhaps add log parsing as
>> a backwards-compatibility solution.
> 
> The problem is deciding what should be an ABI and what shouldn't an ABI 
> when it isn't clear.  It would be great if everything that is logged or 
> shown to userspace would be part of an ABI and would be available no 
> matter how much information is emitted to the log.  That's not scalable, 
> so we have to decide what userspace may depend on and design ABIs that 
> provide that information in an extendable way that won't break or become 
> obsoleted in the foreseeable future.
> 
> Since we can't do that for everything and we have no idea what users will 
> find to parse from the dmesg, I'm advocating that if a change is proposed, 
> like was in this case with ti->flags, and someone has a usecase where the 
> information isn't available in any other way, that they speak up and come 
> up with a maintainable solution so that we've identified the parties 
> involved and can change that log message if necessary.  I only think that 
> should be done, though, when there is a compelling reason for the change.
> 
> I think that was done in this case by suggesting an alternative (printing, 
> at minimum, "M" when a thread has TIF_MEMDIE set instead of the raw flag 
> bits), but I don't think the change itself was compelling enough that it 
> has to be done.  That doesn't mean doing the change I suggested wasn't 
> still appropriate, but at least it was known as a prerequisite before 
> something like this should be merged.

Note: I have already said we shouldn't change TIF_ flags.  The thing I'm
objecting to is that in very short order you have made multiple requests
for API-type stability for things that are explicitly for human
consumption, like dmesg and Sysrq information.

Expecting *anything* in dmesg to remain stable in any way is aggravated
insanity and completely unreasonable.

	-hpa

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2010-10-18 23:23 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-18 20:35 [PATCH 1/2] x86: Cleanup TIF value gaps in shift range Frederic Weisbecker
2010-10-18 20:35 ` [PATCH 2/2] x86: Remove stale TIF_DEBUG Frederic Weisbecker
2010-10-18 22:12   ` [PATCH v2] " Frederic Weisbecker
2010-10-18 20:47 ` [PATCH 1/2] x86: Cleanup TIF value gaps in shift range David Rientjes
2010-10-18 21:23   ` Thomas Gleixner
2010-10-18 21:28     ` H. Peter Anvin
2010-10-18 21:36       ` David Rientjes
2010-10-18 21:43         ` H. Peter Anvin
2010-10-18 22:00           ` David Rientjes
2010-10-18 23:23             ` H. Peter Anvin
2010-10-18 21:30     ` David Rientjes
2010-10-18 20:48 ` H. Peter Anvin
2010-10-18 21:10   ` Frederic Weisbecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox