* [patch] mm, mempolicy: dummy slab_node return value for bugless kernels @ 2012-03-04 21:43 David Rientjes 2012-03-06 20:15 ` Rafael Aquini 2012-03-07 0:08 ` Andrew Morton 0 siblings, 2 replies; 16+ messages in thread From: David Rientjes @ 2012-03-04 21:43 UTC (permalink / raw) To: Andrew Morton; +Cc: KAMEZAWA Hiroyuki, KOSAKI Motohiro, linux-mm BUG() is a no-op when CONFIG_BUG is disabled, so slab_node() needs a dummy return value to avoid reaching the end of a non-void function. Signed-off-by: David Rientjes <rientjes@google.com> --- mm/mempolicy.c | 1 + 1 file changed, 1 insertion(+) diff --git a/mm/mempolicy.c b/mm/mempolicy.c --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -1611,6 +1611,7 @@ unsigned slab_node(struct mempolicy *policy) default: BUG(); + return numa_node_id(); } } -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: dummy slab_node return value for bugless kernels 2012-03-04 21:43 [patch] mm, mempolicy: dummy slab_node return value for bugless kernels David Rientjes @ 2012-03-06 20:15 ` Rafael Aquini 2012-03-07 0:08 ` Andrew Morton 1 sibling, 0 replies; 16+ messages in thread From: Rafael Aquini @ 2012-03-06 20:15 UTC (permalink / raw) To: David Rientjes Cc: Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, linux-mm On Sun, Mar 04, 2012 at 01:43:32PM -0800, David Rientjes wrote: > BUG() is a no-op when CONFIG_BUG is disabled, so slab_node() needs a > dummy return value to avoid reaching the end of a non-void function. > > Signed-off-by: David Rientjes <rientjes@google.com> > --- Nice catch! Reviewed-by: Rafael Aquini <aquini@redhat.com> -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: dummy slab_node return value for bugless kernels 2012-03-04 21:43 [patch] mm, mempolicy: dummy slab_node return value for bugless kernels David Rientjes 2012-03-06 20:15 ` Rafael Aquini @ 2012-03-07 0:08 ` Andrew Morton 2012-03-07 0:55 ` Rafael Aquini 2012-03-07 4:25 ` David Rientjes 1 sibling, 2 replies; 16+ messages in thread From: Andrew Morton @ 2012-03-07 0:08 UTC (permalink / raw) To: David Rientjes; +Cc: KAMEZAWA Hiroyuki, KOSAKI Motohiro, linux-mm On Sun, 4 Mar 2012 13:43:32 -0800 (PST) David Rientjes <rientjes@google.com> wrote: > BUG() is a no-op when CONFIG_BUG is disabled, so slab_node() needs a > dummy return value to avoid reaching the end of a non-void function. > > Signed-off-by: David Rientjes <rientjes@google.com> > --- > mm/mempolicy.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -1611,6 +1611,7 @@ unsigned slab_node(struct mempolicy *policy) > > default: > BUG(); > + return numa_node_id(); > } > } Wait. If the above code generated a warning then surely we get a *lot* of warnings! I'd expect that a lot of code assumes that BUG() never returns? Can we fix this within the BUG() definition? I can't think of a way, unless gcc gives us a way of accessing the return type of the current function, and I don't think it does that. Also, does CONIG_BUG=n even make sense? If we got here and we know that the kernel has malfunctioned, what point is there in pretending otherwise? Odd. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: dummy slab_node return value for bugless kernels 2012-03-07 0:08 ` Andrew Morton @ 2012-03-07 0:55 ` Rafael Aquini 2012-03-07 4:25 ` David Rientjes 1 sibling, 0 replies; 16+ messages in thread From: Rafael Aquini @ 2012-03-07 0:55 UTC (permalink / raw) To: Andrew Morton Cc: David Rientjes, KAMEZAWA Hiroyuki, KOSAKI Motohiro, linux-mm On Tue, Mar 06, 2012 at 04:08:33PM -0800, Andrew Morton wrote: > On Sun, 4 Mar 2012 13:43:32 -0800 (PST) > David Rientjes <rientjes@google.com> wrote: > > > BUG() is a no-op when CONFIG_BUG is disabled, so slab_node() needs a > > dummy return value to avoid reaching the end of a non-void function. > > > > Signed-off-by: David Rientjes <rientjes@google.com> > > --- > > mm/mempolicy.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > > --- a/mm/mempolicy.c > > +++ b/mm/mempolicy.c > > @@ -1611,6 +1611,7 @@ unsigned slab_node(struct mempolicy *policy) > > > > default: > > BUG(); > > + return numa_node_id(); > > } > > } > > Wait. If the above code generated a warning then surely we get a *lot* > of warnings! I'd expect that a lot of code assumes that BUG() never > returns? In a quick make (ARCH=um defconfig | CONFIG_BUG=n) the following four warnings have popped out: kernel/sched/core.c:3144:1: warning: control reaches end of non-void function [-Wreturn-type] mm/bootmem.c:352:1: warning: control reaches end of non-void function [-Wreturn-type] fs/locks.c:1469:1: warning: control reaches end of non-void function [-Wreturn-type] block/cfq-iosched.c:2912:1: warning: control reaches end of non-void function [-Wreturn-type] net/core/ethtool.c:211:1: warning: control reaches end of non-void function [-Wreturn-type] So, yes... Unfortunately, we would see a lot more warnings for a (more) complete kernel configuration. > > Can we fix this within the BUG() definition? I can't think of a way, > unless gcc gives us a way of accessing the return type of the current > function, and I don't think it does that. > > > Also, does CONIG_BUG=n even make sense? If we got here and we know > that the kernel has malfunctioned, what point is there in pretending > otherwise? Odd. I admit I was thinking about in follow David's example and start chasing similar cases to propose a janitorial patch, however, I couldn't agree more with your point here. It seems odd turning CONFIG_BUG off and neglect well known buggy conditions within the code. Perhaps, then, the best way to cope with this oddity would be just drop CONFIG_BUG config knob at all, making it permanently "on". Any other thoughts? Rafael -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: dummy slab_node return value for bugless kernels 2012-03-07 0:08 ` Andrew Morton 2012-03-07 0:55 ` Rafael Aquini @ 2012-03-07 4:25 ` David Rientjes 2012-03-07 4:29 ` [patch] mm, mempolicy: make mempolicies robust against errors David Rientjes 2012-03-07 11:12 ` [patch] mm, mempolicy: dummy slab_node return value for bugless kernels Glauber Costa 1 sibling, 2 replies; 16+ messages in thread From: David Rientjes @ 2012-03-07 4:25 UTC (permalink / raw) To: Andrew Morton; +Cc: KAMEZAWA Hiroyuki, KOSAKI Motohiro, linux-mm On Tue, 6 Mar 2012, Andrew Morton wrote: > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > > --- a/mm/mempolicy.c > > +++ b/mm/mempolicy.c > > @@ -1611,6 +1611,7 @@ unsigned slab_node(struct mempolicy *policy) > > > > default: > > BUG(); > > + return numa_node_id(); > > } > > } > > Wait. If the above code generated a warning then surely we get a *lot* > of warnings! I'd expect that a lot of code assumes that BUG() never > returns? > allyesconfig with CONFIG_BUG=n results in 50 such warnings tree wide, and this is the only one in mm/*. > Also, does CONIG_BUG=n even make sense? If we got here and we know > that the kernel has malfunctioned, what point is there in pretending > otherwise? Odd. > I don't suspect we'll be very popular if we try to remove it, I can see how it would be useful when BUG() is used when the problem isn't really fatal (to stop something like disk corruption), like the above case isn't. If policy->mode isn't one of MPOL_{BIND,INTERLEAVE,PREFERRED} then we'd want WARN_ON_ONCE() at best; someone either didn't test their patch or we've flipped a bit, but the kernel can run happily along using the local node for slab allocations while still notifying the user. mm/mempolicy.c misuses BUG() in every case, Not having the perfect NUMA optimizations is surely annoying, but let's not crash someone's kernel. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch] mm, mempolicy: make mempolicies robust against errors 2012-03-07 4:25 ` David Rientjes @ 2012-03-07 4:29 ` David Rientjes 2012-03-07 5:30 ` KOSAKI Motohiro 2012-03-07 11:12 ` [patch] mm, mempolicy: dummy slab_node return value for bugless kernels Glauber Costa 1 sibling, 1 reply; 16+ messages in thread From: David Rientjes @ 2012-03-07 4:29 UTC (permalink / raw) To: Andrew Morton; +Cc: KAMEZAWA Hiroyuki, KOSAKI Motohiro, linux-mm It's unnecessary to BUG() in situations when a mempolicy has an unsupported mode, it just means that a mode doesn't have complete coverage in all mempolicy functions -- which is an error, but not a fatal error -- or that a bit has flipped. Regardless, it's sufficient to warn the user in the kernel log of the situation once and then proceed without crashing the system. This patch converts nearly all the BUG()'s in mm/mempolicy.c to WARN_ON_ONCE(1) and provides the necessary code to return successfully. Signed-off-by: David Rientjes <rientjes@google.com> --- mm/mempolicy.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/mm/mempolicy.c b/mm/mempolicy.c --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -303,7 +303,7 @@ static void mpol_rebind_default(struct mempolicy *pol, const nodemask_t *nodes, static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes, enum mpol_rebind_step step) { - nodemask_t tmp; + nodemask_t tmp = NODE_MASK_NONE; if (pol->flags & MPOL_F_STATIC_NODES) nodes_and(tmp, pol->w.user_nodemask, *nodes); @@ -322,7 +322,7 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes, tmp = pol->w.cpuset_mems_allowed; pol->w.cpuset_mems_allowed = *nodes; } else - BUG(); + WARN_ON_ONCE(1); } if (nodes_empty(tmp)) @@ -333,7 +333,7 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes, else if (step == MPOL_REBIND_ONCE || step == MPOL_REBIND_STEP2) pol->v.nodes = tmp; else - BUG(); + WARN_ON_ONCE(1); if (!node_isset(current->il_next, tmp)) { current->il_next = next_node(current->il_next, tmp); @@ -397,15 +397,19 @@ static void mpol_rebind_policy(struct mempolicy *pol, const nodemask_t *newmask, if (step == MPOL_REBIND_STEP1 && (pol->flags & MPOL_F_REBINDING)) return; - if (step == MPOL_REBIND_STEP2 && !(pol->flags & MPOL_F_REBINDING)) - BUG(); + if (step == MPOL_REBIND_STEP2 && !(pol->flags & MPOL_F_REBINDING)) { + WARN_ON_ONCE(1); + return; + } if (step == MPOL_REBIND_STEP1) pol->flags |= MPOL_F_REBINDING; else if (step == MPOL_REBIND_STEP2) pol->flags &= ~MPOL_F_REBINDING; - else if (step >= MPOL_REBIND_NSTEP) - BUG(); + else if (step >= MPOL_REBIND_NSTEP) { + WARN_ON_ONCE(1); + return; + } mpol_ops[pol->mode].rebind(pol, newmask, step); } @@ -789,7 +793,7 @@ static void get_policy_nodemask(struct mempolicy *p, nodemask_t *nodes) /* else return empty node mask for local allocation */ break; default: - BUG(); + WARN_ON_ONCE(1); } } @@ -1552,7 +1556,7 @@ static struct zonelist *policy_zonelist(gfp_t gfp, struct mempolicy *policy, nd = first_node(policy->v.nodes); break; default: - BUG(); + WARN_ON_ONCE(1); } return node_zonelist(nd, gfp); } @@ -1611,7 +1615,7 @@ unsigned slab_node(struct mempolicy *policy) } default: - BUG(); + WARN_ON_ONCE(1); return numa_node_id(); } } @@ -1751,7 +1755,7 @@ bool init_nodemask_of_mempolicy(nodemask_t *mask) break; default: - BUG(); + WARN_ON_ONCE(1); } task_unlock(current); @@ -1796,7 +1800,7 @@ bool mempolicy_nodemask_intersects(struct task_struct *tsk, ret = nodes_intersects(mempolicy->v.nodes, *mask); break; default: - BUG(); + WARN_ON_ONCE(1); } out: task_unlock(tsk); @@ -2005,7 +2009,7 @@ bool __mpol_equal(struct mempolicy *a, struct mempolicy *b) case MPOL_PREFERRED: return a->v.preferred_node == b->v.preferred_node; default: - BUG(); + WARN_ON_ONCE(1); return false; } } @@ -2521,7 +2525,9 @@ int mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol, int no_context) break; default: - BUG(); + WARN_ON_ONCE(1); + mode = MPOL_DEFAULT; + nodes_clear(nodes); } l = strlen(policy_modes[mode]); -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: make mempolicies robust against errors 2012-03-07 4:29 ` [patch] mm, mempolicy: make mempolicies robust against errors David Rientjes @ 2012-03-07 5:30 ` KOSAKI Motohiro 2012-03-07 5:58 ` David Rientjes 0 siblings, 1 reply; 16+ messages in thread From: KOSAKI Motohiro @ 2012-03-07 5:30 UTC (permalink / raw) To: David Rientjes; +Cc: Andrew Morton, KAMEZAWA Hiroyuki, linux-mm 2012/3/6 David Rientjes <rientjes@google.com>: > It's unnecessary to BUG() in situations when a mempolicy has an > unsupported mode, it just means that a mode doesn't have complete coverage > in all mempolicy functions -- which is an error, but not a fatal error -- > or that a bit has flipped. Regardless, it's sufficient to warn the user > in the kernel log of the situation once and then proceed without crashing > the system. > > This patch converts nearly all the BUG()'s in mm/mempolicy.c to > WARN_ON_ONCE(1) and provides the necessary code to return successfully. I'm sorry. I simple don't understand the purpose of this patch. every mem policy syscalls have input check then we can't hit BUG()s in mempolicy.c. To me, BUG() is obvious notation than WARN_ON_ONCE(). We usually use WARN_ON_ONCE() for hw drivers code. Because of, the warn-on mean "we believe this route never reach, but we afraid there is crazy buggy hardware". And, now BUG() has renreachable() annotation. why don't it work? #define BUG() \ do { \ asm volatile("ud2"); \ unreachable(); \ } while (0) -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: make mempolicies robust against errors 2012-03-07 5:30 ` KOSAKI Motohiro @ 2012-03-07 5:58 ` David Rientjes 2012-03-07 6:34 ` KOSAKI Motohiro 2012-04-26 14:58 ` Christoph Lameter 0 siblings, 2 replies; 16+ messages in thread From: David Rientjes @ 2012-03-07 5:58 UTC (permalink / raw) To: KOSAKI Motohiro; +Cc: Andrew Morton, KAMEZAWA Hiroyuki, linux-mm [-- Attachment #1: Type: TEXT/PLAIN, Size: 2124 bytes --] On Wed, 7 Mar 2012, KOSAKI Motohiro wrote: > > It's unnecessary to BUG() in situations when a mempolicy has an > > unsupported mode, it just means that a mode doesn't have complete coverage > > in all mempolicy functions -- which is an error, but not a fatal error -- > > or that a bit has flipped. Regardless, it's sufficient to warn the user > > in the kernel log of the situation once and then proceed without crashing > > the system. > > > > This patch converts nearly all the BUG()'s in mm/mempolicy.c to > > WARN_ON_ONCE(1) and provides the necessary code to return successfully. > > I'm sorry. I simple don't understand the purpose of this patch. every > mem policy syscalls have input check then we can't hit BUG()s in > mempolicy.c. To me, BUG() is obvious notation than WARN_ON_ONCE(). > Right, this patch doesn't functionally change anything except it will (1) continue to warn users when there's a legitimate mempolicy code error by way of WARN_ON_ONCE() (which is good), just without crashing the machine unnecessarily and (2) allow the system to stay alive since no mempolicy error changed by this bug is fatal. We should only be using BUG() when the side-effects of continuing are fatal; doing WARN_ON_ONCE(1) is sufficient annotation, I think, that this code should never be reached -- BUG() has no advantage here. > We usually use WARN_ON_ONCE() for hw drivers code. Because of, the > warn-on mean "we believe this route never reach, but we afraid there > is crazy buggy hardware". > > And, now BUG() has renreachable() annotation. why don't it work? > > > #define BUG() \ > do { \ > asm volatile("ud2"); \ > unreachable(); \ > } while (0) > That's not compiled for CONFIG_BUG=n; such a config fallsback to include/asm-generic/bug.h which just does #define BUG() do {} while (0) because CONFIG_BUG specifically _wants_ to bypass BUG()s and is reasonably protected by CONFIG_EXPERT. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: make mempolicies robust against errors 2012-03-07 5:58 ` David Rientjes @ 2012-03-07 6:34 ` KOSAKI Motohiro 2012-03-07 6:56 ` David Rientjes 2012-03-08 23:51 ` Andrew Morton 2012-04-26 14:58 ` Christoph Lameter 1 sibling, 2 replies; 16+ messages in thread From: KOSAKI Motohiro @ 2012-03-07 6:34 UTC (permalink / raw) To: David Rientjes Cc: KOSAKI Motohiro, Andrew Morton, KAMEZAWA Hiroyuki, linux-mm, kosaki.motohiro (3/7/12 12:58 AM), David Rientjes wrote: > On Wed, 7 Mar 2012, KOSAKI Motohiro wrote: > >>> It's unnecessary to BUG() in situations when a mempolicy has an >>> unsupported mode, it just means that a mode doesn't have complete coverage >>> in all mempolicy functions -- which is an error, but not a fatal error -- >>> or that a bit has flipped. Regardless, it's sufficient to warn the user >>> in the kernel log of the situation once and then proceed without crashing >>> the system. >>> >>> This patch converts nearly all the BUG()'s in mm/mempolicy.c to >>> WARN_ON_ONCE(1) and provides the necessary code to return successfully. >> >> I'm sorry. I simple don't understand the purpose of this patch. every >> mem policy syscalls have input check then we can't hit BUG()s in >> mempolicy.c. To me, BUG() is obvious notation than WARN_ON_ONCE(). >> > > Right, this patch doesn't functionally change anything except it will (1) > continue to warn users when there's a legitimate mempolicy code error by > way of WARN_ON_ONCE() (which is good), just without crashing the machine > unnecessarily and (2) allow the system to stay alive since no mempolicy > error changed by this bug is fatal. We should only be using BUG() when > the side-effects of continuing are fatal; doing WARN_ON_ONCE(1) is > sufficient annotation, I think, that this code should never be reached -- > BUG() has no advantage here. > >> We usually use WARN_ON_ONCE() for hw drivers code. Because of, the >> warn-on mean "we believe this route never reach, but we afraid there >> is crazy buggy hardware". >> >> And, now BUG() has renreachable() annotation. why don't it work? >> >> >> #define BUG() \ >> do { \ >> asm volatile("ud2"); \ >> unreachable(); \ >> } while (0) >> > > That's not compiled for CONFIG_BUG=n; such a config fallsback to > include/asm-generic/bug.h which just does > > #define BUG() do {} while (0) > > because CONFIG_BUG specifically _wants_ to bypass BUG()s and is reasonably > protected by CONFIG_EXPERT. So, I strongly suggest to remove CONFIG_BUG=n. It is neglected very long time and much plenty code assume BUG() is not no-op. I don't think we can fix all place. Just one instruction don't hurt code size nor performance. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: make mempolicies robust against errors 2012-03-07 6:34 ` KOSAKI Motohiro @ 2012-03-07 6:56 ` David Rientjes 2012-03-07 16:24 ` KOSAKI Motohiro 2012-03-08 23:51 ` Andrew Morton 1 sibling, 1 reply; 16+ messages in thread From: David Rientjes @ 2012-03-07 6:56 UTC (permalink / raw) To: KOSAKI Motohiro Cc: KOSAKI Motohiro, Andrew Morton, KAMEZAWA Hiroyuki, linux-mm On Wed, 7 Mar 2012, KOSAKI Motohiro wrote: > So, I strongly suggest to remove CONFIG_BUG=n. It is neglected very long time > and > much plenty code assume BUG() is not no-op. I don't think we can fix all > place. > > Just one instruction don't hurt code size nor performance. > It's a different topic, the proposal here is whether an error in mempolicies (either the code or flipped bit) should crash the kernel or not since it's a condition that can easily be recovered from and leave BUG() to errors that actually are fatal. Crashing the kernel offers no advantage. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: make mempolicies robust against errors 2012-03-07 6:56 ` David Rientjes @ 2012-03-07 16:24 ` KOSAKI Motohiro 2012-03-07 21:06 ` David Rientjes 0 siblings, 1 reply; 16+ messages in thread From: KOSAKI Motohiro @ 2012-03-07 16:24 UTC (permalink / raw) To: rientjes Cc: kosaki.motohiro, kosaki.motohiro, akpm, kamezawa.hiroyu, linux-mm On 3/7/2012 1:56 AM, David Rientjes wrote: > On Wed, 7 Mar 2012, KOSAKI Motohiro wrote: > >> So, I strongly suggest to remove CONFIG_BUG=n. It is neglected very long time >> and >> much plenty code assume BUG() is not no-op. I don't think we can fix all >> place. >> >> Just one instruction don't hurt code size nor performance. > > It's a different topic, the proposal here is whether an error in > mempolicies (either the code or flipped bit) should crash the kernel or > not since it's a condition that can easily be recovered from and leave > BUG() to errors that actually are fatal. Crashing the kernel offers no > advantage. Should crash? The code path never reach. thus there is no ideal behavior. In this case, BUG() is just unreachable annotation. So let's just annotate unreachable() even though CONFIG_BUG=n. WARN_ON_ONCE makes code broat and no positive impact. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: make mempolicies robust against errors 2012-03-07 16:24 ` KOSAKI Motohiro @ 2012-03-07 21:06 ` David Rientjes 0 siblings, 0 replies; 16+ messages in thread From: David Rientjes @ 2012-03-07 21:06 UTC (permalink / raw) To: KOSAKI Motohiro; +Cc: kosaki.motohiro, akpm, kamezawa.hiroyu, linux-mm On Wed, 7 Mar 2012, KOSAKI Motohiro wrote: > > It's a different topic, the proposal here is whether an error in > > mempolicies (either the code or flipped bit) should crash the kernel or > > not since it's a condition that can easily be recovered from and leave > > BUG() to errors that actually are fatal. Crashing the kernel offers no > > advantage. > > Should crash? The code path never reach. thus there is no ideal behavior. > In this case, BUG() is just unreachable annotation. So let's just annotate > unreachable() even though CONFIG_BUG=n. > > WARN_ON_ONCE makes code broat and no positive impact. > I think you misunderstand the difference between WARN() and BUG(). Both are intended to never be reached; the difference is that BUG() is a fatal condition and WARN() is not. All of the changes from BUG() to WARN() in this patch are not fatal and has no other side-effects other memory allocations that are not truly interleaved, for example. These should have been WARN() from the beginning. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: make mempolicies robust against errors 2012-03-07 6:34 ` KOSAKI Motohiro 2012-03-07 6:56 ` David Rientjes @ 2012-03-08 23:51 ` Andrew Morton 1 sibling, 0 replies; 16+ messages in thread From: Andrew Morton @ 2012-03-08 23:51 UTC (permalink / raw) To: KOSAKI Motohiro Cc: David Rientjes, KOSAKI Motohiro, KAMEZAWA Hiroyuki, linux-mm On Wed, 07 Mar 2012 01:34:16 -0500 KOSAKI Motohiro <kosaki.motohiro@gmail.com> wrote: > >> And, now BUG() has renreachable() annotation. why don't it work? > >> > >> > >> #define BUG() \ > >> do { \ > >> asm volatile("ud2"); \ > >> unreachable(); \ > >> } while (0) > >> > > > > That's not compiled for CONFIG_BUG=n; such a config fallsback to > > include/asm-generic/bug.h which just does > > > > #define BUG() do {} while (0) > > > > because CONFIG_BUG specifically _wants_ to bypass BUG()s and is reasonably > > protected by CONFIG_EXPERT. > > So, I strongly suggest to remove CONFIG_BUG=n. It is neglected very long time and > much plenty code assume BUG() is not no-op. I don't think we can fix all place. > > Just one instruction don't hurt code size nor performance. Well yes, CONFIG_BUG=n is a crazy thing to do. a) because programmers universally assume that BUG() doesn't return and b) given that the kernel KNOWS that it is about to fall off a cliff, why would anyone want to deprive themselves of information about the forthcoming crash? So perhaps a good compromise here is to do nothing: let the CONFIG_BUG=n build spew a pile of warnings, and let the crazy CONFIG_BUG=n people suffer. That's if any such people exist... -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: make mempolicies robust against errors 2012-03-07 5:58 ` David Rientjes 2012-03-07 6:34 ` KOSAKI Motohiro @ 2012-04-26 14:58 ` Christoph Lameter 1 sibling, 0 replies; 16+ messages in thread From: Christoph Lameter @ 2012-04-26 14:58 UTC (permalink / raw) To: David Rientjes Cc: KOSAKI Motohiro, Andrew Morton, KAMEZAWA Hiroyuki, linux-mm On Tue, 6 Mar 2012, David Rientjes wrote: > That's not compiled for CONFIG_BUG=n; such a config fallsback to > include/asm-generic/bug.h which just does > > #define BUG() do {} while (0) > > because CONFIG_BUG specifically _wants_ to bypass BUG()s and is reasonably > protected by CONFIG_EXPERT. Why would anyone do this? IMHO if you disable CONFIG_BUG and things explode then its your fault. If we must have the ability then make BUG() fallback to something that quiets down the compiler (and set some kind of an "idiot" flag in the tainted flags please so that we can ignore bug reports like that). -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: dummy slab_node return value for bugless kernels 2012-03-07 4:25 ` David Rientjes 2012-03-07 4:29 ` [patch] mm, mempolicy: make mempolicies robust against errors David Rientjes @ 2012-03-07 11:12 ` Glauber Costa 2012-03-07 21:04 ` David Rientjes 1 sibling, 1 reply; 16+ messages in thread From: Glauber Costa @ 2012-03-07 11:12 UTC (permalink / raw) To: David Rientjes Cc: Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, linux-mm On 03/07/2012 08:25 AM, David Rientjes wrote: > On Tue, 6 Mar 2012, Andrew Morton wrote: > >>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c >>> --- a/mm/mempolicy.c >>> +++ b/mm/mempolicy.c >>> @@ -1611,6 +1611,7 @@ unsigned slab_node(struct mempolicy *policy) >>> >>> default: >>> BUG(); >>> + return numa_node_id(); >>> } >>> } >> >> Wait. If the above code generated a warning then surely we get a *lot* >> of warnings! I'd expect that a lot of code assumes that BUG() never >> returns? >> > > allyesconfig with CONFIG_BUG=n results in 50 such warnings tree wide, and > this is the only one in mm/*. > >> Also, does CONIG_BUG=n even make sense? If we got here and we know >> that the kernel has malfunctioned, what point is there in pretending >> otherwise? Odd. >> > > I don't suspect we'll be very popular if we try to remove it, I can see > how it would be useful when BUG() is used when the problem isn't really > fatal (to stop something like disk corruption), like the above case isn't. I guess everyone that is able to track the problem back to an instance of BUG(), be skilled enough to be sure it is not fatal, and then recompile the kernel with this option (that I bet many of us didn't even know that existed), can very well just change it to a WARN_*, (and maybe patch it upstream). -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] mm, mempolicy: dummy slab_node return value for bugless kernels 2012-03-07 11:12 ` [patch] mm, mempolicy: dummy slab_node return value for bugless kernels Glauber Costa @ 2012-03-07 21:04 ` David Rientjes 0 siblings, 0 replies; 16+ messages in thread From: David Rientjes @ 2012-03-07 21:04 UTC (permalink / raw) To: Glauber Costa; +Cc: Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, linux-mm On Wed, 7 Mar 2012, Glauber Costa wrote: > > I don't suspect we'll be very popular if we try to remove it, I can see > > how it would be useful when BUG() is used when the problem isn't really > > fatal (to stop something like disk corruption), like the above case isn't. > I guess everyone that is able to track the problem back to an instance of > BUG(), be skilled enough to be sure it is not fatal, and then recompile the > kernel with this option (that I bet many of us didn't even know that existed), > can very well just change it to a WARN_*, (and maybe patch it upstream). > That's the point of the next patch which changes this to a WARN_ON_ONCE(1) because all of the BUG()'s that it changes in mm/mempolicy.c aren't fatal. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-04-26 14:58 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-04 21:43 [patch] mm, mempolicy: dummy slab_node return value for bugless kernels David Rientjes 2012-03-06 20:15 ` Rafael Aquini 2012-03-07 0:08 ` Andrew Morton 2012-03-07 0:55 ` Rafael Aquini 2012-03-07 4:25 ` David Rientjes 2012-03-07 4:29 ` [patch] mm, mempolicy: make mempolicies robust against errors David Rientjes 2012-03-07 5:30 ` KOSAKI Motohiro 2012-03-07 5:58 ` David Rientjes 2012-03-07 6:34 ` KOSAKI Motohiro 2012-03-07 6:56 ` David Rientjes 2012-03-07 16:24 ` KOSAKI Motohiro 2012-03-07 21:06 ` David Rientjes 2012-03-08 23:51 ` Andrew Morton 2012-04-26 14:58 ` Christoph Lameter 2012-03-07 11:12 ` [patch] mm, mempolicy: dummy slab_node return value for bugless kernels Glauber Costa 2012-03-07 21:04 ` David Rientjes
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).