Re: [Kernel-janitors] [PATCH] Check return code in mm/vmscan.c

Re: [Kernel-janitors] [PATCH] Check return code in mm/vmscan.c

[Matthew Wilcox]

On Sat, Jan 03, 2004 at 09:25:24PM +0800, Eugene Teo wrote:
> http://www.anomalistic.org/patches/vmscan-check-ret-kernel_thread-fix-2.6.1-rc1-mm1.patch
> 
> diff -Naur -X /home/amnesia/w/dontdiff 2.6.1-rc1-mm1/mm/vmscan.c 2.6.1-rc1-mm1-fix/mm/vmscan.c
> --- 2.6.1-rc1-mm1/mm/vmscan.c	2004-01-03 20:33:39.000000000 +0800
> +++ 2.6.1-rc1-mm1-fix/mm/vmscan.c	2004-01-03 21:16:30.000000000 +0800
> @@ -1093,10 +1093,16 @@
>  
>  static int __init kswapd_init(void)
>  {
> +	int ret;
>  	pg_data_t *pgdat;
>  	swap_setup();
> -	for_each_pgdat(pgdat)
> -		kernel_thread(kswapd, pgdat, CLONE_KERNEL);
> +	for_each_pgdat(pgdat) {
> +		ret = kernel_thread(kswapd, pgdat, CLONE_KERNEL);
> +		if (ret < 0) {
> +			printk("%s: unable to start kernel thread\n", __FUNCTION__);
> +			return ret;
> +		}
> +	}
>  	total_memory = nr_free_pagecache_pages();
>  	return 0;
>  }

If your new code is triggered, we've just failed to set up total_memory.
I expect the system to behave very oddly after this ;-)

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain
--
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/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: [Kernel-janitors] [PATCH] Check return code in mm/vmscan.c

[Eugene Teo]

<quote sender="Matthew Wilcox">
> On Sat, Jan 03, 2004 at 09:25:24PM +0800, Eugene Teo wrote:
> > http://www.anomalistic.org/patches/vmscan-check-ret-kernel_thread-fix-2.6.1-rc1-mm1.patch
> > 
> > diff -Naur -X /home/amnesia/w/dontdiff 2.6.1-rc1-mm1/mm/vmscan.c 2.6.1-rc1-mm1-fix/mm/vmscan.c
> > --- 2.6.1-rc1-mm1/mm/vmscan.c	2004-01-03 20:33:39.000000000 +0800
> > +++ 2.6.1-rc1-mm1-fix/mm/vmscan.c	2004-01-03 21:16:30.000000000 +0800
> > @@ -1093,10 +1093,16 @@
> >  
> >  static int __init kswapd_init(void)
> >  {
> > +	int ret;
> >  	pg_data_t *pgdat;
> >  	swap_setup();
> > -	for_each_pgdat(pgdat)
> > -		kernel_thread(kswapd, pgdat, CLONE_KERNEL);
> > +	for_each_pgdat(pgdat) {
> > +		ret = kernel_thread(kswapd, pgdat, CLONE_KERNEL);
> > +		if (ret < 0) {
> > +			printk("%s: unable to start kernel thread\n", __FUNCTION__);
> > +			return ret;
> > +		}
> > +	}
> >  	total_memory = nr_free_pagecache_pages();
> >  	return 0;
> >  }
> 
> If your new code is triggered, we've just failed to set up total_memory.
> I expect the system to behave very oddly after this ;-)

a panic call seems to be more appropriate :)

Here is the new fix. Patch compiles, and tested.

http://www.anomalistic.org/patches/vmscan-check-ret-kernel_thread-fix-2.6.1-rc1-mm1.patch

diff -Naur -X /home/amnesia/w/dontdiff 2.6.1-rc1-mm1/mm/vmscan.c 2.6.1-rc1-mm1-fix/mm/vmscan.c
--- 2.6.1-rc1-mm1/mm/vmscan.c	2004-01-04 10:29:24.000000000 +0800
+++ 2.6.1-rc1-mm1-fix/mm/vmscan.c	2004-01-04 13:04:52.000000000 +0800
@@ -1093,10 +1093,14 @@
 
 static int __init kswapd_init(void)
 {
+	int ret;
 	pg_data_t *pgdat;
 	swap_setup();
-	for_each_pgdat(pgdat)
-		kernel_thread(kswapd, pgdat, CLONE_KERNEL);
+	for_each_pgdat(pgdat) {
+		ret = kernel_thread(kswapd, pgdat, CLONE_KERNEL);
+		if (ret < 0)
+			panic("%s: unable to initialise kswapd\n", __FUNCTION__);
+	}
 	total_memory = nr_free_pagecache_pages();
 	return 0;
 }

