On 3/13/2024 8:51 PM, Matthew Wilcox wrote:
On Wed, Mar 13, 2024 at 11:11:04AM -0700, Jane Chu wrote:
On 3/12/2024 8:23 PM, Matthew Wilcox wrote:
I don't understand what you're saying is wrong with this patch.
All I'm doing (apart from the renaming of ksm_addr to addr) is moving
the vma_pgoff_address() call from __add_to_kill() to its caller.
Is there some reason this doesn't work?
Sorry for not being clear. What I meant to say is that, before the patch,
page_address_in_vma(p, vma) is the means for determining tk->addr, except
for fsdax when filesystems made a claim of wanting to participate in the UE
handling, in that case, vma_pgoff_address(fsdax_pgoff, 1, vma) is used
instead. The difference between the two means is that, although the former
eventually calls the latter _if_ vma->vm_file->f_mapping exists and is
stable, what I am unclear from Shiyang Ruan's earlier patch is that, he
seems to be concerning a case where f_mapping is not reliable, hence his
patch went straight to call vma_pgoff_address(fsdax_pgoff, 1, vma), and on
top of that, providing nr=1 to ignore the address wrap around case.
So I don't know whether removing the fsdax special case is okay.
I don't think I'm removing the fsdax special case, just moving it.
If I subtract out the renaming from this patch, it looks like:
tk->addr = addr ? addr : page_address_in_vma(p, vma);
- if (is_zone_device_page(p)) {
- if (fsdax_pgoff != FSDAX_INVALID_PGOFF)
- tk->addr = vma_pgoff_address(fsdax_pgoff, 1, vma);
+ if (is_zone_device_page(p)) {
...
{
- __add_to_kill(tsk, p, vma, to_kill, 0, pgoff);
+ unsigned long addr = vma_pgoff_address(pgoff, 1, vma);
+ __add_to_kill(tsk, p, vma, to_kill, addr);
}
So instead of passing in '0' as the addr from add_to_kill_fsdax(),
we do the address lookup in add_to_kill_fsdax() and pass it in.
That means we don't call page_address_in_vma() for DAX because
addr is not 0.
You're right! I overlooked the addr being passed in in fsdax case is
different.
Thanks for taking the time explaining to me.
Reviewed-by: Jane Chu <jane.chu@xxxxxxxxxx>
-jane
