Re: [PATCH v5 8/8] device-dax: compound devmap support

[Joao Martins <joao.m.martins@oracle.com>]

On 11/19/21 16:55, Jason Gunthorpe wrote:
> On Fri, Nov 19, 2021 at 04:12:18PM +0000, Joao Martins wrote:
> 
>>> Dan, any thoughts (see also below) ? You probably hold all that
>>> history since its inception on commit 2232c6382a4 ("device-dax: Enable page_mapping()")
>>> and commit 35de299547d1 ("device-dax: Set page->index").
>>>
>> Below is what I have staged so far as a percursor patch (see below scissors mark).
>>
>> It also lets me simplify compound page case for __dax_set_mapping() in this patch,
>> like below diff.
>>
>> But I still wonder whether this ordering adjustment of @mapping setting is best placed
>> as a percursor patch whenever pgmap/page refcount changes happen. Anyways it's just a
>> thought.
> 
> naively I would have thought you'd set the mapping on all pages when
> you create the address_space and allocate pages into it. 

Today in fsdax/device-dax (hugetlb too) this is set on fault and set once
only (as you say) on the mapped pages. fsdax WARN_ON() you when you clearing
a page mapping that was not set to the expected address_space (similar to
what I did here)

Similar to what I do in the previous snippet I pasted. Except that maybe
I should just clear the mapping rather than clearing if f_mapping is the
expected one.

> AFAIK devmap
> assigns all pages to a single address_space, so shouldn't the mapping
> just be done once?
Isn't it a bit more efficient that you set only when you try to map a page?

I take it that's why it is being done this way.

	Joao