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.
