Re: [PATCH 1/5] vmscan: remove all_unreclaimable check from direct reclaim path completely

From: KOSAKI Motohiro
Date: Wed Mar 23 2011 - 22:12:06 EST

> On Wed, Mar 23, 2011 at 5:44 PM, KOSAKI Motohiro
> <kosaki.motohiro@xxxxxxxxxxxxxx> wrote:
> >> > Boo.
> >> > You seems forgot why you introduced current all_unreclaimable() function.
> >> > While hibernation, we can't trust all_unreclaimable.
> >>
> >> Hmm. AFAIR, the why we add all_unreclaimable is when the hibernation is going on,
> >> kswapd is freezed so it can't mark the zone->all_unreclaimable.
> >> So I think hibernation can't be a problem.
> >> Am I miss something?
> >
> > Ahh, I missed. thans correct me. Okay, I recognized both mine and your works.
> > Can you please explain why do you like your one than mine?
> Just _simple_ :)
> I don't want to change many lines although we can do it simple and very clear.
> >
> > btw, Your one is very similar andrey's initial patch. If your one is
> > better, I'd like to ack with andrey instead.
> When Andrey sent a patch, I though this as zone_reclaimable() is right
> place to check it than out of zone_reclaimable. Why I didn't ack is
> that Andrey can't explain root cause but you did so you persuade me.
> I don't mind if Andrey move the check in zone_reclaimable and resend
> or I resend with concrete description.
> Anyway, most important thing is good description to show the root cause.
> It is applied to your patch, too.
> You should have written down root cause in description.

honestly, I really dislike to use mixing zone->pages_scanned and
zone->all_unreclaimable. because I think it's no simple. I don't
think it's good taste nor easy to review. Even though you who VM
expert didn't understand this issue at once, it's smell of too
mess code.

therefore, I prefore to take either 1) just remove the function or
2) just only check zone->all_unreclaimable and oom_killer_disabled
instead zone->pages_scanned.

And, I agree I need to rewrite the description.
How's this?