On Thu, May 31, 2018 at 12:34 PM Ivan Kalvachev <ikalvachev@xxxxxxxxx> wrote:
>
> Fixes commit 69d763fc6d3aee787a3e8c8c35092b4f4960fa5d
> (mm: pin address_space before dereferencing it while isolating an LRU page)
>
> working code:
>
> mapping = page_mapping(page);
> if (mapping && !mapping->a_ops->migratepage)
> return ret;
>
> buggy code:
>
> if (!trylock_page(page))
> return ret;
>
> mapping = page_mapping(page);
> migrate_dirty = mapping && mapping->a_ops->migratepage;
> unlock_page(page);
> if (!migrate_dirty)
> return ret;
>
> The problem is that !(a && b) = (!a || !b) while the old code was (a && !b).
> The commit message of the buggy commit explains the need for locking/unlocking
> around the check but does not give any reason for the change of the condition.
> It seems to be an unintended change.
>
> The result of that change is noticeable under swap pressure.
> Big memory consumers like browsers would have a lot of pages swapped out,
> even pages that are been used actively, causing the process to repeatedly
> block for second or longer. At the same time there would be gigabytes of
> unused free memory (sometimes half of the total RAM).
> The buffers/cache would also be at minimum size.
>
> Fixes: 69d763fc6d3a ("mm: pin address_space before dereferencing it while isolating an LRU page")
> Signed-off-by: Ivan Kalvachev <ikalvachev@xxxxxxxxx>
> ---
> mm/vmscan.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 9b697323a88c..83df26078d13 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1418,9 +1418,9 @@ int __isolate_lru_page(struct page *page, isolate_mode_t mode)
> return ret;
>
> mapping = page_mapping(page);
> - migrate_dirty = mapping && mapping->a_ops->migratepage;
> + migrate_dirty = mapping && !mapping->a_ops->migratepage;
> unlock_page(page);
> - if (!migrate_dirty)
> + if (migrate_dirty)
> return ret;
> }
> }
> --
> 2.17.1
This looks like yesterday's https://lkml.org/lkml/2018/5/30/1158
