Re: [PATCH v2 1/1] fs/splice: add missing callback for inaccessible pages

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 05.05.20 16:33, Ulrich Weigand wrote:
> On Tue, May 05, 2020 at 04:03:00PM +0200, Christian Borntraeger wrote:
>>> Just looked at 
>>> commit 88b1a17dfc3ed7728316478fae0f5ad508f50397  mm: add 'try_get_page()' helper function
>>>
>>> which says:
>>>     Also like 'get_page()', you can't use this function unless you already
>>>     had a reference to the page.  The intent is that you can use this
>>>     exactly like get_page(), but in situations where you want to limit the
>>>     maximum reference count.
>>>     
>>>     The code currently does an unconditional WARN_ON_ONCE() if we ever hit
>>>     the reference count issues (either zero or negative), as a notification
>>>     that the conditional non-increment actually happened.
>>>
>>> If try_get_page must not be called with an existing reference, that means
>> 		s/not//
>>> that when we call it the page reference is already higher and our freeze
>>> will never succeed. That would imply that we cannot trigger this. No?
> 
> Well, my understanding is that the "existing" reference may be one of the
> references that is expected by our freeze code, in particular in gup the
> existing reference is simply the one from the pte.  So in this case our
> freeze *would* succeeed.

If thats the case then "<0" looks indeed better than "<=0" for the check.
I think try_get_page was never meant to exclude a parallel page_ref_freeze/unfreeze.