From: Alistair Popple <apopple@nvidia•com>
To: "Huang, Ying" <ying.huang@intel•com>
Cc: "Sierra Guiza, Alejandro \(Alex\)" <alex.sierra@amd•com>,
Ralph Campbell <rcampbell@nvidia•com>,
Lyude Paul <lyude@redhat•com>, Karol Herbst <kherbst@redhat•com>,
David Hildenbrand <david@redhat•com>,
Nadav Amit <nadav.amit@gmail•com>,
Felix Kuehling <Felix.Kuehling@amd•com>,
linuxppc-dev@lists•ozlabs.org,
LKML <linux-kernel@vger•kernel.org>, Peter Xu <peterx@redhat•com>,
Linux MM <linux-mm@kvack•org>,
Logan Gunthorpe <logang@deltatee•com>,
Matthew Wilcox <willy@infradead•org>,
Jason Gunthorpe <jgg@nvidia•com>,
John Hubbard <jhubbard@nvidia•com>,
stable@vger•kernel.org, Andrew Morton <akpm@linux-foundation•org>,
huang ying <huang.ying.caritas@gmail•com>,
Ben Skeggs <bskeggs@redhat•com>
Subject: Re: [PATCH v2 1/2] mm/migrate_device.c: Copy pte dirty bit to page
Date: Wed, 24 Aug 2022 11:56:25 +1000 [thread overview]
Message-ID: <87czcqiecd.fsf@nvdebian.thelocal> (raw)
In-Reply-To: <874jy9aqts.fsf@yhuang6-desk2.ccr.corp.intel.com>
"Huang, Ying" <ying.huang@intel•com> writes:
> Peter Xu <peterx@redhat•com> writes:
>
>> On Thu, Aug 18, 2022 at 02:34:45PM +0800, Huang, Ying wrote:
>>> > In this specific case, the only way to do safe tlb batching in my mind is:
>>> >
>>> > pte_offset_map_lock();
>>> > arch_enter_lazy_mmu_mode();
>>> > // If any pending tlb, do it now
>>> > if (mm_tlb_flush_pending())
>>> > flush_tlb_range(vma, start, end);
>>> > else
>>> > flush_tlb_batched_pending();
>>>
>>> I don't think we need the above 4 lines. Because we will flush TLB
>>> before we access the pages.
I agree. For migration the TLB flush is only important if the PTE is
present, and in that case we do a TLB flush anyway.
>> Could you elaborate?
>
> As you have said below, we don't use non-present PTEs and flush present
> PTEs before we access the pages.
>
>>> Can you find any issue if we don't use the above 4 lines?
>>
>> It seems okay to me to leave stall tlb at least within the scope of this
>> function. It only collects present ptes and flush propoerly for them. I
>> don't quickly see any other implications to other not touched ptes - unlike
>> e.g. mprotect(), there's a strong barrier of not allowing further write
>> after mprotect() returns.
>
> Yes. I think so too.
>
>> Still I don't know whether there'll be any side effect of having stall tlbs
>> in !present ptes because I'm not familiar enough with the private dev swap
>> migration code. But I think having them will be safe, even if redundant.
What side-effect were you thinking of? I don't see any issue with not
TLB flushing stale device-private TLBs prior to the migration because
they're not accessible anyway and shouldn't be in any TLB.
> I don't think it's a good idea to be redundant. That may hide the real
> issue.
>
> Best Regards,
> Huang, Ying
Thanks all for the discussion. Having done some more reading I agree
that it's safe to assume HW dirty bits are write-through, so will remove
the ptep_clear_flush() and use ptep_get_and_clear() instead. Will split
out the TLB flushing fix into a separate patch in this series.
- Alistair
next prev parent reply other threads:[~2022-08-24 2:21 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-16 7:39 [PATCH v2 1/2] mm/migrate_device.c: Copy pte dirty bit to page Alistair Popple
2022-08-16 7:39 ` [PATCH v2 2/2] selftests/hmm-tests: Add test for dirty bits Alistair Popple
2022-08-16 8:10 ` [PATCH v2 1/2] mm/migrate_device.c: Copy pte dirty bit to page huang ying
2022-08-16 20:35 ` Peter Xu
2022-08-17 1:49 ` Alistair Popple
2022-08-17 2:45 ` Peter Xu
2022-08-17 5:41 ` Alistair Popple
2022-08-17 7:17 ` Huang, Ying
2022-08-17 9:41 ` Nadav Amit
2022-08-17 19:27 ` Peter Xu
2022-08-18 6:34 ` Huang, Ying
2022-08-18 14:44 ` Peter Xu
2022-08-19 2:51 ` Huang, Ying
2022-08-24 1:56 ` Alistair Popple [this message]
2022-08-24 20:25 ` Peter Xu
2022-08-24 20:48 ` Peter Xu
2022-08-25 0:42 ` Alistair Popple
2022-08-25 1:24 ` Alistair Popple
2022-08-25 15:04 ` Peter Xu
2022-08-25 22:09 ` Alistair Popple
2022-08-25 23:36 ` Peter Xu
2022-08-25 14:40 ` Peter Xu
2022-08-18 5:59 ` Huang, Ying
2022-08-17 19:07 ` Peter Xu
2022-08-17 1:38 ` Alistair Popple
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87czcqiecd.fsf@nvdebian.thelocal \
--to=apopple@nvidia$(echo .)com \
--cc=Felix.Kuehling@amd$(echo .)com \
--cc=akpm@linux-foundation$(echo .)org \
--cc=alex.sierra@amd$(echo .)com \
--cc=bskeggs@redhat$(echo .)com \
--cc=david@redhat$(echo .)com \
--cc=huang.ying.caritas@gmail$(echo .)com \
--cc=jgg@nvidia$(echo .)com \
--cc=jhubbard@nvidia$(echo .)com \
--cc=kherbst@redhat$(echo .)com \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-mm@kvack$(echo .)org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=logang@deltatee$(echo .)com \
--cc=lyude@redhat$(echo .)com \
--cc=nadav.amit@gmail$(echo .)com \
--cc=peterx@redhat$(echo .)com \
--cc=rcampbell@nvidia$(echo .)com \
--cc=stable@vger$(echo .)kernel.org \
--cc=willy@infradead$(echo .)org \
--cc=ying.huang@intel$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox