2022-09-08 01:14:45

by Alex Zhu (Kernel)

[permalink] [raw]
Subject: [PATCH] docs/mm: Improve grammar on mmu_notifier documentation

From: Alexander Zhu <[email protected]>

Improve grammar on mmu_notifier documentation.

Signed-off-by: Alexander Zhu <[email protected]>
---
Documentation/mm/mmu_notifier.rst | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/Documentation/mm/mmu_notifier.rst b/Documentation/mm/mmu_notifier.rst
index df5d7777fc6b..e22b591fc406 100644
--- a/Documentation/mm/mmu_notifier.rst
+++ b/Documentation/mm/mmu_notifier.rst
@@ -7,10 +7,11 @@ When clearing a pte/pmd we are given a choice to notify the event through
(notify version of \*_clear_flush call mmu_notifier_invalidate_range) under
the page table lock. But that notification is not necessary in all cases.

-For secondary TLB (non CPU TLB) like IOMMU TLB or device TLB (when device use
-thing like ATS/PASID to get the IOMMU to walk the CPU page table to access a
-process virtual address space). There is only 2 cases when you need to notify
-those secondary TLB while holding page table lock when clearing a pte/pmd:
+For secondary TLB (non CPU TLB) like IOMMU TLB or device TLB (when the device
+uses something like ATS/PASID to get the IOMMU to walk the CPU page table to
+access a process virtual address space). There are only 2 cases when you need
+to notify the secondary TLB while holding the page table lock when clearing
+a pte/pmd:

A) page backing address is free before mmu_notifier_invalidate_range_end()
B) a page table entry is updated to point to a new page (COW, write fault
@@ -27,13 +28,13 @@ happen:
- set page table entry to point to new page

If clearing the page table entry is not followed by a notify before setting
-the new pte/pmd value then you can break memory model like C11 or C++11 for
-the device.
+the new pte/pmd value then you can break the memory model like C11 or C++11
+for the device.

Consider the following scenario (device use a feature similar to ATS/PASID):

-Two address addrA and addrB such that \|addrA - addrB\| >= PAGE_SIZE we assume
-they are write protected for COW (other case of B apply too).
+Two addresses addrA and addrB such that \|addrA - addrB\| >= PAGE_SIZE we assume
+they are write protected for COW (other case of B applies as well).

::

@@ -86,14 +87,13 @@ they are write protected for COW (other case of B apply too).
CPU-thread-3 {}
DEV-thread-0 {read addrA from old page}
DEV-thread-2 {read addrB from new page}
-
-So here because at time N+2 the clear page table entry was not pair with a
-notification to invalidate the secondary TLB, the device see the new value for
-addrB before seeing the new value for addrA. This break total memory ordering
+Here because at time N+2 the clear page table entry was not paired with a
+notification to invalidate the secondary TLB, the device sees the new value for
+addrB before seeing the new value for addrA. This breaks total memory ordering
for the device.

When changing a pte to write protect or to point to a new write protected page
with same content (KSM) it is fine to delay the mmu_notifier_invalidate_range
call to mmu_notifier_invalidate_range_end() outside the page table lock. This
is true even if the thread doing the page table update is preempted right after
-releasing page table lock but before call mmu_notifier_invalidate_range_end().
+releasing the page table lock but before calling mmu_notifier_invalidate_range_end().
--
2.30.2


2022-09-10 23:59:02

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] docs/mm: Improve grammar on mmu_notifier documentation

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on lwn/docs-next]
[also build test WARNING on lwn-2.6/docs-next linus/master v6.0-rc4 next-20220909]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/alexlzhu-fb-com/docs-mm-Improve-grammar-on-mmu_notifier-documentation/20220908-082109
base: git://git.lwn.net/linux.git docs-next
reproduce:
# https://github.com/intel-lab-lkp/linux/commit/8d304367a02999d5fc990a6845ffb2c2cdf4f058
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review alexlzhu-fb-com/docs-mm-Improve-grammar-on-mmu_notifier-documentation/20220908-082109
git checkout 8d304367a02999d5fc990a6845ffb2c2cdf4f058
make menuconfig
# enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS
make htmldocs

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> Documentation/mm/mmu_notifier.rst:90: WARNING: Literal block ends without a blank line; unexpected unindent.

vim +90 Documentation/mm/mmu_notifier.rst

40
41 [Time N] --------------------------------------------------------------------
42 CPU-thread-0 {try to write to addrA}
43 CPU-thread-1 {try to write to addrB}
44 CPU-thread-2 {}
45 CPU-thread-3 {}
46 DEV-thread-0 {read addrA and populate device TLB}
47 DEV-thread-2 {read addrB and populate device TLB}
48 [Time N+1] ------------------------------------------------------------------
49 CPU-thread-0 {COW_step0: {mmu_notifier_invalidate_range_start(addrA)}}
50 CPU-thread-1 {COW_step0: {mmu_notifier_invalidate_range_start(addrB)}}
51 CPU-thread-2 {}
52 CPU-thread-3 {}
53 DEV-thread-0 {}
54 DEV-thread-2 {}
55 [Time N+2] ------------------------------------------------------------------
56 CPU-thread-0 {COW_step1: {update page table to point to new page for addrA}}
57 CPU-thread-1 {COW_step1: {update page table to point to new page for addrB}}
58 CPU-thread-2 {}
59 CPU-thread-3 {}
60 DEV-thread-0 {}
61 DEV-thread-2 {}
62 [Time N+3] ------------------------------------------------------------------
63 CPU-thread-0 {preempted}
64 CPU-thread-1 {preempted}
65 CPU-thread-2 {write to addrA which is a write to new page}
66 CPU-thread-3 {}
67 DEV-thread-0 {}
68 DEV-thread-2 {}
69 [Time N+3] ------------------------------------------------------------------
70 CPU-thread-0 {preempted}
71 CPU-thread-1 {preempted}
72 CPU-thread-2 {}
73 CPU-thread-3 {write to addrB which is a write to new page}
74 DEV-thread-0 {}
75 DEV-thread-2 {}
76 [Time N+4] ------------------------------------------------------------------
77 CPU-thread-0 {preempted}
78 CPU-thread-1 {COW_step3: {mmu_notifier_invalidate_range_end(addrB)}}
79 CPU-thread-2 {}
80 CPU-thread-3 {}
81 DEV-thread-0 {}
82 DEV-thread-2 {}
83 [Time N+5] ------------------------------------------------------------------
84 CPU-thread-0 {preempted}
85 CPU-thread-1 {}
86 CPU-thread-2 {}
87 CPU-thread-3 {}
88 DEV-thread-0 {read addrA from old page}
89 DEV-thread-2 {read addrB from new page}
> 90 Here because at time N+2 the clear page table entry was not paired with a
91 notification to invalidate the secondary TLB, the device sees the new value for
92 addrB before seeing the new value for addrA. This breaks total memory ordering
93 for the device.
94

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (3.98 kB)
config (39.19 kB)
Download all attachments

2022-09-11 03:27:02

by Bagas Sanjaya

[permalink] [raw]
Subject: Re: [PATCH] docs/mm: Improve grammar on mmu_notifier documentation

On Wed, Sep 07, 2022 at 05:19:48PM -0700, [email protected] wrote:
> @@ -86,14 +87,13 @@ they are write protected for COW (other case of B apply too).
> CPU-thread-3 {}
> DEV-thread-0 {read addrA from old page}
> DEV-thread-2 {read addrB from new page}
> -
> -So here because at time N+2 the clear page table entry was not pair with a
> -notification to invalidate the secondary TLB, the device see the new value for
> -addrB before seeing the new value for addrA. This break total memory ordering
> +Here because at time N+2 the clear page table entry was not paired with a
> +notification to invalidate the secondary TLB, the device sees the new value for
> +addrB before seeing the new value for addrA. This breaks total memory ordering

You remove the blank line separator between the code block and
paragraph, which kernel test robot (and Sphinx) complains; so you need
to keep it:

---- >8 ----
diff --git a/Documentation/mm/mmu_notifier.rst b/Documentation/mm/mmu_notifier.rst
index e22b591fc4061f..751b6eaf456e52 100644
--- a/Documentation/mm/mmu_notifier.rst
+++ b/Documentation/mm/mmu_notifier.rst
@@ -87,6 +87,7 @@ they are write protected for COW (other case of B applies as well).
CPU-thread-3 {}
DEV-thread-0 {read addrA from old page}
DEV-thread-2 {read addrB from new page}
+
Here because at time N+2 the clear page table entry was not paired with a
notification to invalidate the secondary TLB, the device sees the new value for
addrB before seeing the new value for addrA. This breaks total memory ordering

Thanks.

--
An old man doll... just what I always wanted! - Clara


Attachments:
(No filename) (1.61 kB)
signature.asc (235.00 B)
Download all attachments