linux-kernel - Re: [PATCH -mm 08/25] add some sanity checks to get_scan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080608111148.21ef0bfb@bree.surriel.com>
Date:	Sun, 8 Jun 2008 11:11:48 -0400
From:	Rik van Riel <riel@...hat.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org, lee.schermerhorn@...com,
	kosaki.motohiro@...fujitsu.com
Subject: Re: [PATCH -mm 08/25] add some sanity checks to get_scan_ratio

On Fri, 6 Jun 2008 18:04:47 -0700
Andrew Morton <akpm@...ux-foundation.org> wrote:

> > +	if (unlikely(zone->recent_scanned_file > file / 4)) {
> 
> I see nothing in the changelog about this and there are no comments. 
> How can a reader possibly work out what you were thinking when this
> was typed in??

Pulled into the main split LRU patch and commented.
 
> Perhaps the (nr_swap_pages <= 0) test could happen earlier on.

Done.
 
> Please quadruple-check this code like a paranoid maniac looking for
> underflows, overflows and divides-by-zero.  Bear in mind that x/(y+1)
> can get a div-by-zero for sufficiently-unepected values of y.

Done that already.

> Oh, so that's what the [0] and [1] in get_scan_ratio() mean.  Perhaps
> doing this:
> 
> 	if (nr_swap_pages <= 0) {
> 		percent[0] = 0;		/* anon */
> 		percent[1] = 100;	/* file */
> 
> would clarify things. 

Added lots of comments on this.

> > +++ linux-2.6.26-rc2-mm1/include/linux/mmzone.h	2008-05-28 12:11:51.000000000 -0400
> > @@ -289,6 +289,8 @@ struct zone {
> >  
> >  	unsigned long		recent_rotated_anon;
> >  	unsigned long		recent_rotated_file;
> > +	unsigned long		recent_scanned_anon;
> > +	unsigned long		recent_scanned_file;
> 
> I think struct zone is sufficiently important and obscure that
> field-by-field /*documentation*/ is needed.  Not as kerneldoc, please -
> better to do it at the definition site

Added documentation for these.

-- 
All rights reversed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/