Re: [PATCH v2] iomap: get/put the page in iomap_page_create/release()

[Piotr Jaroszynski <pjaroszynski@nvidia.com>]

On 12/3/18 3:22 PM, Andrew Morton wrote:
> On Thu, 15 Nov 2018 10:41:40 -0800 p.jaroszynski@gmail.com wrote:
> 
>> migrate_page_move_mapping() expects pages with private data set to have
>> a page_count elevated by 1. This is what used to happen for xfs through
>> the buffer_heads code before the switch to iomap in commit 82cb14175e7d
>> ("xfs: add support for sub-pagesize writeback without buffer_heads").
>> Not having the count elevated causes move_pages() to fail on memory
>> mapped files coming from xfs.
>>
>> Make iomap compatible with the migrate_page_move_mapping() assumption
>> by elevating the page count as part of iomap_page_create() and lowering
>> it in iomap_page_release().
> 
> What are the real-world end-user effects of this bug?

It causes the move_pages() syscall to misbehave on memory mapped files
from xfs. It does not not move any pages, which I suppose is "just" a
perf issue, but it also ends up returning a positive number which is out
of spec for the syscall. Talking to Michal Hocko, it sounds like
returning positive numbers might be a necessary update to move_pages()
anyway though, see [1].

I only hit this in tests that verify that move_pages() actually moved
the pages. The test also got confused by the positive return from
move_pages() (it got treated as a success as positive numbers were not
expected and not handled) making it a bit harder to track down what's
going on.

> 
> Is a -stable backport warranted?
> 

I would say yes, but this is my first kernel contribution so others
would be probably better judges of that.

[1] - https://lkml.kernel.org/r/20181116114955.GJ14706@dhcp22.suse.cz

Thanks,
Piotr