Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756208AbbHZVjX (ORCPT ); Wed, 26 Aug 2015 17:39:23 -0400 Received: from mga11.intel.com ([192.55.52.93]:3879 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751483AbbHZVjW (ORCPT ); Wed, 26 Aug 2015 17:39:22 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,418,1437462000"; d="scan'208";a="791604311" From: "Williams, Dan J" To: "hch@lst.de" CC: "linux-kernel@vger.kernel.org" , "mingo@kernel.org" , "linux-mm@kvack.org" , "hpa@zytor.com" , "linux-nvdimm@lists.01.org" , "ross.zwisler@linux.intel.com" , "boaz@plexistor.com" , "david@fromorbit.com" Subject: Re: [PATCH v2 9/9] devm_memremap_pages: protect against pmem device unbind Thread-Topic: [PATCH v2 9/9] devm_memremap_pages: protect against pmem device unbind Thread-Index: AQHQ4EepD3tiuKjWvUOyuCg/ryennA== Date: Wed, 26 Aug 2015 21:39:18 +0000 Message-ID: <1440625157.31365.21.camel@intel.com> References: <20150826010220.8851.18077.stgit@dwillia2-desk3.amr.corp.intel.com> <20150826012813.8851.35482.stgit@dwillia2-desk3.amr.corp.intel.com> <20150826124649.GA8014@lst.de> In-Reply-To: <20150826124649.GA8014@lst.de> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.22.254.138] Content-Type: text/plain; charset="utf-8" Content-ID: <71F1E1F799720F42A2A7D815EACA3C4A@intel.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id t7QLdT9t027908 Content-Length: 7359 Lines: 251 On Wed, 2015-08-26 at 14:46 +0200, Christoph Hellwig wrote: > On Tue, Aug 25, 2015 at 09:28:13PM -0400, Dan Williams wrote: > > Given that: > > > > 1/ device ->remove() can not be failed > > > > 2/ a pmem device may be unbound at any time > > > > 3/ we do not know what other parts of the kernel are actively using a > > 'struct page' from devm_memremap_pages() > > > > ...provide a facility for active usages of device memory to block pmem > > device unbind. With a percpu_ref it should be feasible to take a > > reference on a per-I/O or other high frequency basis. > > Without a caller of get_page_map this is just adding dead code. I'd > suggest to group it in a series with that caller. > Agreed, we can drop this until the first user arrives. > Also if the page_map gets exposed in a header the name is a bit too generic. > memremap_map maybe? Done, and in the patch below I hide the internal implementation details of page_map in kernel/memremap.c and only expose the percpu_ref in the public memremap_map. 8<---- Subject: devm_memremap_pages: protect against pmem device unbind From: Dan Williams Given that: 1/ device ->remove() can not be failed 2/ a pmem device may be unbound at any time 3/ we do not know what other parts of the kernel are actively using a 'struct page' from devm_memremap_pages() ...provide a facility for active usages of device memory to block pmem device unbind. With a percpu_ref it should be feasible to take a reference on a per-I/O or other high frequency basis. Cc: Christoph Hellwig Signed-off-by: Dan Williams --- include/linux/io.h | 30 ++++++++++++++++ kernel/memremap.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 125 insertions(+), 1 deletion(-) diff --git a/include/linux/io.h b/include/linux/io.h index de64c1e53612..9e696b114c6d 100644 --- a/include/linux/io.h +++ b/include/linux/io.h @@ -90,8 +90,25 @@ void devm_memunmap(struct device *dev, void *addr); void *__devm_memremap_pages(struct device *dev, struct resource *res); #ifdef CONFIG_ZONE_DEVICE +#include + +struct memremap_map { + struct percpu_ref percpu_ref; +}; + void *devm_memremap_pages(struct device *dev, struct resource *res); +struct memremap_map * __must_check get_memremap_map(resource_size_t addr); +static inline void ref_memremap_map(struct memremap_map *memremap_map) +{ + percpu_ref_get(&memremap_map->percpu_ref); +} + +static inline void put_memremap_map(struct memremap_map *memremap_map) +{ + percpu_ref_put(&memremap_map->percpu_ref); +} #else +struct memremap_map; static inline void *devm_memremap_pages(struct device *dev, struct resource *res) { /* @@ -102,6 +119,19 @@ static inline void *devm_memremap_pages(struct device *dev, struct resource *res WARN_ON_ONCE(1); return ERR_PTR(-ENXIO); } + +static inline __must_check struct memremap_map *get_memremap_map(resource_size_t addr) +{ + return NULL; +} + +static inline void ref_memremap_map(struct memremap_map *memremap_map) +{ +} + +static inline void put_memremap_map(struct memremap_map *memremap_map) +{ +} #endif /* diff --git a/kernel/memremap.c b/kernel/memremap.c index 72b0c66628b6..5b9f04789b96 100644 --- a/kernel/memremap.c +++ b/kernel/memremap.c @@ -11,7 +11,11 @@ * General Public License for more details. */ #include +#include #include +#include +#include +#include #include #include #include @@ -138,14 +142,74 @@ void devm_memunmap(struct device *dev, void *addr) EXPORT_SYMBOL(devm_memunmap); #ifdef CONFIG_ZONE_DEVICE +static DEFINE_MUTEX(page_map_lock); +static DECLARE_WAIT_QUEUE_HEAD(page_map_wait); +static LIST_HEAD(page_maps); + +enum { + PAGE_MAP_LIVE, + PAGE_MAP_CONFIRM, +}; + struct page_map { struct resource res; + struct list_head list; + unsigned long flags; + struct memremap_map map; + struct device *dev; }; +static struct page_map *to_page_map(struct percpu_ref *ref) +{ + return container_of(ref, struct page_map, map.percpu_ref); +} + +static void page_map_release(struct percpu_ref *ref) +{ + struct page_map *page_map = to_page_map(ref); + + /* signal page_map is idle (no more refs) */ + clear_bit(PAGE_MAP_LIVE, &page_map->flags); + wake_up_all(&page_map_wait); +} + +static void page_map_confirm(struct percpu_ref *ref) +{ + struct page_map *page_map = to_page_map(ref); + + /* signal page_map is confirmed dead (slow path ref mode) */ + set_bit(PAGE_MAP_CONFIRM, &page_map->flags); + wake_up_all(&page_map_wait); +} + +static void page_map_destroy(struct page_map *page_map) +{ + long tmo; + + /* flush new lookups */ + mutex_lock(&page_map_lock); + list_del_rcu(&page_map->list); + mutex_unlock(&page_map_lock); + synchronize_rcu(); + + percpu_ref_kill_and_confirm(&page_map->map.percpu_ref, page_map_confirm); + do { + tmo = wait_event_interruptible_timeout(page_map_wait, + !test_bit(PAGE_MAP_LIVE, &page_map->flags) + && test_bit(PAGE_MAP_CONFIRM, &page_map->flags), 5*HZ); + if (tmo <= 0) + dev_dbg(page_map->dev, + "page map active, continuing to wait...\n"); + } while (tmo <= 0); +} + static void devm_memremap_pages_release(struct device *dev, void *res) { struct page_map *page_map = res; + if (test_bit(PAGE_MAP_LIVE, &page_map->flags)) + page_map_destroy(page_map); + /* pages are dead and unused, undo the arch mapping */ arch_remove_memory(page_map->res.start, resource_size(&page_map->res)); } @@ -155,7 +219,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res) int is_ram = region_intersects(res->start, resource_size(res), "System RAM"); struct page_map *page_map; - int error, nid; + int error, nid, rc; if (is_ram == REGION_MIXED) { WARN_ONCE(1, "%s attempted on mixed region %pr\n", @@ -172,6 +236,12 @@ void *devm_memremap_pages(struct device *dev, struct resource *res) return ERR_PTR(-ENOMEM); memcpy(&page_map->res, res, sizeof(*res)); + INIT_LIST_HEAD(&page_map->list); + page_map->dev = dev; + rc = percpu_ref_init(&page_map->map.percpu_ref, page_map_release, 0, + GFP_KERNEL); + if (rc) + return ERR_PTR(rc); nid = dev_to_node(dev); if (nid < 0) @@ -183,8 +253,32 @@ void *devm_memremap_pages(struct device *dev, struct resource *res) return ERR_PTR(error); } + set_bit(PAGE_MAP_LIVE, &page_map->flags); + mutex_lock(&page_map_lock); + list_add_rcu(&page_map->list, &page_maps); + mutex_unlock(&page_map_lock); + devres_add(dev, page_map); return __va(res->start); } EXPORT_SYMBOL(devm_memremap_pages); + +struct memremap_map * __must_check get_memremap_map(resource_size_t addr) +{ + struct memremap_map *ret = NULL; + struct page_map *page_map; + + rcu_read_lock(); + list_for_each_entry_rcu(page_map, &page_maps, list) { + if (addr >= page_map->res.start && addr <= page_map->res.end) { + if (percpu_ref_tryget(&page_map->map.percpu_ref)) + ret = &page_map->map; + break; + } + } + rcu_read_unlock(); + + return ret; +} +EXPORT_SYMBOL_GPL(get_memremap_map); #endif /* CONFIG_ZONE_DEVICE */ ????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?