* [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
* [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 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: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: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
* 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
* 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 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
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