-- 
Eugene TEO   <eugeneteo@eugeneteo.net>   <http://www.anomalistic.org/>
1024D/14A0DDE5 print D851 4574 E357 469C D308  A01E 7321 A38A 14A0 DDE5
main(i) { putchar(182623909 >> (i-1) * 5&31|!!(i<7)<<6) && main(++i); }

--
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/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: [Kernel-janitors] [PATCH] Check return code in mm/vmscan.c

[Randy.Dunlap]

On Sun, 4 Jan 2004 13:11:26 +0800 Eugene Teo <eugene.teo@eugeneteo.net> wrote:

| <quote sender="Matthew Wilcox">
| > On Sat, Jan 03, 2004 at 09:25:24PM +0800, Eugene Teo wrote:
| > > http://www.anomalistic.org/patches/vmscan-check-ret-kernel_thread-fix-2.6.1-rc1-mm1.patch
| > > 
| > > diff -Naur -X /home/amnesia/w/dontdiff 2.6.1-rc1-mm1/mm/vmscan.c 2.6.1-rc1-mm1-fix/mm/vmscan.c
| > > --- 2.6.1-rc1-mm1/mm/vmscan.c	2004-01-03 20:33:39.000000000 +0800
| > > +++ 2.6.1-rc1-mm1-fix/mm/vmscan.c	2004-01-03 21:16:30.000000000 +0800
| > > @@ -1093,10 +1093,16 @@
| > >  
| > >  static int __init kswapd_init(void)
| > >  {
| > > +	int ret;
| > >  	pg_data_t *pgdat;
| > >  	swap_setup();
| > > -	for_each_pgdat(pgdat)
| > > -		kernel_thread(kswapd, pgdat, CLONE_KERNEL);
| > > +	for_each_pgdat(pgdat) {
| > > +		ret = kernel_thread(kswapd, pgdat, CLONE_KERNEL);
| > > +		if (ret < 0) {
| > > +			printk("%s: unable to start kernel thread\n", __FUNCTION__);
| > > +			return ret;
| > > +		}
| > > +	}
| > >  	total_memory = nr_free_pagecache_pages();
| > >  	return 0;
| > >  }
| > 
| > If your new code is triggered, we've just failed to set up total_memory.
| > I expect the system to behave very oddly after this ;-)
| 
| a panic call seems to be more appropriate :)
| 
| Here is the new fix. Patch compiles, and tested.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Eugene, patch is tested how?  What do you mean by "tested"?
In what conditions?

| http://www.anomalistic.org/patches/vmscan-check-ret-kernel_thread-fix-2.6.1-rc1-mm1.patch
| 
| diff -Naur -X /home/amnesia/w/dontdiff 2.6.1-rc1-mm1/mm/vmscan.c 2.6.1-rc1-mm1-fix/mm/vmscan.c
| --- 2.6.1-rc1-mm1/mm/vmscan.c	2004-01-04 10:29:24.000000000 +0800
| +++ 2.6.1-rc1-mm1-fix/mm/vmscan.c	2004-01-04 13:04:52.000000000 +0800
| @@ -1093,10 +1093,14 @@
|  
|  static int __init kswapd_init(void)
|  {
| +	int ret;
|  	pg_data_t *pgdat;
|  	swap_setup();
| -	for_each_pgdat(pgdat)
| -		kernel_thread(kswapd, pgdat, CLONE_KERNEL);
| +	for_each_pgdat(pgdat) {
| +		ret = kernel_thread(kswapd, pgdat, CLONE_KERNEL);
| +		if (ret < 0)
| +			panic("%s: unable to initialise kswapd\n", __FUNCTION__);
| +	}
|  	total_memory = nr_free_pagecache_pages();
|  	return 0;
|  }
| 

--
~Randy
kernel-janitors project:  http://janitor.kernelnewbies.org/
--
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/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>