regarding [PATCH mmotm] vmscan: raise the bar to PAGEOUT_IO_SYNC stalls

[Kail]

I think there is an error in the porting of the patch in subject in
zen-stable.
Looking at http://lkml.org/lkml/2010/8/1/40 and confirmed by this
porting to .35 http://www.phoronix.com/forums/showpost.php?p=140694&postcount=23
i see that the result of shrink_page_list(&page_list, sc,
PAGEOUT_IO_ASYNC) is called nr_reclaimed in the official patch but is
called nr_freed in the current kernel ( both .34 and .35 ). On that
return is called the new should_reclaim_stall(..).

But since the naming change and the fact that nr_reclaimed actually
exists even if with a different meaning it compiles fine. But i still
think the following change is needed:

- if (should_reclaim_stall(nr_taken, nr_reclaimed, priority, sc)) {
+ if (should_reclaim_stall(nr_taken, nr_freed, priority, sc)) {

I'm not a vm expert so i don't really know the difference between the
current nr_reclaimed and the other but seems to me that the Wu
Fengguang idea was to check against the return of shrink_page_list.

[Steven Barrett]

Hi Wu,

This was sent to the Zen-Kernel mailing list, do you think the code change suggested is a bugfix?

I did notice myself that the variable nr_reclaimed and nr_freed are inconsistent when you look at the variable you are passing to should_reclaim_stall(...), and the variable it initializes as in the function header.

-------- Original Message --------

Subject:	regarding [PATCH mmotm] vmscan: raise the bar to PAGEOUT_IO_SYNC stalls
Date:	Fri, 6 Aug 2010 02:55:03 -0700 (PDT)
From:	Kail <kai...@gmail.com>
Reply-To:	zen_k...@googlegroups.com
To:	Zen-Kernel <zen_k...@googlegroups.com>

[Steven Barrett]

The patch I initially committed to 2.6.35 on the zen-kernel project is
here:

http://git.zen-kernel.org/?p=kernel/zen.git;a=commit;h=6d1a3963a64325da2bc5d975c8df2807fc826af5

I also committed the change in your new (or old, depends how you look at
it) patch:

http://git.zen-kernel.org/?p=kernel/zen.git;a=commit;h=749cd85bcfbdf1863456abda9e078d7046d3a744

Thanks!

On 08/07/2010 12:04 PM, Wu Fengguang wrote:
> Hi Steven,

>
> On Sat, Aug 07, 2010 at 11:05:49AM +0800, Steven Barrett wrote:
>> Hi Wu,
>>
>> This was sent to the Zen-Kernel mailing list, do you think the code change suggested is a bugfix?

> Would you forward the whole change? Thanks.

>
>> I did notice myself that the variable nr_reclaimed and nr_freed are inconsistent when you look at the variable you are passing to should_reclaim_stall(...), and the variable it initializes as in the function header.

> Attached was my original patch for linux-next. In fact there was no
> such naming confusion there.
>
> When rebasing the patch to mmotm, I found the nr_freed in
> shrink_inactive_list() was renamed to nr_reclaimed. So I did minimal
> change to make the patch work. The function should_reclaim_stall()
> continues to use nr_freed, while the call site is converted to
> nr_reclaim.
>
> This is ugly. Sorry for that. I should have renamed all nr_freed
> occurrences..
>
> Thanks,
> Fengguang