| Date | Sun, 8 Jun 2008 11:11:48 -0400 | From | Rik van Riel <> | 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@linux-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.
|