From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f69.google.com (mail-pg0-f69.google.com [74.125.83.69]) by kanga.kvack.org (Postfix) with ESMTP id 8C596681010 for ; Thu, 16 Feb 2017 16:57:54 -0500 (EST) Received: by mail-pg0-f69.google.com with SMTP id v184so39004340pgv.6 for ; Thu, 16 Feb 2017 13:57:54 -0800 (PST) Received: from mga04.intel.com (mga04.intel.com. [192.55.52.120]) by mx.google.com with ESMTPS id 63si8157380pff.175.2017.02.16.13.57.53 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 Feb 2017 13:57:53 -0800 (PST) Subject: [PATCH v2 0/2] fix devm_memremap_pages() mem hotplug locking From: Dan Williams Date: Thu, 16 Feb 2017 13:53:48 -0800 Message-ID: <148728202805.38457.18028105614854319884.stgit@dwillia2-desk3.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: akpm@linux-foundation.org Cc: Michal Hocko , Toshi Kani , Ben Hutchings , Greg Kroah-Hartman , linux-nvdimm@lists.01.org, stable@vger.kernel.org, linux-mm@kvack.org, 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f70.google.com (mail-pg0-f70.google.com [74.125.83.70]) by kanga.kvack.org (Postfix) with ESMTP id DC18C681010 for ; Thu, 16 Feb 2017 16:57:59 -0500 (EST) Received: by mail-pg0-f70.google.com with SMTP id f5so39255264pgi.1 for ; Thu, 16 Feb 2017 13:57:59 -0800 (PST) Received: from mga11.intel.com (mga11.intel.com. [192.55.52.93]) by mx.google.com with ESMTPS id c66si8183303pfb.26.2017.02.16.13.57.58 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 Feb 2017 13:57:59 -0800 (PST) Subject: [PATCH v2 1/2] mm, devm_memremap_pages: hold device_hotplug lock over mem_hotplug_{begin, done} From: Dan Williams Date: Thu, 16 Feb 2017 13:53:53 -0800 Message-ID: <148728203365.38457.17804568297887708345.stgit@dwillia2-desk3.amr.corp.intel.com> In-Reply-To: <148728202805.38457.18028105614854319884.stgit@dwillia2-desk3.amr.corp.intel.com> References: <148728202805.38457.18028105614854319884.stgit@dwillia2-desk3.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: akpm@linux-foundation.org Cc: Michal Hocko , Toshi Kani , linux-nvdimm@lists.01.org, Logan Gunthorpe , stable@vger.kernel.org, linux-mm@kvack.org, 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: Cc: Michal Hocko Cc: Toshi Kani Cc: Vlastimil Babka Cc: Logan Gunthorpe Fixes: f931ab479dd2 ("mm: fix devm_memremap_pages crash, use mem_hotplug_{begin, done}") Reported-by: Ben Hutchings Signed-off-by: Dan Williams --- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f200.google.com (mail-pf0-f200.google.com [209.85.192.200]) by kanga.kvack.org (Postfix) with ESMTP id CB5F2681010 for ; Thu, 16 Feb 2017 16:58:04 -0500 (EST) Received: by mail-pf0-f200.google.com with SMTP id 204so40238454pfx.1 for ; Thu, 16 Feb 2017 13:58:04 -0800 (PST) Received: from mga02.intel.com (mga02.intel.com. [134.134.136.20]) by mx.google.com with ESMTPS id 38si8142036pld.335.2017.02.16.13.58.03 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 Feb 2017 13:58:03 -0800 (PST) Subject: [PATCH v2 2/2] mm: validate device_hotplug is held for memory hotplug From: Dan Williams Date: Thu, 16 Feb 2017 13:53:58 -0800 Message-ID: <148728203880.38457.1158394701925100383.stgit@dwillia2-desk3.amr.corp.intel.com> In-Reply-To: <148728202805.38457.18028105614854319884.stgit@dwillia2-desk3.amr.corp.intel.com> References: <148728202805.38457.18028105614854319884.stgit@dwillia2-desk3.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: akpm@linux-foundation.org Cc: Michal Hocko , Toshi Kani , Ben Hutchings , Greg Kroah-Hartman , linux-nvdimm@lists.01.org, linux-mm@kvack.org, 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 Cc: Toshi Kani Cc: Vlastimil Babka Cc: Logan Gunthorpe Cc: Greg Kroah-Hartman Reported-by: Ben Hutchings Signed-off-by: Dan Williams --- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f71.google.com (mail-pg0-f71.google.com [74.125.83.71]) by kanga.kvack.org (Postfix) with ESMTP id 5956F681021 for ; Thu, 16 Feb 2017 22:08:04 -0500 (EST) Received: by mail-pg0-f71.google.com with SMTP id 65so47584022pgi.7 for ; Thu, 16 Feb 2017 19:08:04 -0800 (PST) Received: from mga05.intel.com (mga05.intel.com. [192.55.52.43]) by mx.google.com with ESMTPS id l1si8858931plk.130.2017.02.16.19.08.03 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 Feb 2017 19:08:03 -0800 (PST) Date: Thu, 16 Feb 2017 20:08:02 -0700 From: Ross Zwisler Subject: Re: [PATCH v2 2/2] mm: validate device_hotplug is held for memory hotplug Message-ID: <20170217030802.GA27382@linux.intel.com> References: <148728202805.38457.18028105614854319884.stgit@dwillia2-desk3.amr.corp.intel.com> <148728203880.38457.1158394701925100383.stgit@dwillia2-desk3.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <148728203880.38457.1158394701925100383.stgit@dwillia2-desk3.amr.corp.intel.com> Sender: owner-linux-mm@kvack.org List-ID: To: Dan Williams Cc: akpm@linux-foundation.org, Michal Hocko , linux-nvdimm@lists.01.org, Greg Kroah-Hartman , linux-mm@kvack.org, 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 > Cc: Toshi Kani > Cc: Vlastimil Babka > Cc: Logan Gunthorpe > Cc: Greg Kroah-Hartman > Reported-by: Ben Hutchings > Signed-off-by: Dan Williams > --- > 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot0-f200.google.com (mail-ot0-f200.google.com [74.125.82.200]) by kanga.kvack.org (Postfix) with ESMTP id A6FCF681021 for ; Thu, 16 Feb 2017 22:28:59 -0500 (EST) Received: by mail-ot0-f200.google.com with SMTP id s36so40843685otd.3 for ; Thu, 16 Feb 2017 19:28:59 -0800 (PST) Received: from mail-oi0-x230.google.com (mail-oi0-x230.google.com. [2607:f8b0:4003:c06::230]) by mx.google.com with ESMTPS id d21si466440oig.82.2017.02.16.19.28.58 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 Feb 2017 19:28:58 -0800 (PST) Received: by mail-oi0-x230.google.com with SMTP id u143so18787067oif.3 for ; Thu, 16 Feb 2017 19:28:58 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20170217030802.GA27382@linux.intel.com> References: <148728202805.38457.18028105614854319884.stgit@dwillia2-desk3.amr.corp.intel.com> <148728203880.38457.1158394701925100383.stgit@dwillia2-desk3.amr.corp.intel.com> <20170217030802.GA27382@linux.intel.com> From: Dan Williams Date: Thu, 16 Feb 2017 19:28:57 -0800 Message-ID: Subject: Re: [PATCH v2 2/2] mm: validate device_hotplug is held for memory hotplug Content-Type: text/plain; charset=UTF-8 Sender: owner-linux-mm@kvack.org List-ID: To: Ross Zwisler Cc: Andrew Morton , Michal Hocko , "linux-nvdimm@lists.01.org" , Greg Kroah-Hartman , Linux MM , Ben Hutchings , Vlastimil Babka On Thu, Feb 16, 2017 at 7:08 PM, Ross Zwisler 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 >> Cc: Toshi Kani >> Cc: Vlastimil Babka >> Cc: Logan Gunthorpe >> Cc: Greg Kroah-Hartman >> Reported-by: Ben Hutchings >> Signed-off-by: Dan Williams >> --- >> 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: email@kvack.org