[patch] mm, mempolicy: dummy slab_node return value for bugless kernels

[patch] mm, mempolicy: dummy slab_node return value for bugless kernels

[David Rientjes]

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>

Re: [patch] mm, mempolicy: dummy slab_node return value for bugless kernels

[Rafael Aquini]

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>

Re: [patch] mm, mempolicy: dummy slab_node return value for bugless kernels

[Andrew Morton]

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>

Re: [patch] mm, mempolicy: dummy slab_node return value for bugless kernels

[Rafael Aquini]

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>

Re: [patch] mm, mempolicy: dummy slab_node return value for bugless kernels

[David Rientjes]

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>

[patch] mm, mempolicy: make mempolicies robust against errors

[David Rientjes]

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>

Re: [patch] mm, mempolicy: make mempolicies robust against errors

[KOSAKI Motohiro]

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>

Re: [patch] mm, mempolicy: make mempolicies robust against errors

[David Rientjes]

[-- 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.

Re: [patch] mm, mempolicy: make mempolicies robust against errors

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

Re: [patch] mm, mempolicy: make mempolicies robust against errors

[David Rientjes]

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>

Re: [patch] mm, mempolicy: dummy slab_node return value for bugless kernels

[Glauber Costa]

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>

Re: [patch] mm, mempolicy: make mempolicies robust against errors

[KOSAKI Motohiro]

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>

Re: [patch] mm, mempolicy: dummy slab_node return value for bugless kernels

[David Rientjes]

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>

Re: [patch] mm, mempolicy: make mempolicies robust against errors

[David Rientjes]

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>

Re: [patch] mm, mempolicy: make mempolicies robust against errors

[Andrew Morton]

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>

Re: [patch] mm, mempolicy: make mempolicies robust against errors

[Christoph Lameter]

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>