* [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.