Re: [PATCH v2 2/2] core-api/memory-hotplug.rst: divide Locking Internal section by different locks
On Thu, Dec 06, 2018 at 08:26:22AM +0800, Wei Yang wrote: > Currently locking for memory hotplug is a little complicated. > > Generally speaking, we leverage the two global lock: > > * device_hotplug_lock > * mem_hotplug_lock > > to serialise the process. > > While for the long term, we are willing to have more fine-grained lock > to provide higher scalability. > > This patch divides Locking Internal section based on these two global > locks to help readers to understand it. Also it adds some new finding to > enrich it. > > [David: words arrangement] > > Signed-off-by: Wei Yang Reviewd-by: Mike Rapoport > --- > v2: adjustment based on David and Mike comment > --- > Documentation/core-api/memory-hotplug.rst | 27 --- > 1 file changed, 24 insertions(+), 3 deletions(-) > > diff --git a/Documentation/core-api/memory-hotplug.rst > b/Documentation/core-api/memory-hotplug.rst > index de7467e48067..51d477ad4b80 100644 > --- a/Documentation/core-api/memory-hotplug.rst > +++ b/Documentation/core-api/memory-hotplug.rst > @@ -89,6 +89,20 @@ NOTIFY_STOP stops further processing of the notification > queue. > Locking Internals > = > > +In addition to fine grained locks like pgdat_resize_lock, there are three > locks > +involved > + > +- device_hotplug_lock > +- mem_hotplug_lock > +- device_lock > + > +Currently, they are twisted together for all kinds of reasons. The following > +part is divided into device_hotplug_lock and mem_hotplug_lock parts > +respectively to describe those tricky situations. > + > +device_hotplug_lock > +- > + > When adding/removing memory that uses memory block devices (i.e. ordinary > RAM), > the device_hotplug_lock should be held to: > > @@ -111,13 +125,20 @@ As the device is visible to user space before taking > the device_lock(), this > can result in a lock inversion. > > onlining/offlining of memory should be done via device_online()/ > -device_offline() - to make sure it is properly synchronized to actions > -via sysfs. Holding device_hotplug_lock is advised (to e.g. protect > online_type) > +device_offline() - to make sure it is properly synchronized to actions via > +sysfs. Even if mem_hotplug_lock is used to protect the process, because of > the > +lock inversion described above, holding device_hotplug_lock is still advised > +(to e.g. protect online_type) > + > +mem_hotplug_lock > +- > > When adding/removing/onlining/offlining memory or adding/removing > heterogeneous/device memory, we should always hold the mem_hotplug_lock in > write mode to serialise memory hotplug (e.g. access to global/zone > -variables). > +variables). Currently, we take advantage of this to serialise sparsemem's > +mem_section handling in sparse_add_one_section() and > +sparse_remove_one_section(). > > In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read > mode allows for a quite efficient get_online_mems/put_online_mems > -- > 2.15.1 > -- Sincerely yours, Mike.
[PATCH v2 1/2] admin-guide/memory-hotplug.rst: remove locking internal part from admin-guide
Locking Internal section exists in core-api documentation, which is more suitable for this. This patch removes the duplication part here. Signed-off-by: Wei Yang Reviewed-by: David Hildenbrand Acked-by: Mike Rapoport --- Documentation/admin-guide/mm/memory-hotplug.rst | 40 - 1 file changed, 40 deletions(-) diff --git a/Documentation/admin-guide/mm/memory-hotplug.rst b/Documentation/admin-guide/mm/memory-hotplug.rst index 5c4432c96c4b..241f4ce1e387 100644 --- a/Documentation/admin-guide/mm/memory-hotplug.rst +++ b/Documentation/admin-guide/mm/memory-hotplug.rst @@ -392,46 +392,6 @@ Need more implementation yet - Notification completion of remove works by OS to firmware. - Guard from remove if not yet. - -Locking Internals -= - -When adding/removing memory that uses memory block devices (i.e. ordinary RAM), -the device_hotplug_lock should be held to: - -- synchronize against online/offline requests (e.g. via sysfs). This way, memory - block devices can only be accessed (.online/.state attributes) by user - space once memory has been fully added. And when removing memory, we - know nobody is in critical sections. -- synchronize against CPU hotplug and similar (e.g. relevant for ACPI and PPC) - -Especially, there is a possible lock inversion that is avoided using -device_hotplug_lock when adding memory and user space tries to online that -memory faster than expected: - -- device_online() will first take the device_lock(), followed by - mem_hotplug_lock -- add_memory_resource() will first take the mem_hotplug_lock, followed by - the device_lock() (while creating the devices, during bus_add_device()). - -As the device is visible to user space before taking the device_lock(), this -can result in a lock inversion. - -onlining/offlining of memory should be done via device_online()/ -device_offline() - to make sure it is properly synchronized to actions -via sysfs. Holding device_hotplug_lock is advised (to e.g. protect online_type) - -When adding/removing/onlining/offlining memory or adding/removing -heterogeneous/device memory, we should always hold the mem_hotplug_lock in -write mode to serialise memory hotplug (e.g. access to global/zone -variables). - -In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read -mode allows for a quite efficient get_online_mems/put_online_mems -implementation, so code accessing memory can protect from that memory -vanishing. - - Future Work === -- 2.15.1
[PATCH v2 2/2] core-api/memory-hotplug.rst: divide Locking Internal section by different locks
Currently locking for memory hotplug is a little complicated. Generally speaking, we leverage the two global lock: * device_hotplug_lock * mem_hotplug_lock to serialise the process. While for the long term, we are willing to have more fine-grained lock to provide higher scalability. This patch divides Locking Internal section based on these two global locks to help readers to understand it. Also it adds some new finding to enrich it. [David: words arrangement] Signed-off-by: Wei Yang --- v2: adjustment based on David and Mike comment --- Documentation/core-api/memory-hotplug.rst | 27 --- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/Documentation/core-api/memory-hotplug.rst b/Documentation/core-api/memory-hotplug.rst index de7467e48067..51d477ad4b80 100644 --- a/Documentation/core-api/memory-hotplug.rst +++ b/Documentation/core-api/memory-hotplug.rst @@ -89,6 +89,20 @@ NOTIFY_STOP stops further processing of the notification queue. Locking Internals = +In addition to fine grained locks like pgdat_resize_lock, there are three locks +involved + +- device_hotplug_lock +- mem_hotplug_lock +- device_lock + +Currently, they are twisted together for all kinds of reasons. The following +part is divided into device_hotplug_lock and mem_hotplug_lock parts +respectively to describe those tricky situations. + +device_hotplug_lock +- + When adding/removing memory that uses memory block devices (i.e. ordinary RAM), the device_hotplug_lock should be held to: @@ -111,13 +125,20 @@ As the device is visible to user space before taking the device_lock(), this can result in a lock inversion. onlining/offlining of memory should be done via device_online()/ -device_offline() - to make sure it is properly synchronized to actions -via sysfs. Holding device_hotplug_lock is advised (to e.g. protect online_type) +device_offline() - to make sure it is properly synchronized to actions via +sysfs. Even if mem_hotplug_lock is used to protect the process, because of the +lock inversion described above, holding device_hotplug_lock is still advised +(to e.g. protect online_type) + +mem_hotplug_lock +- When adding/removing/onlining/offlining memory or adding/removing heterogeneous/device memory, we should always hold the mem_hotplug_lock in write mode to serialise memory hotplug (e.g. access to global/zone -variables). +variables). Currently, we take advantage of this to serialise sparsemem's +mem_section handling in sparse_add_one_section() and +sparse_remove_one_section(). In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read mode allows for a quite efficient get_online_mems/put_online_mems -- 2.15.1
Re: [PATCH 2/2] core-api/memory-hotplug.rst: divide Locking Internal section by different locks
On Wed, Dec 05, 2018 at 01:13:10PM +0100, Michal Hocko wrote: >On Wed 05-12-18 10:34:26, Wei Yang wrote: >> Currently locking for memory hotplug is a little complicated. >> >> Generally speaking, we leverage the two global lock: >> >> * device_hotplug_lock >> * mem_hotplug_lock >> >> to serialise the process. >> >> While for the long term, we are willing to have more fine-grained lock >> to provide higher scalability. >> >> This patch divides Locking Internal section based on these two global >> locks to help readers to understand it. Also it adds some new finding to >> enrich it. >> >> [David: words arrangement] >> >> Signed-off-by: Wei Yang > >For a love of mine I cannot find the locking description by Oscar. Maybe >it never existed and I just made it up ;) But if it is not imaginary >then my recollection is that it was much more comprehensive. If not then >even this is a good start. Thanks. If Oscar has already has some work on it, this could be a complement to his work :-) > >> --- >> Documentation/core-api/memory-hotplug.rst | 27 --- >> 1 file changed, 24 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/core-api/memory-hotplug.rst >> b/Documentation/core-api/memory-hotplug.rst >> index de7467e48067..95662b283328 100644 >> --- a/Documentation/core-api/memory-hotplug.rst >> +++ b/Documentation/core-api/memory-hotplug.rst >> @@ -89,6 +89,20 @@ NOTIFY_STOP stops further processing of the notification >> queue. >> Locking Internals >> = >> >> +There are three locks involved in memory-hotplug, two global lock and one >> local >> +lock: >> + >> +- device_hotplug_lock >> +- mem_hotplug_lock >> +- device_lock >> + >> +Currently, they are twisted together for all kinds of reasons. The following >> +part is divided into device_hotplug_lock and mem_hotplug_lock parts >> +respectively to describe those tricky situations. >> + >> +device_hotplug_lock >> +- >> + >> When adding/removing memory that uses memory block devices (i.e. ordinary >> RAM), >> the device_hotplug_lock should be held to: >> >> @@ -111,13 +125,20 @@ As the device is visible to user space before taking >> the device_lock(), this >> can result in a lock inversion. >> >> onlining/offlining of memory should be done via device_online()/ >> -device_offline() - to make sure it is properly synchronized to actions >> -via sysfs. Holding device_hotplug_lock is advised (to e.g. protect >> online_type) >> +device_offline() - to make sure it is properly synchronized to actions via >> +sysfs. Even mem_hotplug_lock is used to protect the process, because of the >> +lock inversion described above, holding device_hotplug_lock is still advised >> +(to e.g. protect online_type) >> + >> +mem_hotplug_lock >> +- >> >> When adding/removing/onlining/offlining memory or adding/removing >> heterogeneous/device memory, we should always hold the mem_hotplug_lock in >> write mode to serialise memory hotplug (e.g. access to global/zone >> -variables). >> +variables). Currently, we take advantage of this to serialise sparsemem's >> +mem_section handling in sparse_add_one_section() and >> +sparse_remove_one_section(). >> >> In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read >> mode allows for a quite efficient get_online_mems/put_online_mems >> -- >> 2.15.1 >> > >-- >Michal Hocko >SUSE Labs -- Wei Yang Help you, Help me
Re: [PATCH 2/2] core-api/memory-hotplug.rst: divide Locking Internal section by different locks
On Wed 05-12-18 10:34:26, Wei Yang wrote: > Currently locking for memory hotplug is a little complicated. > > Generally speaking, we leverage the two global lock: > > * device_hotplug_lock > * mem_hotplug_lock > > to serialise the process. > > While for the long term, we are willing to have more fine-grained lock > to provide higher scalability. > > This patch divides Locking Internal section based on these two global > locks to help readers to understand it. Also it adds some new finding to > enrich it. > > [David: words arrangement] > > Signed-off-by: Wei Yang For a love of mine I cannot find the locking description by Oscar. Maybe it never existed and I just made it up ;) But if it is not imaginary then my recollection is that it was much more comprehensive. If not then even this is a good start. > --- > Documentation/core-api/memory-hotplug.rst | 27 --- > 1 file changed, 24 insertions(+), 3 deletions(-) > > diff --git a/Documentation/core-api/memory-hotplug.rst > b/Documentation/core-api/memory-hotplug.rst > index de7467e48067..95662b283328 100644 > --- a/Documentation/core-api/memory-hotplug.rst > +++ b/Documentation/core-api/memory-hotplug.rst > @@ -89,6 +89,20 @@ NOTIFY_STOP stops further processing of the notification > queue. > Locking Internals > = > > +There are three locks involved in memory-hotplug, two global lock and one > local > +lock: > + > +- device_hotplug_lock > +- mem_hotplug_lock > +- device_lock > + > +Currently, they are twisted together for all kinds of reasons. The following > +part is divided into device_hotplug_lock and mem_hotplug_lock parts > +respectively to describe those tricky situations. > + > +device_hotplug_lock > +- > + > When adding/removing memory that uses memory block devices (i.e. ordinary > RAM), > the device_hotplug_lock should be held to: > > @@ -111,13 +125,20 @@ As the device is visible to user space before taking > the device_lock(), this > can result in a lock inversion. > > onlining/offlining of memory should be done via device_online()/ > -device_offline() - to make sure it is properly synchronized to actions > -via sysfs. Holding device_hotplug_lock is advised (to e.g. protect > online_type) > +device_offline() - to make sure it is properly synchronized to actions via > +sysfs. Even mem_hotplug_lock is used to protect the process, because of the > +lock inversion described above, holding device_hotplug_lock is still advised > +(to e.g. protect online_type) > + > +mem_hotplug_lock > +- > > When adding/removing/onlining/offlining memory or adding/removing > heterogeneous/device memory, we should always hold the mem_hotplug_lock in > write mode to serialise memory hotplug (e.g. access to global/zone > -variables). > +variables). Currently, we take advantage of this to serialise sparsemem's > +mem_section handling in sparse_add_one_section() and > +sparse_remove_one_section(). > > In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read > mode allows for a quite efficient get_online_mems/put_online_mems > -- > 2.15.1 > -- Michal Hocko SUSE Labs
Re: [PATCH 1/2] admin-guide/memory-hotplug.rst: remove locking internal part from admin-guide
On Wed 05-12-18 10:34:25, Wei Yang wrote: > Locking Internal section exists in core-api documentation, which is more > suitable for this. > > This patch removes the duplication part here. > > Signed-off-by: Wei Yang Yes this doesn't really make any sense in an admin guide. It is a pure implementation detail nobody should be relying on. -- Michal Hocko SUSE Labs
[PATCH v1 8/9] drm/doc: Add initial komeda driver documentation
Signed-off-by: James (Qian) Wang
---
Documentation/gpu/drivers.rst| 1 +
Documentation/gpu/komeda-kms.rst | 483 +++
2 files changed, 484 insertions(+)
create mode 100644 Documentation/gpu/komeda-kms.rst
diff --git a/Documentation/gpu/drivers.rst b/Documentation/gpu/drivers.rst
index 7c1672118a73..978e6da9bbff 100644
--- a/Documentation/gpu/drivers.rst
+++ b/Documentation/gpu/drivers.rst
@@ -17,6 +17,7 @@ GPU Driver Documentation
vkms
bridge/dw-hdmi
xen-front
+ komeda-kms
.. only:: subproject and html
diff --git a/Documentation/gpu/komeda-kms.rst b/Documentation/gpu/komeda-kms.rst
new file mode 100644
index ..8af925ca0869
--- /dev/null
+++ b/Documentation/gpu/komeda-kms.rst
@@ -0,0 +1,483 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==
+ drm/komeda ARM display driver
+==
+
+The drm/komeda driver supports for the ARM display processor D71 and later
+IPs, this document is for giving a brief overview of driver design: how it
+works and why design it like that.
+
+Overview of D71 like display IPs
+
+
+From D71, Arm display IP begins to adopt a flexible and modularized
+architecture. A display pipeline is made up of multiple individual and
+functional pipeline stages called components, and every component has some
+specific capabilities that can give the flowed pipeline pixel data a
+particular processing.
+
+Typical D71 components:
+
+Layer
+-
+Layer is the first pipeline stage, which is for preparing the pixel data
+for the next stage. It fetches the pixel from memory, decodes it if it's
+AFBC, rotates the source image, unpacks or converts YUV pixels to the device
+internal RGB pixels, then adjust the color_space of pixels if need.
+
+Scaler
+--
+As its name, scaler is taking responsability for scaling, and D71 also
+supports image enhancements by scaler.
+The usage of scaler is very flexible and can be connected to layer output
+for layer scaling, or connected to compositor and scale the whole display
+frame and then feed the output data into wb_layer which will then write it
+into memory.
+
+Compositor (compiz)
+---
+Compositor is for blending multiple layers or pixel data flows into one
+single display frame, and its output frame can be fed into post image
+processor and then on the monitor or fed into wb_layer and written to memory
+at the same time. And user also can insert a scaler between compositor and
+wb_layer to down scale the display frame first and then writing to memory.
+
+Writeback Layer (wb_layer)
+--
+Writeback layer do the opposite things of Layer, Which connect to compiz and
try
+to write the composition result to memory.
+
+Post image processor (improc)
+-
+Post image processor is for adjusting frame data like gamma and color space
+to fit the requirements of the monitor.
+
+Timing controller (timing_ctrlr)
+
+Final stage of display pipeline, Timing controller is not for the pixel
+handling, but only for controlling the display timing.
+
+Merger
+--
+D71 scaler mostly only has half the horizontal input/output capabilities
compare
+with Layer, Like if Layer supports 4K input size, the scaler only can supports
+2K input/output in some time. To achieve the fully frame scaling, D71 introduce
+Layer Split, which split the whole image to two half part and feed them to two
+Layers A and B, and do the scaling independently, after scaling the result need
+to be feed to merger to merge two part image, and then output to compiz.
+
+Splitter
+
+Similar to Layer Split, but Splitter is used for writeback, which split the
+compiz result to two part and then feed them to two scaler.
+
+Possible D71 Pipeline usage
+===
+
+Benefit from the modularized architecture, D71 pipelines can be easily adjusted
+to fit different usages, following are some typical pipeline data flow
+configurations:
+
+Single pipeline data flow
+-
+
+.. kernel-render:: DOT
+ :alt: Single pipeline digraph
+ :caption: Single pipeline data flow
+
+ digraph single_ppl {
+ rankdir=LR;
+
+ subgraph {
+ "Memory";
+ "Monitor";
+ }
+
+ subgraph cluster_pipeline {
+ style=dashed
+ node [shape=box]
+ {
+ node [bgcolor=grey style=dashed]
+ "Scaler-0";
+ "Scaler-1";
+ "Scaler-0/1"
+ }
+
+ node [bgcolor=grey style=filled]
+ "Layer-0" -> "Scaler-0"
+ "Layer-1" -> "Scaler-0"
+ "Layer-2" -> "Scaler-1"
+ "Layer-3" -> "Scaler-1"
+
+ "Layer-0" -> "Compiz"
+ "Layer-1" -> "Compiz"
+ "Layer-2" -> "Compiz"
+ "Layer-3" -> "Compiz"
+ "Scaler-0" -> "Compiz"
+ "Scaler-1" -> "Compiz"
+
+ "Compiz" -> "Scaler-0/1" ->
[PATCH v1 8/9] drm/doc: Add initial komeda driver documentation
Signed-off-by: James (Qian) Wang
---
Documentation/gpu/drivers.rst| 1 +
Documentation/gpu/komeda-kms.rst | 483 +++
2 files changed, 484 insertions(+)
create mode 100644 Documentation/gpu/komeda-kms.rst
diff --git a/Documentation/gpu/drivers.rst b/Documentation/gpu/drivers.rst
index 7c1672118a73..978e6da9bbff 100644
--- a/Documentation/gpu/drivers.rst
+++ b/Documentation/gpu/drivers.rst
@@ -17,6 +17,7 @@ GPU Driver Documentation
vkms
bridge/dw-hdmi
xen-front
+ komeda-kms
.. only:: subproject and html
diff --git a/Documentation/gpu/komeda-kms.rst b/Documentation/gpu/komeda-kms.rst
new file mode 100644
index ..8af925ca0869
--- /dev/null
+++ b/Documentation/gpu/komeda-kms.rst
@@ -0,0 +1,483 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==
+ drm/komeda ARM display driver
+==
+
+The drm/komeda driver supports for the ARM display processor D71 and later
+IPs, this document is for giving a brief overview of driver design: how it
+works and why design it like that.
+
+Overview of D71 like display IPs
+
+
+From D71, Arm display IP begins to adopt a flexible and modularized
+architecture. A display pipeline is made up of multiple individual and
+functional pipeline stages called components, and every component has some
+specific capabilities that can give the flowed pipeline pixel data a
+particular processing.
+
+Typical D71 components:
+
+Layer
+-
+Layer is the first pipeline stage, which is for preparing the pixel data
+for the next stage. It fetches the pixel from memory, decodes it if it's
+AFBC, rotates the source image, unpacks or converts YUV pixels to the device
+internal RGB pixels, then adjust the color_space of pixels if need.
+
+Scaler
+--
+As its name, scaler is taking responsability for scaling, and D71 also
+supports image enhancements by scaler.
+The usage of scaler is very flexible and can be connected to layer output
+for layer scaling, or connected to compositor and scale the whole display
+frame and then feed the output data into wb_layer which will then write it
+into memory.
+
+Compositor (compiz)
+---
+Compositor is for blending multiple layers or pixel data flows into one
+single display frame, and its output frame can be fed into post image
+processor and then on the monitor or fed into wb_layer and written to memory
+at the same time. And user also can insert a scaler between compositor and
+wb_layer to down scale the display frame first and then writing to memory.
+
+Writeback Layer (wb_layer)
+--
+Writeback layer do the opposite things of Layer, Which connect to compiz and
try
+to write the composition result to memory.
+
+Post image processor (improc)
+-
+Post image processor is for adjusting frame data like gamma and color space
+to fit the requirements of the monitor.
+
+Timing controller (timing_ctrlr)
+
+Final stage of display pipeline, Timing controller is not for the pixel
+handling, but only for controlling the display timing.
+
+Merger
+--
+D71 scaler mostly only has half the horizontal input/output capabilities
compare
+with Layer, Like if Layer supports 4K input size, the scaler only can supports
+2K input/output in some time. To achieve the fully frame scaling, D71 introduce
+Layer Split, which split the whole image to two half part and feed them to two
+Layers A and B, and do the scaling independently, after scaling the result need
+to be feed to merger to merge two part image, and then output to compiz.
+
+Splitter
+
+Similar to Layer Split, but Splitter is used for writeback, which split the
+compiz result to two part and then feed them to two scaler.
+
+Possible D71 Pipeline usage
+===
+
+Benefit from the modularized architecture, D71 pipelines can be easily adjusted
+to fit different usages, following are some typical pipeline data flow
+configurations:
+
+Single pipeline data flow
+-
+
+.. kernel-render:: DOT
+ :alt: Single pipeline digraph
+ :caption: Single pipeline data flow
+
+ digraph single_ppl {
+ rankdir=LR;
+
+ subgraph {
+ "Memory";
+ "Monitor";
+ }
+
+ subgraph cluster_pipeline {
+ style=dashed
+ node [shape=box]
+ {
+ node [bgcolor=grey style=dashed]
+ "Scaler-0";
+ "Scaler-1";
+ "Scaler-0/1"
+ }
+
+ node [bgcolor=grey style=filled]
+ "Layer-0" -> "Scaler-0"
+ "Layer-1" -> "Scaler-0"
+ "Layer-2" -> "Scaler-1"
+ "Layer-3" -> "Scaler-1"
+
+ "Layer-0" -> "Compiz"
+ "Layer-1" -> "Compiz"
+ "Layer-2" -> "Compiz"
+ "Layer-3" -> "Compiz"
+ "Scaler-0" -> "Compiz"
+ "Scaler-1" -> "Compiz"
+
+ "Compiz" -> "Scaler-0/1" ->
Re: [PATCH 2/2] core-api/memory-hotplug.rst: divide Locking Internal section by different locks
On Wed, Dec 05, 2018 at 10:40:45AM +0200, Mike Rapoport wrote: >On Wed, Dec 05, 2018 at 10:34:26AM +0800, Wei Yang wrote: >> Currently locking for memory hotplug is a little complicated. >> >> Generally speaking, we leverage the two global lock: >> >> * device_hotplug_lock >> * mem_hotplug_lock >> >> to serialise the process. >> >> While for the long term, we are willing to have more fine-grained lock >> to provide higher scalability. >> >> This patch divides Locking Internal section based on these two global >> locks to help readers to understand it. Also it adds some new finding to >> enrich it. >> >> [David: words arrangement] >> >> Signed-off-by: Wei Yang >> --- >> Documentation/core-api/memory-hotplug.rst | 27 --- >> 1 file changed, 24 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/core-api/memory-hotplug.rst >> b/Documentation/core-api/memory-hotplug.rst >> index de7467e48067..95662b283328 100644 >> --- a/Documentation/core-api/memory-hotplug.rst >> +++ b/Documentation/core-api/memory-hotplug.rst >> @@ -89,6 +89,20 @@ NOTIFY_STOP stops further processing of the notification >> queue. >> Locking Internals >> = >> >> +There are three locks involved in memory-hotplug, two global lock and one >> local > >typo: ^locks > Thanks :-) >> +lock: >> + >> +- device_hotplug_lock >> +- mem_hotplug_lock >> +- device_lock >> + >> +Currently, they are twisted together for all kinds of reasons. The following >> +part is divided into device_hotplug_lock and mem_hotplug_lock parts >> +respectively to describe those tricky situations. >> + >> +device_hotplug_lock >> +- >> + >> When adding/removing memory that uses memory block devices (i.e. ordinary >> RAM), >> the device_hotplug_lock should be held to: >> >> @@ -111,13 +125,20 @@ As the device is visible to user space before taking >> the device_lock(), this >> can result in a lock inversion. >> >> onlining/offlining of memory should be done via device_online()/ >> -device_offline() - to make sure it is properly synchronized to actions >> -via sysfs. Holding device_hotplug_lock is advised (to e.g. protect >> online_type) >> +device_offline() - to make sure it is properly synchronized to actions via >> +sysfs. Even mem_hotplug_lock is used to protect the process, because of the > >I think it should be "Even if mem_hotplug_lock ..." > Ah, my poor English, will fix it in next version. :-) >> +lock inversion described above, holding device_hotplug_lock is still advised >> +(to e.g. protect online_type) >> + >> +mem_hotplug_lock >> +- >> >> When adding/removing/onlining/offlining memory or adding/removing >> heterogeneous/device memory, we should always hold the mem_hotplug_lock in >> write mode to serialise memory hotplug (e.g. access to global/zone >> -variables). >> +variables). Currently, we take advantage of this to serialise sparsemem's >> +mem_section handling in sparse_add_one_section() and >> +sparse_remove_one_section(). >> >> In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read >> mode allows for a quite efficient get_online_mems/put_online_mems >> -- >> 2.15.1 >> > >-- >Sincerely yours, >Mike. -- Wei Yang Help you, Help me
Re: [PATCH 2/2] core-api/memory-hotplug.rst: divide Locking Internal section by different locks
On Wed, Dec 05, 2018 at 09:08:47AM +0100, David Hildenbrand wrote: >On 05.12.18 03:34, Wei Yang wrote: >> Currently locking for memory hotplug is a little complicated. >> >> Generally speaking, we leverage the two global lock: >> >> * device_hotplug_lock >> * mem_hotplug_lock >> >> to serialise the process. >> >> While for the long term, we are willing to have more fine-grained lock >> to provide higher scalability. >> >> This patch divides Locking Internal section based on these two global >> locks to help readers to understand it. Also it adds some new finding to >> enrich it. >> >> [David: words arrangement] >> >> Signed-off-by: Wei Yang >> --- >> Documentation/core-api/memory-hotplug.rst | 27 --- >> 1 file changed, 24 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/core-api/memory-hotplug.rst >> b/Documentation/core-api/memory-hotplug.rst >> index de7467e48067..95662b283328 100644 >> --- a/Documentation/core-api/memory-hotplug.rst >> +++ b/Documentation/core-api/memory-hotplug.rst >> @@ -89,6 +89,20 @@ NOTIFY_STOP stops further processing of the notification >> queue. >> Locking Internals >> = >> >> +There are three locks involved in memory-hotplug, two global lock and one >> local >> +lock: >> + >> +- device_hotplug_lock >> +- mem_hotplug_lock >> +- device_lock >> + > >Do we really only ever use these three and not anything else when >adding/removing/onlining/offlining memory? > >(I am thinking e.g. about pgdat_resize_lock) Yes there are more than those three, pgdat_resize_lock is one of them. > >If so, you should phrase that maybe more generally Or add more details :) Yep, while I don't get a whole picture about the pgdat_resize_lock. The usage of this lock scatter in many places. > >"In addition to fine grained locks like pgdat_resize_lock, there are >three locks involved ..." > Sounds better :-) -- Wei Yang Help you, Help me
Re: [PATCH 1/2] admin-guide/memory-hotplug.rst: remove locking internal part from admin-guide
On Wed, Dec 05, 2018 at 10:30:13AM +0200, Mike Rapoport wrote: >On Wed, Dec 05, 2018 at 09:03:24AM +0100, David Hildenbrand wrote: >> On 05.12.18 03:34, Wei Yang wrote: >> > Locking Internal section exists in core-api documentation, which is more >> > suitable for this. >> > >> > This patch removes the duplication part here. >> > >> > Signed-off-by: Wei Yang >> > --- >> > Documentation/admin-guide/mm/memory-hotplug.rst | 40 >> > - >> > 1 file changed, 40 deletions(-) >> > >> > diff --git a/Documentation/admin-guide/mm/memory-hotplug.rst >> > b/Documentation/admin-guide/mm/memory-hotplug.rst >> > index 5c4432c96c4b..241f4ce1e387 100644 >> > --- a/Documentation/admin-guide/mm/memory-hotplug.rst >> > +++ b/Documentation/admin-guide/mm/memory-hotplug.rst >> > @@ -392,46 +392,6 @@ Need more implementation yet >> > - Notification completion of remove works by OS to firmware. >> > - Guard from remove if not yet. > >[ ... ] > >> > Future Work >> > === >> > >> > >> >> I reported this yesterday to Jonathan and Mike >> >> https://lkml.org/lkml/2018/12/3/340 > >Somehow I've missed it... > >> Anyhow >> >> Reviewed-by: David Hildenbrand > >Acked-by: Mike Rapoport > Thanks :-) >> >> -- >> >> Thanks, >> >> David / dhildenb >> > >-- >Sincerely yours, >Mike. -- Wei Yang Help you, Help me
Re: [PATCH 1/2] admin-guide/memory-hotplug.rst: remove locking internal part from admin-guide
On Wed, Dec 05, 2018 at 09:03:24AM +0100, David Hildenbrand wrote: >On 05.12.18 03:34, Wei Yang wrote: >> Locking Internal section exists in core-api documentation, which is more >> suitable for this. >> >> This patch removes the duplication part here. >> >> Signed-off-by: Wei Yang >> --- >> Documentation/admin-guide/mm/memory-hotplug.rst | 40 >> - >> 1 file changed, 40 deletions(-) >> >> diff --git a/Documentation/admin-guide/mm/memory-hotplug.rst >> b/Documentation/admin-guide/mm/memory-hotplug.rst >> index 5c4432c96c4b..241f4ce1e387 100644 >> --- a/Documentation/admin-guide/mm/memory-hotplug.rst >> +++ b/Documentation/admin-guide/mm/memory-hotplug.rst >> @@ -392,46 +392,6 @@ Need more implementation yet >> - Notification completion of remove works by OS to firmware. >> - Guard from remove if not yet. >> >> - >> -Locking Internals >> -= >> - >> -When adding/removing memory that uses memory block devices (i.e. ordinary >> RAM), >> -the device_hotplug_lock should be held to: >> - >> -- synchronize against online/offline requests (e.g. via sysfs). This way, >> memory >> - block devices can only be accessed (.online/.state attributes) by user >> - space once memory has been fully added. And when removing memory, we >> - know nobody is in critical sections. >> -- synchronize against CPU hotplug and similar (e.g. relevant for ACPI and >> PPC) >> - >> -Especially, there is a possible lock inversion that is avoided using >> -device_hotplug_lock when adding memory and user space tries to online that >> -memory faster than expected: >> - >> -- device_online() will first take the device_lock(), followed by >> - mem_hotplug_lock >> -- add_memory_resource() will first take the mem_hotplug_lock, followed by >> - the device_lock() (while creating the devices, during bus_add_device()). >> - >> -As the device is visible to user space before taking the device_lock(), this >> -can result in a lock inversion. >> - >> -onlining/offlining of memory should be done via device_online()/ >> -device_offline() - to make sure it is properly synchronized to actions >> -via sysfs. Holding device_hotplug_lock is advised (to e.g. protect >> online_type) >> - >> -When adding/removing/onlining/offlining memory or adding/removing >> -heterogeneous/device memory, we should always hold the mem_hotplug_lock in >> -write mode to serialise memory hotplug (e.g. access to global/zone >> -variables). >> - >> -In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read >> -mode allows for a quite efficient get_online_mems/put_online_mems >> -implementation, so code accessing memory can protect from that memory >> -vanishing. >> - >> - >> Future Work >> === >> >> > >I reported this yesterday to Jonathan and Mike > >https://lkml.org/lkml/2018/12/3/340 > Ah, Thanks :-) > >Anyhow > >Reviewed-by: David Hildenbrand > >-- > >Thanks, > >David / dhildenb -- Wei Yang Help you, Help me
Re: [PATCH 2/2] core-api/memory-hotplug.rst: divide Locking Internal section by different locks
On Wed, Dec 05, 2018 at 10:34:26AM +0800, Wei Yang wrote: > Currently locking for memory hotplug is a little complicated. > > Generally speaking, we leverage the two global lock: > > * device_hotplug_lock > * mem_hotplug_lock > > to serialise the process. > > While for the long term, we are willing to have more fine-grained lock > to provide higher scalability. > > This patch divides Locking Internal section based on these two global > locks to help readers to understand it. Also it adds some new finding to > enrich it. > > [David: words arrangement] > > Signed-off-by: Wei Yang > --- > Documentation/core-api/memory-hotplug.rst | 27 --- > 1 file changed, 24 insertions(+), 3 deletions(-) > > diff --git a/Documentation/core-api/memory-hotplug.rst > b/Documentation/core-api/memory-hotplug.rst > index de7467e48067..95662b283328 100644 > --- a/Documentation/core-api/memory-hotplug.rst > +++ b/Documentation/core-api/memory-hotplug.rst > @@ -89,6 +89,20 @@ NOTIFY_STOP stops further processing of the notification > queue. > Locking Internals > = > > +There are three locks involved in memory-hotplug, two global lock and one > local typo: ^locks > +lock: > + > +- device_hotplug_lock > +- mem_hotplug_lock > +- device_lock > + > +Currently, they are twisted together for all kinds of reasons. The following > +part is divided into device_hotplug_lock and mem_hotplug_lock parts > +respectively to describe those tricky situations. > + > +device_hotplug_lock > +- > + > When adding/removing memory that uses memory block devices (i.e. ordinary > RAM), > the device_hotplug_lock should be held to: > > @@ -111,13 +125,20 @@ As the device is visible to user space before taking > the device_lock(), this > can result in a lock inversion. > > onlining/offlining of memory should be done via device_online()/ > -device_offline() - to make sure it is properly synchronized to actions > -via sysfs. Holding device_hotplug_lock is advised (to e.g. protect > online_type) > +device_offline() - to make sure it is properly synchronized to actions via > +sysfs. Even mem_hotplug_lock is used to protect the process, because of the I think it should be "Even if mem_hotplug_lock ..." > +lock inversion described above, holding device_hotplug_lock is still advised > +(to e.g. protect online_type) > + > +mem_hotplug_lock > +- > > When adding/removing/onlining/offlining memory or adding/removing > heterogeneous/device memory, we should always hold the mem_hotplug_lock in > write mode to serialise memory hotplug (e.g. access to global/zone > -variables). > +variables). Currently, we take advantage of this to serialise sparsemem's > +mem_section handling in sparse_add_one_section() and > +sparse_remove_one_section(). > > In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read > mode allows for a quite efficient get_online_mems/put_online_mems > -- > 2.15.1 > -- Sincerely yours, Mike.
Re: [PATCH 1/2] admin-guide/memory-hotplug.rst: remove locking internal part from admin-guide
On Wed, Dec 05, 2018 at 09:03:24AM +0100, David Hildenbrand wrote: > On 05.12.18 03:34, Wei Yang wrote: > > Locking Internal section exists in core-api documentation, which is more > > suitable for this. > > > > This patch removes the duplication part here. > > > > Signed-off-by: Wei Yang > > --- > > Documentation/admin-guide/mm/memory-hotplug.rst | 40 > > - > > 1 file changed, 40 deletions(-) > > > > diff --git a/Documentation/admin-guide/mm/memory-hotplug.rst > > b/Documentation/admin-guide/mm/memory-hotplug.rst > > index 5c4432c96c4b..241f4ce1e387 100644 > > --- a/Documentation/admin-guide/mm/memory-hotplug.rst > > +++ b/Documentation/admin-guide/mm/memory-hotplug.rst > > @@ -392,46 +392,6 @@ Need more implementation yet > > - Notification completion of remove works by OS to firmware. > > - Guard from remove if not yet. [ ... ] > > Future Work > > === > > > > > > I reported this yesterday to Jonathan and Mike > > https://lkml.org/lkml/2018/12/3/340 Somehow I've missed it... > Anyhow > > Reviewed-by: David Hildenbrand Acked-by: Mike Rapoport > > -- > > Thanks, > > David / dhildenb > -- Sincerely yours, Mike.
Re: [PATCH 2/2] core-api/memory-hotplug.rst: divide Locking Internal section by different locks
On 05.12.18 03:34, Wei Yang wrote: > Currently locking for memory hotplug is a little complicated. > > Generally speaking, we leverage the two global lock: > > * device_hotplug_lock > * mem_hotplug_lock > > to serialise the process. > > While for the long term, we are willing to have more fine-grained lock > to provide higher scalability. > > This patch divides Locking Internal section based on these two global > locks to help readers to understand it. Also it adds some new finding to > enrich it. > > [David: words arrangement] > > Signed-off-by: Wei Yang > --- > Documentation/core-api/memory-hotplug.rst | 27 --- > 1 file changed, 24 insertions(+), 3 deletions(-) > > diff --git a/Documentation/core-api/memory-hotplug.rst > b/Documentation/core-api/memory-hotplug.rst > index de7467e48067..95662b283328 100644 > --- a/Documentation/core-api/memory-hotplug.rst > +++ b/Documentation/core-api/memory-hotplug.rst > @@ -89,6 +89,20 @@ NOTIFY_STOP stops further processing of the notification > queue. > Locking Internals > = > > +There are three locks involved in memory-hotplug, two global lock and one > local > +lock: > + > +- device_hotplug_lock > +- mem_hotplug_lock > +- device_lock > + Do we really only ever use these three and not anything else when adding/removing/onlining/offlining memory? (I am thinking e.g. about pgdat_resize_lock) If so, you should phrase that maybe more generally Or add more details :) "In addition to fine grained locks like pgdat_resize_lock, there are three locks involved ..." > +Currently, they are twisted together for all kinds of reasons. The following > +part is divided into device_hotplug_lock and mem_hotplug_lock parts > +respectively to describe those tricky situations. > + > +device_hotplug_lock > +- > + > When adding/removing memory that uses memory block devices (i.e. ordinary > RAM), > the device_hotplug_lock should be held to: > > @@ -111,13 +125,20 @@ As the device is visible to user space before taking > the device_lock(), this > can result in a lock inversion. > > onlining/offlining of memory should be done via device_online()/ > -device_offline() - to make sure it is properly synchronized to actions > -via sysfs. Holding device_hotplug_lock is advised (to e.g. protect > online_type) > +device_offline() - to make sure it is properly synchronized to actions via > +sysfs. Even mem_hotplug_lock is used to protect the process, because of the > +lock inversion described above, holding device_hotplug_lock is still advised > +(to e.g. protect online_type) > + > +mem_hotplug_lock > +- > > When adding/removing/onlining/offlining memory or adding/removing > heterogeneous/device memory, we should always hold the mem_hotplug_lock in > write mode to serialise memory hotplug (e.g. access to global/zone > -variables). > +variables). Currently, we take advantage of this to serialise sparsemem's > +mem_section handling in sparse_add_one_section() and > +sparse_remove_one_section(). > > In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read > mode allows for a quite efficient get_online_mems/put_online_mems > -- Thanks, David / dhildenb
Re: [PATCH 1/2] admin-guide/memory-hotplug.rst: remove locking internal part from admin-guide
On 05.12.18 03:34, Wei Yang wrote: > Locking Internal section exists in core-api documentation, which is more > suitable for this. > > This patch removes the duplication part here. > > Signed-off-by: Wei Yang > --- > Documentation/admin-guide/mm/memory-hotplug.rst | 40 > - > 1 file changed, 40 deletions(-) > > diff --git a/Documentation/admin-guide/mm/memory-hotplug.rst > b/Documentation/admin-guide/mm/memory-hotplug.rst > index 5c4432c96c4b..241f4ce1e387 100644 > --- a/Documentation/admin-guide/mm/memory-hotplug.rst > +++ b/Documentation/admin-guide/mm/memory-hotplug.rst > @@ -392,46 +392,6 @@ Need more implementation yet > - Notification completion of remove works by OS to firmware. > - Guard from remove if not yet. > > - > -Locking Internals > -= > - > -When adding/removing memory that uses memory block devices (i.e. ordinary > RAM), > -the device_hotplug_lock should be held to: > - > -- synchronize against online/offline requests (e.g. via sysfs). This way, > memory > - block devices can only be accessed (.online/.state attributes) by user > - space once memory has been fully added. And when removing memory, we > - know nobody is in critical sections. > -- synchronize against CPU hotplug and similar (e.g. relevant for ACPI and > PPC) > - > -Especially, there is a possible lock inversion that is avoided using > -device_hotplug_lock when adding memory and user space tries to online that > -memory faster than expected: > - > -- device_online() will first take the device_lock(), followed by > - mem_hotplug_lock > -- add_memory_resource() will first take the mem_hotplug_lock, followed by > - the device_lock() (while creating the devices, during bus_add_device()). > - > -As the device is visible to user space before taking the device_lock(), this > -can result in a lock inversion. > - > -onlining/offlining of memory should be done via device_online()/ > -device_offline() - to make sure it is properly synchronized to actions > -via sysfs. Holding device_hotplug_lock is advised (to e.g. protect > online_type) > - > -When adding/removing/onlining/offlining memory or adding/removing > -heterogeneous/device memory, we should always hold the mem_hotplug_lock in > -write mode to serialise memory hotplug (e.g. access to global/zone > -variables). > - > -In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read > -mode allows for a quite efficient get_online_mems/put_online_mems > -implementation, so code accessing memory can protect from that memory > -vanishing. > - > - > Future Work > === > > I reported this yesterday to Jonathan and Mike https://lkml.org/lkml/2018/12/3/340 Anyhow Reviewed-by: David Hildenbrand -- Thanks, David / dhildenb
