[PATCH] 2.5.7 acct.c oops

[PATCH] 2.5.7 acct.c oops

[Bob Miller]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.537   -> 1.538  
#	       kernel/acct.c	1.8     -> 1.9    
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/03/21	rem@doc.pdx.osdl.net	1.538
# Fixed acct.c code by removing the BUG_ON code because it doesn't work
# on UP systems.
# --------------------------------------------
#
diff -Nru a/kernel/acct.c b/kernel/acct.c
--- a/kernel/acct.c	Thu Mar 21 11:32:05 2002
+++ b/kernel/acct.c	Thu Mar 21 11:32:05 2002
@@ -166,8 +166,6 @@
 {
 	struct file *old_acct = NULL;
 
-	BUG_ON(!spin_is_locked(&acct_globals.lock));
-
 	if (acct_globals.file) {
 		old_acct = acct_globals.file;
 		del_timer(&acct_globals.timer);

Re: [PATCH] 2.5.7 acct.c oops

[Bill Davidsen]

On Thu, 21 Mar 2002, Bob Miller wrote:

> # The following is the BitKeeper ChangeSet Log
> # --------------------------------------------
> # 02/03/21	rem@doc.pdx.osdl.net	1.538
> # Fixed acct.c code by removing the BUG_ON code because it doesn't work
> # on UP systems.
> # --------------------------------------------

  Please help my education... after looking at the code, I don't see why
the BUG_ON was removed rather than made dependent on SMP, assuming that
the BK comment and my hasty reading of code actually mean that it did work
for SMP.

  Is this a solid "can't happen" now and no test needed, or is a better
test in the works, or ???

  I didn't try to compile it, so there may be something I'm totally
missing.

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: [PATCH] 2.5.7 acct.c oops

[Robert Love]

On Thu, 2002-03-21 at 16:26, Bill Davidsen wrote:

>   Please help my education... after looking at the code, I don't see why
> the BUG_ON was removed rather than made dependent on SMP, assuming that
> the BK comment and my hasty reading of code actually mean that it did work
> for SMP.
> 
>   Is this a solid "can't happen" now and no test needed, or is a better
> test in the works, or ???
> 
>   I didn't try to compile it, so there may be something I'm totally
> missing.

While he could of wrapped the test dependent in #ifdef/#endif
CONFIG_SMP, the test really is not overly needed.  It is more a result
of the previous code, which Bob Miller himself fixed up and then much
rewrote the locking for.

Since he recently did the cleanup (and even added that BUG_ON) I trust
he knows if we can remove it.

	Robert Love

Re: [PATCH] 2.5.7 acct.c oops

[Bob Miller]

On Thu, Mar 21, 2002 at 04:34:49PM -0500, Robert Love wrote:
> On Thu, 2002-03-21 at 16:26, Bill Davidsen wrote:
> 
> >   Please help my education... after looking at the code, I don't see why
> > the BUG_ON was removed rather than made dependent on SMP, assuming that
> > the BK comment and my hasty reading of code actually mean that it did work
> > for SMP.
> > 
> >   Is this a solid "can't happen" now and no test needed, or is a better
> > test in the works, or ???
> > 
> >   I didn't try to compile it, so there may be something I'm totally
> > missing.
> 
> While he could of wrapped the test dependent in #ifdef/#endif
> CONFIG_SMP, the test really is not overly needed.  It is more a result
> of the previous code, which Bob Miller himself fixed up and then much
> rewrote the locking for.
> 
> Since he recently did the cleanup (and even added that BUG_ON) I trust
> he knows if we can remove it.
> 
> 	Robert Love

As Robert has sermized...  When I did the cleanup I debated whether to put
the BUG_ON() in there in the first place.  I used it as an extra bit of
paranoia not knowing that it would not return correct results on a UP.

-- 
Bob Miller					Email: rem@osdl.org
Open Source Development Lab			Phone: 503.626.2455 Ext. 17

Re: [PATCH] 2.5.7 acct.c oops

[Bill Davidsen]

On Thu, 21 Mar 2002, Bob Miller wrote:

> On Thu, Mar 21, 2002 at 04:34:49PM -0500, Robert Love wrote:

> > While he could of wrapped the test dependent in #ifdef/#endif
> > CONFIG_SMP, the test really is not overly needed.  It is more a result
> > of the previous code, which Bob Miller himself fixed up and then much
> > rewrote the locking for.
> > 
> > Since he recently did the cleanup (and even added that BUG_ON) I trust
> > he knows if we can remove it.
> > 
> > 	Robert Love
> 
> As Robert has sermized...  When I did the cleanup I debated whether to put
> the BUG_ON() in there in the first place.  I used it as an extra bit of
> paranoia not knowing that it would not return correct results on a UP.

  Okay, that's a reasonable decision. I'm all in favor of paranoia, any
problem at this point would be best detected early on, nut if you're
convinced that it's safe without it, I certainly can't argue, and wasn't
intending to initially.

  Actually I don't think CONFIG_SMP would completely avoid problems, if I
read the code right it would still have a problem if you had an SMP kernel
running "nosmp"  option at boot (or possibly booted on a uni, although I
haven't tried w/o 'nosmp'). 

  Thanks for all the information, clearly if any check is needed, that
check is not the one, at least as is.

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.