When handling a fault in an mshare range, redirect charges for page
tables and other allocations to the mshare owner rather than the
current task.

Signed-off-by: Anthony Yznaga <anthony.yznaga@oracle.com>
---
 mm/memory.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index 177eb53475cb..127db0b9932c 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -6468,9 +6468,17 @@ vm_fault_t handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
 	struct mm_struct *mm = vma->vm_mm;
 	vm_fault_t ret;
 	bool is_droppable;
+	bool is_mshare = mm_flags_test(MMF_MSHARE, mm);
+	struct mem_cgroup *mshare_memcg;
+	struct mem_cgroup *memcg;
 
 	__set_current_state(TASK_RUNNING);
 
+	if (unlikely(is_mshare)) {
+		mshare_memcg = get_mem_cgroup_from_mm(vma->vm_mm);
+		memcg = set_active_memcg(mshare_memcg);
+	}
+
 	ret = sanitize_fault_flags(vma, &flags);
 	if (ret)
 		goto out;
@@ -6530,6 +6538,11 @@ vm_fault_t handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
 out:
 	mm_account_fault(mm, regs, address, flags, ret);
 
+	if (unlikely(is_mshare)) {
+		set_active_memcg(memcg);
+		mem_cgroup_put(mshare_memcg);
+	}
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(handle_mm_fault);
-- 
2.47.1

On 20.08.25 03:04, Anthony Yznaga wrote:
> When handling a fault in an mshare range, redirect charges for page
> tables and other allocations to the mshare owner rather than the
> current task.
> 

That looks rather weird. I would have thought there would be an easy way 
to query the mshare owner for a given mshare mapping, and if the current 
MM corresponds to that owner you know that you are running in the owner 
context.

Of course, we could have a helper like is_mshare_owner(mapping, current) 
or sth like that.


-- 
Cheers

David / dhildenb

On 9/8/25 11:50 AM, David Hildenbrand wrote:
> On 20.08.25 03:04, Anthony Yznaga wrote:
>> When handling a fault in an mshare range, redirect charges for page
>> tables and other allocations to the mshare owner rather than the
>> current task.
>>
> 
> That looks rather weird. I would have thought there would be an easy way 
> to query the mshare owner for a given mshare mapping, and if the current 
> MM corresponds to that owner you know that you are running in the owner 
> context.
> 
> Of course, we could have a helper like is_mshare_owner(mapping, current) 
> or sth like that.
> 

I'm not quite following you. Charges for newly faulted pages will be 
automatically directed to the mshare owner because the mshare mm will 
have its mm_owner field pointing to the owner. On the other hand, 
allocations for page table pages are handled differently. 
GFP_PGTABLE_USER causes the accounting to go through 
__memcg_kmem_charge_page() which will charge them to the memcg for the 
current task unless unless current->active_memcg is set to point to 
another memcg.

On 08.09.25 21:21, Anthony Yznaga wrote:
> 
> 
> On 9/8/25 11:50 AM, David Hildenbrand wrote:
>> On 20.08.25 03:04, Anthony Yznaga wrote:
>>> When handling a fault in an mshare range, redirect charges for page
>>> tables and other allocations to the mshare owner rather than the
>>> current task.
>>>
>>
>> That looks rather weird. I would have thought there would be an easy way
>> to query the mshare owner for a given mshare mapping, and if the current
>> MM corresponds to that owner you know that you are running in the owner
>> context.
>>
>> Of course, we could have a helper like is_mshare_owner(mapping, current)
>> or sth like that.
>>
> 
> I'm not quite following you. Charges for newly faulted pages will be
> automatically directed to the mshare owner because the mshare mm will
> have its mm_owner field pointing to the owner. On the other hand,
> allocations for page table pages are handled differently.
> GFP_PGTABLE_USER causes the accounting to go through
> __memcg_kmem_charge_page() which will charge them to the memcg for the
> current task unless unless current->active_memcg is set to point to
> another memcg.

As a note, I think at some point we discussed re-routing page faults to 
the owner, so the owner can take care of all of that naturally. Is that 
what's happening here?


So, are we running into that code that we have current be another MM 
than vma->vm_mm?

Reminds me of: FOLL_REMOTE->FAULT_FLAG_REMOTE. But I guess, we don't 
take care of different accounting in that case.

Anyhow, what I meant is that you could just check whether you have a 
mshare VMA, and if so check if current is different to the mshare MM 
owner. So I don't immediately see why MMF_MSHARE is required.

-- 
Cheers

David / dhildenb

On 9/8/25 1:28 PM, David Hildenbrand wrote:
> On 08.09.25 21:21, Anthony Yznaga wrote:
>>
>>
>> On 9/8/25 11:50 AM, David Hildenbrand wrote:
>>> On 20.08.25 03:04, Anthony Yznaga wrote:
>>>> When handling a fault in an mshare range, redirect charges for page
>>>> tables and other allocations to the mshare owner rather than the
>>>> current task.
>>>>
>>>
>>> That looks rather weird. I would have thought there would be an easy way
>>> to query the mshare owner for a given mshare mapping, and if the current
>>> MM corresponds to that owner you know that you are running in the owner
>>> context.
>>>
>>> Of course, we could have a helper like is_mshare_owner(mapping, current)
>>> or sth like that.
>>>
>>
>> I'm not quite following you. Charges for newly faulted pages will be
>> automatically directed to the mshare owner because the mshare mm will
>> have its mm_owner field pointing to the owner. On the other hand,
>> allocations for page table pages are handled differently.
>> GFP_PGTABLE_USER causes the accounting to go through
>> __memcg_kmem_charge_page() which will charge them to the memcg for the
>> current task unless unless current->active_memcg is set to point to
>> another memcg.
> 
> As a note, I think at some point we discussed re-routing page faults to 
> the owner, so the owner can take care of all of that naturally. Is that 
> what's happening here?

The memcg charges are routed to the owner but otherwise the page faults 
are handled using the mshare mm.

> 
> 
> So, are we running into that code that we have current be another MM 
> than vma->vm_mm?

That will always be the case when handling a page fault in an mshare 
region. The fault handling code uses the msharefs VMA to find the actual 
VMA associated with the mshare mm. The actual VMA has its vm_mm pointing 
to the mshare mm, and it is the VMA passed to handle_mm_fault().

> 
> Reminds me of: FOLL_REMOTE->FAULT_FLAG_REMOTE. But I guess, we don't 
> take care of different accounting in that case.
> 
> Anyhow, what I meant is that you could just check whether you have a 
> mshare VMA, and if so check if current is different to the mshare MM 
> owner. So I don't immediately see why MMF_MSHARE is required.

It's because it is not the msharefs VMA that is passed to 
handle_mm_fault(). At one point I did have the code for setting 
active_memcg in do_user_addr_fault(), but it did not seem as clean.
The msharefs VMA is not passed handle_mm_fault() because protection 
checks done in the arch-specific handling need to be done against the 
actual VMA.