lkml.org 
[lkml]   [2010]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH 2/2] vmscan: don't subtraction of unsined
Date
> On Fri, 25 Jun 2010, KOSAKI Motohiro wrote:
>
> > 'slab_reclaimable' and 'nr_pages' are unsigned. so, subtraction is
> > unsafe.
>
> Why? We are subtracting the current value of NR_SLAB_RECLAIMABLE from the
> earlier one. The result can be negative (maybe concurrent allocations) and
> then the nr_reclaimed gets decremented instead. This is okay since we
> have not reached our goal then of reducing the number of reclaimable slab
> pages on the zone.

It's unsigned. negative mean very big value. so

"zone_page_state(zone, NR_SLAB_RECLAIMABLE) > slab_reclaimable - nr_pages)" will
be evaluated false.

ok, your mysterious 'order' parameter (as pointed [1/2]) almostly prevent this case.
because passing 'order' makes very heavy slab pressure and it avoid negative occur.

but unnaturall coding style can make confusing to reviewers. ya, it's not
big issue. but I also don't find no fixing reason.


>
> > @@ -2622,17 +2624,21 @@ static int __zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int order)
> > * Note that shrink_slab will free memory on all zones and may
> > * take a long time.
> > */
> > - while (shrink_slab(sc.nr_scanned, gfp_mask, lru_pages) &&
> > - zone_page_state(zone, NR_SLAB_RECLAIMABLE) >
> > - slab_reclaimable - nr_pages)
>
> The comparison could be a problem here. So
>
> zone_page_state(zone, NR_SLAB_RECLAIMABLE) + nr_pages >
> slab_reclaimable
>
> ?

My patch take the same thing. but It avoided two line comparision.
Do you mean you like this style? (personally, I don't). If so, I'll
repost this patch.







\
 
 \ /
  Last update: 2010-06-28 04:29    [W:0.478 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site