Re: [PATCH V1 1/9] mm/gup: repin_folio_unhugely

[David Hildenbrand <david@redhat.com>]

>>> reference fiddling is skipped.
>>
>> Please point me in mm/gup.c at that handling.
>>
>> IIRC is_zero_folio() does *not* include the huge zero page.
>>
>> Yes, we should likely be special-casing the huge zeropage in mm/gup.c, but it's not that easy because PINs can outlive MMs ... so *not* grabbing a reference could currently be harmful.
>>
>> But that has do be changed consistently, not with doing things here different compared to other gup.c functions.
> 

Sorry for the late reply, conferences ...

> folios_put() -> folios_put_refs() -> is_huge_zero_folio()

unpin_user_folio() -> gup_put_folio() will do

1) atomic_sub(refs, &folio->_pincount);
2) folio_put_refs(folio, refs);


What you cite above is for releasing huge folios that are mapped into 
user space.

> 
> I will run some tests with huge zero folios to verify the ref and pin
> counts behave correctly.

Whatever is pinned is supposed to be released via the unpin interface. 
We have to be consistent there, so this patch would be wrong (likely you 
never get the huge zero folio).

> 
>>>>>> +        return;
>>>>>> +    atomic_add(npin - 1, &folio->_refcount);
>>>>>> +    atomic_add(npin - 1, &folio->_pincount);
>>>>>> +}
>>>>>> +EXPORT_SYMBOL_GPL(repin_folio_unhugely);
>>>>
>>>> Can we ... find a better name? For example, it's "large" folio not "huge"...
>>>>
>>>> And repin is really misleading. We are simply adding more pins to an already pinned one ...
>>>
>>> Jason suggests a better name in the other thread.
>>
>> I would prefer something that simply adds more pins to an already pinned folio. Much easier to get.
> 
> How about folio_add_pins()?

Something like that, but more importantly, via which interface did we 
obtain the pins? That will make a difference and should be documented.

-- 
Cheers,

David / dhildenb