Re: [PATCH v2 07/11] mm/hmm: add default fault flags to avoid the need to pre-fill pfns arrays.

[John Hubbard]

On 3/28/19 6:17 PM, Jerome Glisse wrote:
> On Thu, Mar 28, 2019 at 09:42:31AM -0700, Ira Weiny wrote:
>> On Thu, Mar 28, 2019 at 04:28:47PM -0700, John Hubbard wrote:
>>> On 3/28/19 4:21 PM, Jerome Glisse wrote:
>>>> On Thu, Mar 28, 2019 at 03:40:42PM -0700, John Hubbard wrote:
>>>>> On 3/28/19 3:31 PM, Jerome Glisse wrote:
>>>>>> On Thu, Mar 28, 2019 at 03:19:06PM -0700, John Hubbard wrote:
>>>>>>> On 3/28/19 3:12 PM, Jerome Glisse wrote:
>>>>>>>> On Thu, Mar 28, 2019 at 02:59:50PM -0700, John Hubbard wrote:
>>>>>>>>> On 3/25/19 7:40 AM, jglisse@xxxxxxxxxx wrote:
>>>>>>>>>> From: JÃrÃme Glisse <jglisse@xxxxxxxxxx>
>>> [...]
>> Indeed I did not realize there is an hmm "pfn" until I saw this function:
>>
>> /*
>>  * hmm_pfn_from_pfn() - create a valid HMM pfn value from pfn
>>  * @range: range use to encode HMM pfn value
>>  * @pfn: pfn value for which to create the HMM pfn
>>  * Returns: valid HMM pfn for the pfn
>>  */
>> static inline uint64_t hmm_pfn_from_pfn(const struct hmm_range *range,
>>                                         unsigned long pfn)
>>
>> So should this patch contain some sort of helper like this... maybe?
>>
>> I'm assuming the "hmm_pfn" being returned above is the device pfn being
>> discussed here?
>>
>> I'm also thinking calling it pfn is confusing.  I'm not advocating a new type
>> but calling the "device pfn's" "hmm_pfn" or "device_pfn" seems like it would
>> have shortened the discussion here.
>>
> 
> That helper is also use today by nouveau so changing that name is not that
> easy it does require the multi-release dance. So i am not sure how much
> value there is in a name change.
> 

Once the dust settles, I would expect that a name change for this could go
via Andrew's tree, right? It seems incredible to claim that we've built something
that effectively does not allow any minor changes!

I do think it's worth some *minor* trouble to improve the name, assuming that we
can do it in a simple patch, rather than some huge maintainer-level effort.

This field name not a large thing, but the cumulative effect of having a number of
naming glitches within HMM is significant. The size and complexity of HMM has
always made it hard to attract code reviewers, so let's improve what we can, to
counteract that.

thanks,
-- 
John Hubbard
NVIDIA