* [PATCH v2 0/2] fix devm_memremap_pages() mem hotplug locking
@ 2017-02-16 21:53 ` Dan Williams
0 siblings, 0 replies; 10+ messages in thread
From: Dan Williams @ 2017-02-16 21:53 UTC (permalink / raw)
To: akpm
Cc: Michal Hocko, linux-nvdimm, Greg Kroah-Hartman, stable, linux-mm,
Ben Hutchings, Vlastimil Babka
Changes since v1 [1]:
* Reflowed the patches on 4.10-rc8. The v1 series no longer applies
to -mm now that the sub-section memory hotplug support has been
deferred to 4.12 [2].
[1]: https://lists.01.org/pipermail/linux-nvdimm/2017-February/008848.html
[2]: http://www.spinics.net/lists/linux-mm/msg121990.html
---
Ben notes that commit f931ab479dd2 "mm: fix devm_memremap_pages crash,
use mem_hotplug_{begin, done}" is incomplete and broken. Writes to
mem_hotplug.active_writer need to be coordinated under the device
hotplug lock. Otherwise, we can potentially corrupt mem_hotplug.refcount
leading to soft lockups.
---
Dan Williams (2):
mm, devm_memremap_pages: hold device_hotplug lock over mem_hotplug_{begin,done}
mm: validate device_hotplug is held for memory hotplug
drivers/base/core.c | 5 +++++
include/linux/device.h | 1 +
kernel/memremap.c | 6 ++++++
mm/memory_hotplug.c | 2 ++
4 files changed, 14 insertions(+)
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 0/2] fix devm_memremap_pages() mem hotplug locking
@ 2017-02-16 21:53 ` Dan Williams
0 siblings, 0 replies; 10+ messages in thread
From: Dan Williams @ 2017-02-16 21:53 UTC (permalink / raw)
To: akpm
Cc: Michal Hocko, Toshi Kani, Ben Hutchings, Greg Kroah-Hartman,
linux-nvdimm, stable, linux-mm, Logan Gunthorpe, Vlastimil Babka
Changes since v1 [1]:
* Reflowed the patches on 4.10-rc8. The v1 series no longer applies
to -mm now that the sub-section memory hotplug support has been
deferred to 4.12 [2].
[1]: https://lists.01.org/pipermail/linux-nvdimm/2017-February/008848.html
[2]: http://www.spinics.net/lists/linux-mm/msg121990.html
---
Ben notes that commit f931ab479dd2 "mm: fix devm_memremap_pages crash,
use mem_hotplug_{begin, done}" is incomplete and broken. Writes to
mem_hotplug.active_writer need to be coordinated under the device
hotplug lock. Otherwise, we can potentially corrupt mem_hotplug.refcount
leading to soft lockups.
---
Dan Williams (2):
mm, devm_memremap_pages: hold device_hotplug lock over mem_hotplug_{begin,done}
mm: validate device_hotplug is held for memory hotplug
drivers/base/core.c | 5 +++++
include/linux/device.h | 1 +
kernel/memremap.c | 6 ++++++
mm/memory_hotplug.c | 2 ++
4 files changed, 14 insertions(+)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] mm, devm_memremap_pages: hold device_hotplug lock over mem_hotplug_{begin, done}
2017-02-16 21:53 ` Dan Williams
@ 2017-02-16 21:53 ` Dan Williams
-1 siblings, 0 replies; 10+ messages in thread
From: Dan Williams @ 2017-02-16 21:53 UTC (permalink / raw)
To: akpm
Cc: Michal Hocko, linux-nvdimm, Ben Hutchings, stable, linux-mm,
Vlastimil Babka
The mem_hotplug_{begin,done} lock coordinates with
{get,put}_online_mems() to hold off "readers" of the current state of
memory from new hotplug actions. mem_hotplug_begin() expects exclusive
access, via the device_hotplug lock, to set mem_hotplug.active_writer.
Calling mem_hotplug_begin() without locking device_hotplug can lead to
corrupting mem_hotplug.refcount and missed wakeups / soft lockups.
Cc: <stable@vger.kernel.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Toshi Kani <toshi.kani@hpe.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Logan Gunthorpe <logang@deltatee.com>
Fixes: f931ab479dd2 ("mm: fix devm_memremap_pages crash, use mem_hotplug_{begin, done}")
Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
kernel/memremap.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/memremap.c b/kernel/memremap.c
index 9ecedc28b928..06123234f118 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -246,9 +246,13 @@ static void devm_memremap_pages_release(struct device *dev, void *data)
/* pages are dead and unused, undo the arch mapping */
align_start = res->start & ~(SECTION_SIZE - 1);
align_size = ALIGN(resource_size(res), SECTION_SIZE);
+
+ lock_device_hotplug();
mem_hotplug_begin();
arch_remove_memory(align_start, align_size);
mem_hotplug_done();
+ unlock_device_hotplug();
+
untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
pgmap_radix_release(res);
dev_WARN_ONCE(dev, pgmap->altmap && pgmap->altmap->alloc,
@@ -360,9 +364,11 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
if (error)
goto err_pfn_remap;
+ lock_device_hotplug();
mem_hotplug_begin();
error = arch_add_memory(nid, align_start, align_size, true);
mem_hotplug_done();
+ unlock_device_hotplug();
if (error)
goto err_add_memory;
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] mm, devm_memremap_pages: hold device_hotplug lock over mem_hotplug_{begin, done}
@ 2017-02-16 21:53 ` Dan Williams
0 siblings, 0 replies; 10+ messages in thread
From: Dan Williams @ 2017-02-16 21:53 UTC (permalink / raw)
To: akpm
Cc: Michal Hocko, Toshi Kani, linux-nvdimm, Logan Gunthorpe, stable,
linux-mm, Ben Hutchings, Vlastimil Babka
The mem_hotplug_{begin,done} lock coordinates with
{get,put}_online_mems() to hold off "readers" of the current state of
memory from new hotplug actions. mem_hotplug_begin() expects exclusive
access, via the device_hotplug lock, to set mem_hotplug.active_writer.
Calling mem_hotplug_begin() without locking device_hotplug can lead to
corrupting mem_hotplug.refcount and missed wakeups / soft lockups.
Cc: <stable@vger.kernel.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Toshi Kani <toshi.kani@hpe.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Logan Gunthorpe <logang@deltatee.com>
Fixes: f931ab479dd2 ("mm: fix devm_memremap_pages crash, use mem_hotplug_{begin, done}")
Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
kernel/memremap.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/memremap.c b/kernel/memremap.c
index 9ecedc28b928..06123234f118 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -246,9 +246,13 @@ static void devm_memremap_pages_release(struct device *dev, void *data)
/* pages are dead and unused, undo the arch mapping */
align_start = res->start & ~(SECTION_SIZE - 1);
align_size = ALIGN(resource_size(res), SECTION_SIZE);
+
+ lock_device_hotplug();
mem_hotplug_begin();
arch_remove_memory(align_start, align_size);
mem_hotplug_done();
+ unlock_device_hotplug();
+
untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
pgmap_radix_release(res);
dev_WARN_ONCE(dev, pgmap->altmap && pgmap->altmap->alloc,
@@ -360,9 +364,11 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
if (error)
goto err_pfn_remap;
+ lock_device_hotplug();
mem_hotplug_begin();
error = arch_add_memory(nid, align_start, align_size, true);
mem_hotplug_done();
+ unlock_device_hotplug();
if (error)
goto err_add_memory;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] mm: validate device_hotplug is held for memory hotplug
2017-02-16 21:53 ` Dan Williams
@ 2017-02-16 21:53 ` Dan Williams
-1 siblings, 0 replies; 10+ messages in thread
From: Dan Williams @ 2017-02-16 21:53 UTC (permalink / raw)
To: akpm
Cc: Michal Hocko, linux-nvdimm, Greg Kroah-Hartman, linux-mm,
Ben Hutchings, Vlastimil Babka
mem_hotplug_begin() assumes that it can set mem_hotplug.active_writer
and run the hotplug process without racing another thread. Validate this
assumption with a lockdep assertion.
Cc: Michal Hocko <mhocko@suse.com>
Cc: Toshi Kani <toshi.kani@hpe.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/base/core.c | 5 +++++
include/linux/device.h | 1 +
mm/memory_hotplug.c | 2 ++
3 files changed, 8 insertions(+)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 8c25e68e67d7..3050e6f99403 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -638,6 +638,11 @@ int lock_device_hotplug_sysfs(void)
return restart_syscall();
}
+void assert_held_device_hotplug(void)
+{
+ lockdep_assert_held(&device_hotplug_lock);
+}
+
#ifdef CONFIG_BLOCK
static inline int device_is_not_partition(struct device *dev)
{
diff --git a/include/linux/device.h b/include/linux/device.h
index 491b4c0ca633..815965ee55dd 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1135,6 +1135,7 @@ static inline bool device_supports_offline(struct device *dev)
extern void lock_device_hotplug(void);
extern void unlock_device_hotplug(void);
extern int lock_device_hotplug_sysfs(void);
+void assert_held_device_hotplug(void);
extern int device_offline(struct device *dev);
extern int device_online(struct device *dev);
extern void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index b8c11e063ff0..1635a2a085e5 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -126,6 +126,8 @@ void put_online_mems(void)
void mem_hotplug_begin(void)
{
+ assert_held_device_hotplug();
+
mem_hotplug.active_writer = current;
memhp_lock_acquire();
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] mm: validate device_hotplug is held for memory hotplug
@ 2017-02-16 21:53 ` Dan Williams
0 siblings, 0 replies; 10+ messages in thread
From: Dan Williams @ 2017-02-16 21:53 UTC (permalink / raw)
To: akpm
Cc: Michal Hocko, Toshi Kani, Ben Hutchings, Greg Kroah-Hartman,
linux-nvdimm, linux-mm, Logan Gunthorpe, Vlastimil Babka
mem_hotplug_begin() assumes that it can set mem_hotplug.active_writer
and run the hotplug process without racing another thread. Validate this
assumption with a lockdep assertion.
Cc: Michal Hocko <mhocko@suse.com>
Cc: Toshi Kani <toshi.kani@hpe.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/base/core.c | 5 +++++
include/linux/device.h | 1 +
mm/memory_hotplug.c | 2 ++
3 files changed, 8 insertions(+)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 8c25e68e67d7..3050e6f99403 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -638,6 +638,11 @@ int lock_device_hotplug_sysfs(void)
return restart_syscall();
}
+void assert_held_device_hotplug(void)
+{
+ lockdep_assert_held(&device_hotplug_lock);
+}
+
#ifdef CONFIG_BLOCK
static inline int device_is_not_partition(struct device *dev)
{
diff --git a/include/linux/device.h b/include/linux/device.h
index 491b4c0ca633..815965ee55dd 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1135,6 +1135,7 @@ static inline bool device_supports_offline(struct device *dev)
extern void lock_device_hotplug(void);
extern void unlock_device_hotplug(void);
extern int lock_device_hotplug_sysfs(void);
+void assert_held_device_hotplug(void);
extern int device_offline(struct device *dev);
extern int device_online(struct device *dev);
extern void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index b8c11e063ff0..1635a2a085e5 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -126,6 +126,8 @@ void put_online_mems(void)
void mem_hotplug_begin(void)
{
+ assert_held_device_hotplug();
+
mem_hotplug.active_writer = current;
memhp_lock_acquire();
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] mm: validate device_hotplug is held for memory hotplug
2017-02-16 21:53 ` Dan Williams
@ 2017-02-17 3:08 ` Ross Zwisler
-1 siblings, 0 replies; 10+ messages in thread
From: Ross Zwisler @ 2017-02-17 3:08 UTC (permalink / raw)
To: Dan Williams
Cc: Michal Hocko, linux-nvdimm, Greg Kroah-Hartman, linux-mm, akpm,
Ben Hutchings, Vlastimil Babka
On Thu, Feb 16, 2017 at 01:53:58PM -0800, Dan Williams wrote:
> mem_hotplug_begin() assumes that it can set mem_hotplug.active_writer
> and run the hotplug process without racing another thread. Validate this
> assumption with a lockdep assertion.
>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Toshi Kani <toshi.kani@hpe.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reported-by: Ben Hutchings <ben@decadent.org.uk>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> drivers/base/core.c | 5 +++++
> include/linux/device.h | 1 +
> mm/memory_hotplug.c | 2 ++
> 3 files changed, 8 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 8c25e68e67d7..3050e6f99403 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -638,6 +638,11 @@ int lock_device_hotplug_sysfs(void)
> return restart_syscall();
> }
>
> +void assert_held_device_hotplug(void)
> +{
> + lockdep_assert_held(&device_hotplug_lock);
> +}
> +
> #ifdef CONFIG_BLOCK
> static inline int device_is_not_partition(struct device *dev)
> {
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 491b4c0ca633..815965ee55dd 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1135,6 +1135,7 @@ static inline bool device_supports_offline(struct device *dev)
> extern void lock_device_hotplug(void);
> extern void unlock_device_hotplug(void);
> extern int lock_device_hotplug_sysfs(void);
> +void assert_held_device_hotplug(void);
> extern int device_offline(struct device *dev);
> extern int device_online(struct device *dev);
> extern void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index b8c11e063ff0..1635a2a085e5 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -126,6 +126,8 @@ void put_online_mems(void)
>
> void mem_hotplug_begin(void)
> {
> + assert_held_device_hotplug();
What's the benefit to defining assert_held_device_hotplug() as a one line
wrapper, instead of just calling lockdep_assert_held(&device_hotplug_lock)
directly?
> +
> mem_hotplug.active_writer = current;
>
> memhp_lock_acquire();
>
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] mm: validate device_hotplug is held for memory hotplug
@ 2017-02-17 3:08 ` Ross Zwisler
0 siblings, 0 replies; 10+ messages in thread
From: Ross Zwisler @ 2017-02-17 3:08 UTC (permalink / raw)
To: Dan Williams
Cc: akpm, Michal Hocko, linux-nvdimm, Greg Kroah-Hartman, linux-mm,
Ben Hutchings, Vlastimil Babka
On Thu, Feb 16, 2017 at 01:53:58PM -0800, Dan Williams wrote:
> mem_hotplug_begin() assumes that it can set mem_hotplug.active_writer
> and run the hotplug process without racing another thread. Validate this
> assumption with a lockdep assertion.
>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Toshi Kani <toshi.kani@hpe.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reported-by: Ben Hutchings <ben@decadent.org.uk>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> drivers/base/core.c | 5 +++++
> include/linux/device.h | 1 +
> mm/memory_hotplug.c | 2 ++
> 3 files changed, 8 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 8c25e68e67d7..3050e6f99403 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -638,6 +638,11 @@ int lock_device_hotplug_sysfs(void)
> return restart_syscall();
> }
>
> +void assert_held_device_hotplug(void)
> +{
> + lockdep_assert_held(&device_hotplug_lock);
> +}
> +
> #ifdef CONFIG_BLOCK
> static inline int device_is_not_partition(struct device *dev)
> {
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 491b4c0ca633..815965ee55dd 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1135,6 +1135,7 @@ static inline bool device_supports_offline(struct device *dev)
> extern void lock_device_hotplug(void);
> extern void unlock_device_hotplug(void);
> extern int lock_device_hotplug_sysfs(void);
> +void assert_held_device_hotplug(void);
> extern int device_offline(struct device *dev);
> extern int device_online(struct device *dev);
> extern void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index b8c11e063ff0..1635a2a085e5 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -126,6 +126,8 @@ void put_online_mems(void)
>
> void mem_hotplug_begin(void)
> {
> + assert_held_device_hotplug();
What's the benefit to defining assert_held_device_hotplug() as a one line
wrapper, instead of just calling lockdep_assert_held(&device_hotplug_lock)
directly?
> +
> mem_hotplug.active_writer = current;
>
> memhp_lock_acquire();
>
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] mm: validate device_hotplug is held for memory hotplug
2017-02-17 3:08 ` Ross Zwisler
@ 2017-02-17 3:28 ` Dan Williams
-1 siblings, 0 replies; 10+ messages in thread
From: Dan Williams @ 2017-02-17 3:28 UTC (permalink / raw)
To: Ross Zwisler
Cc: Michal Hocko, linux-nvdimm, Greg Kroah-Hartman, Linux MM,
Andrew Morton, Ben Hutchings, Vlastimil Babka
On Thu, Feb 16, 2017 at 7:08 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> On Thu, Feb 16, 2017 at 01:53:58PM -0800, Dan Williams wrote:
>> mem_hotplug_begin() assumes that it can set mem_hotplug.active_writer
>> and run the hotplug process without racing another thread. Validate this
>> assumption with a lockdep assertion.
>>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: Toshi Kani <toshi.kani@hpe.com>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: Logan Gunthorpe <logang@deltatee.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Reported-by: Ben Hutchings <ben@decadent.org.uk>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>> drivers/base/core.c | 5 +++++
>> include/linux/device.h | 1 +
>> mm/memory_hotplug.c | 2 ++
>> 3 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 8c25e68e67d7..3050e6f99403 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -638,6 +638,11 @@ int lock_device_hotplug_sysfs(void)
>> return restart_syscall();
>> }
>>
>> +void assert_held_device_hotplug(void)
>> +{
>> + lockdep_assert_held(&device_hotplug_lock);
>> +}
>> +
>> #ifdef CONFIG_BLOCK
>> static inline int device_is_not_partition(struct device *dev)
>> {
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 491b4c0ca633..815965ee55dd 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -1135,6 +1135,7 @@ static inline bool device_supports_offline(struct device *dev)
>> extern void lock_device_hotplug(void);
>> extern void unlock_device_hotplug(void);
>> extern int lock_device_hotplug_sysfs(void);
>> +void assert_held_device_hotplug(void);
>> extern int device_offline(struct device *dev);
>> extern int device_online(struct device *dev);
>> extern void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode);
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index b8c11e063ff0..1635a2a085e5 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -126,6 +126,8 @@ void put_online_mems(void)
>>
>> void mem_hotplug_begin(void)
>> {
>> + assert_held_device_hotplug();
>
> What's the benefit to defining assert_held_device_hotplug() as a one line
> wrapper, instead of just calling lockdep_assert_held(&device_hotplug_lock)
> directly?
>
This allows us to keep device_hotplug_lock statically defined and an
internal implementation detail of the device core.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] mm: validate device_hotplug is held for memory hotplug
@ 2017-02-17 3:28 ` Dan Williams
0 siblings, 0 replies; 10+ messages in thread
From: Dan Williams @ 2017-02-17 3:28 UTC (permalink / raw)
To: Ross Zwisler
Cc: Andrew Morton, Michal Hocko, linux-nvdimm, Greg Kroah-Hartman,
Linux MM, Ben Hutchings, Vlastimil Babka
On Thu, Feb 16, 2017 at 7:08 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> On Thu, Feb 16, 2017 at 01:53:58PM -0800, Dan Williams wrote:
>> mem_hotplug_begin() assumes that it can set mem_hotplug.active_writer
>> and run the hotplug process without racing another thread. Validate this
>> assumption with a lockdep assertion.
>>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: Toshi Kani <toshi.kani@hpe.com>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: Logan Gunthorpe <logang@deltatee.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Reported-by: Ben Hutchings <ben@decadent.org.uk>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>> drivers/base/core.c | 5 +++++
>> include/linux/device.h | 1 +
>> mm/memory_hotplug.c | 2 ++
>> 3 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 8c25e68e67d7..3050e6f99403 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -638,6 +638,11 @@ int lock_device_hotplug_sysfs(void)
>> return restart_syscall();
>> }
>>
>> +void assert_held_device_hotplug(void)
>> +{
>> + lockdep_assert_held(&device_hotplug_lock);
>> +}
>> +
>> #ifdef CONFIG_BLOCK
>> static inline int device_is_not_partition(struct device *dev)
>> {
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 491b4c0ca633..815965ee55dd 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -1135,6 +1135,7 @@ static inline bool device_supports_offline(struct device *dev)
>> extern void lock_device_hotplug(void);
>> extern void unlock_device_hotplug(void);
>> extern int lock_device_hotplug_sysfs(void);
>> +void assert_held_device_hotplug(void);
>> extern int device_offline(struct device *dev);
>> extern int device_online(struct device *dev);
>> extern void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode);
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index b8c11e063ff0..1635a2a085e5 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -126,6 +126,8 @@ void put_online_mems(void)
>>
>> void mem_hotplug_begin(void)
>> {
>> + assert_held_device_hotplug();
>
> What's the benefit to defining assert_held_device_hotplug() as a one line
> wrapper, instead of just calling lockdep_assert_held(&device_hotplug_lock)
> directly?
>
This allows us to keep device_hotplug_lock statically defined and an
internal implementation detail of the device core.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-02-17 3:28 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-16 21:53 [PATCH v2 0/2] fix devm_memremap_pages() mem hotplug locking Dan Williams
2017-02-16 21:53 ` Dan Williams
2017-02-16 21:53 ` [PATCH v2 1/2] mm, devm_memremap_pages: hold device_hotplug lock over mem_hotplug_{begin, done} Dan Williams
2017-02-16 21:53 ` Dan Williams
2017-02-16 21:53 ` [PATCH v2 2/2] mm: validate device_hotplug is held for memory hotplug Dan Williams
2017-02-16 21:53 ` Dan Williams
2017-02-17 3:08 ` Ross Zwisler
2017-02-17 3:08 ` Ross Zwisler
2017-02-17 3:28 ` Dan Williams
2017-02-17 3:28 ` Dan Williams
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.