Re: [PATCH 19/34] mm: move most file-based accounting to the node

[Johannes Weiner]

On Fri, Jul 08, 2016 at 10:34:55AM +0100, Mel Gorman wrote:
> There are now a number of accounting oddities such as mapped file pages
> being accounted for on the node while the total number of file pages are
> accounted on the zone. This can be coped with to some extent but it's
> confusing so this patch moves the relevant file-based accounted. Due to
> throttling logic in the page allocator for reliable OOM detection, it is
> still necessary to track dirty and writeback pages on a per-zone basis.
> 
> Signed-off-by: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>
> Acked-by: Vlastimil Babka <vbabka@xxxxxxx>
> Acked-by: Michal Hocko <mhocko@xxxxxxxx>

The straight conversion bits are mind-numbing to review, so I focussed
mostly on the NR_ZONE_WRITE_PENDING sites. They look good to me except
for the migration one:

> @@ -505,15 +505,17 @@ int migrate_page_move_mapping(struct address_space *mapping,
>  	 * are mapped to swap space.
>  	 */
>  	if (newzone != oldzone) {
> -		__dec_zone_state(oldzone, NR_FILE_PAGES);
> -		__inc_zone_state(newzone, NR_FILE_PAGES);
> +		__dec_node_state(oldzone->zone_pgdat, NR_FILE_PAGES);
> +		__inc_node_state(newzone->zone_pgdat, NR_FILE_PAGES);
>  		if (PageSwapBacked(page) && !PageSwapCache(page)) {
> -			__dec_zone_state(oldzone, NR_SHMEM);
> -			__inc_zone_state(newzone, NR_SHMEM);
> +			__dec_node_state(oldzone->zone_pgdat, NR_SHMEM);
> +			__inc_node_state(newzone->zone_pgdat, NR_SHMEM);
>  		}
>  		if (dirty && mapping_cap_account_dirty(mapping)) {
> -			__dec_zone_state(oldzone, NR_FILE_DIRTY);
> -			__inc_zone_state(newzone, NR_FILE_DIRTY);
> +			__dec_node_state(oldzone->zone_pgdat, NR_FILE_DIRTY);
> +			__dec_zone_state(oldzone, NR_ZONE_WRITE_PENDING);
> +			__inc_node_state(newzone->zone_pgdat, NR_FILE_DIRTY);
> +			__dec_zone_state(newzone, NR_ZONE_WRITE_PENDING);

That double dec of NR_ZONE_WRITE_PENDING should be dec(old) -> inc(new).

Otherwise, the patch looks good to me.
Acked-by: Johannes Weiner <hannes@xxxxxxxxxxx>