From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.0 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 552A8C4727E for ; Wed, 30 Sep 2020 17:28:42 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A8B4820706 for ; Wed, 30 Sep 2020 17:28:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=intel-com.20150623.gappssmtp.com header.i=@intel-com.20150623.gappssmtp.com header.b="zmyxtM71" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A8B4820706 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id D28568E0003; Wed, 30 Sep 2020 13:28:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CDA6F8E0001; Wed, 30 Sep 2020 13:28:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BF04F8E0003; Wed, 30 Sep 2020 13:28:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0073.hostedemail.com [216.40.44.73]) by kanga.kvack.org (Postfix) with ESMTP id AA2F88E0001 for ; Wed, 30 Sep 2020 13:28:40 -0400 (EDT) Received: from smtpin02.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 6B9C28249980 for ; Wed, 30 Sep 2020 17:28:40 +0000 (UTC) X-FDA: 77320412400.02.frame02_0711f1d27194 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin02.hostedemail.com (Postfix) with ESMTP id 48678101DAF2A for ; Wed, 30 Sep 2020 17:28:40 +0000 (UTC) X-HE-Tag: frame02_0711f1d27194 X-Filterd-Recvd-Size: 8768 Received: from mail-ej1-f66.google.com (mail-ej1-f66.google.com [209.85.218.66]) by imf41.hostedemail.com (Postfix) with ESMTP for ; Wed, 30 Sep 2020 17:28:39 +0000 (UTC) Received: by mail-ej1-f66.google.com with SMTP id z23so3963090ejr.13 for ; Wed, 30 Sep 2020 10:28:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=K7sPk6Sik27AWrREdtUYnYe92i3vslb/zKKVkOgBVaQ=; b=zmyxtM718bJxbCH+GLRAIs/H47BTp2R1awvT53QplORdnHGWVeJTQdK9fdRVtvBv5W ZZFMGzmU2zd4bdTHGfpR4tVDd1Y0Otit4r7u3R2D/10aTTTl9ESuMHc7s8OzpDsc1AmQ 7KFl9Ey9DKu3G0yrF0FT/zZ+60uBSEAJcZeq0yQ67A6v6Hw9k7bWJHF1ay4LU+UHikkk +6hYpTQSopFGY9z8J2KLxvVqdRC9FZS2YYvVRND4NXtYyWogwO3aEEB9ydcWm6G7YAY+ anaXgr8WTNEHkZfbgg/UocgrBOEELE7O0mBzxih/A2gUQEIuX1xMkxLYYueDwJzCOT7Y fYHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=K7sPk6Sik27AWrREdtUYnYe92i3vslb/zKKVkOgBVaQ=; b=lstcD7C5OExQtFsmViHY9tIk2/ExRolhE0lkTnt0HaaXhk6/Qxge6u4cR2GebS+nix 5tonjgpmfPk5xQtb8qb5SQkz8ZYxiyUB+hVCSXeCwdI0OPWueWDPGQSTUJRnkq9WwEHt EowXAhgjI/f96VoXrXqkSdw9YleexazzeancbwYEVS4s1miNR4Olm05U6VqDdd1h1i2i ZNfaoKca5BDZ7fQgYwZ3VYytVQVzx532b1Ut9zllZ4kuXTrEu/9pxwEQkzMa5ftAHi8e jgNMaUWG2aMFLBKMeZ94rn5XRaJEe7E4tWsT+kxcAG2t4NIWYZ7KW3T1xixlu5Xv+5bk Ldng== X-Gm-Message-State: AOAM532UWVKHQL8BMZ9rjlNJkjFkbRORhr6jnOMEvWpuAvfRK85tcmWP 6RDmFN1mxk3Zppz0IHdv6seY8AnkN8U5k4bxjUl32Q== X-Google-Smtp-Source: ABdhPJwHTh9jNRUe26y/9bRenkD0CrDNpv/b4M+zhuDvrSCF8so++nqTlmejCH4CxF24yboF7SB9F7SGef38jSfloJI= X-Received: by 2002:a17:906:2354:: with SMTP id m20mr3803912eja.341.1601486917942; Wed, 30 Sep 2020 10:28:37 -0700 (PDT) MIME-Version: 1.0 References: <160106109960.30709.7379926726669669398.stgit@dwillia2-desk3.amr.corp.intel.com> <160106112239.30709.15909567572288425294.stgit@dwillia2-desk3.amr.corp.intel.com> <86f450e7-d1a7-3d82-b486-afd6682c5942@redhat.com> In-Reply-To: <86f450e7-d1a7-3d82-b486-afd6682c5942@redhat.com> From: Dan Williams Date: Wed, 30 Sep 2020 10:28:26 -0700 Message-ID: Subject: Re: [PATCH v5 04/17] device-dax/kmem: replace release_resource() with release_mem_region() To: David Hildenbrand Cc: Andrew Morton , Vishal Verma , Dave Hansen , Pavel Tatashin , Brice Goglin , Dave Jiang , Ira Weiny , Jia He , Joao Martins , Jonathan Cameron , Linux MM , linux-nvdimm , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Sep 30, 2020 at 9:23 AM David Hildenbrand wrote: > > On 25.09.20 21:12, Dan Williams wrote: > > Towards removing the mode specific @dax_kmem_res attribute from the > > generic 'struct dev_dax', and preparing for multi-range support, change > > the kmem driver to use the idiomatic release_mem_region() to pair with > > the initial request_mem_region(). This also eliminates the need to open > > code the release of the resource allocated by request_mem_region(). > > > > As there are no more dax_kmem_res users, delete this struct member. > > > > Cc: David Hildenbrand > > Cc: Vishal Verma > > Cc: Dave Hansen > > Cc: Pavel Tatashin > > Cc: Brice Goglin > > Cc: Dave Jiang > > Cc: David Hildenbrand > > Cc: Ira Weiny > > Cc: Jia He > > Cc: Joao Martins > > Cc: Jonathan Cameron > > Signed-off-by: Dan Williams > > --- > > drivers/dax/dax-private.h | 3 --- > > drivers/dax/kmem.c | 20 +++++++------------- > > 2 files changed, 7 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h > > index 6779f683671d..12a2dbc43b40 100644 > > --- a/drivers/dax/dax-private.h > > +++ b/drivers/dax/dax-private.h > > @@ -42,8 +42,6 @@ struct dax_region { > > * @dev - device core > > * @pgmap - pgmap for memmap setup / lifetime (driver owned) > > * @range: resource range for the instance > > - * @dax_mem_res: physical address range of hotadded DAX memory > > - * @dax_mem_name: name for hotadded DAX memory via add_memory_driver_managed() > > */ > > struct dev_dax { > > struct dax_region *region; > > @@ -52,7 +50,6 @@ struct dev_dax { > > struct device dev; > > struct dev_pagemap *pgmap; > > struct range range; > > - struct resource *dax_kmem_res; > > }; > > > > static inline u64 range_len(struct range *range) > > diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c > > index 6fe2cb1c5f7c..e56fc688bdc5 100644 > > --- a/drivers/dax/kmem.c > > +++ b/drivers/dax/kmem.c > > @@ -33,7 +33,7 @@ int dev_dax_kmem_probe(struct device *dev) > > { > > struct dev_dax *dev_dax = to_dev_dax(dev); > > struct range range = dax_kmem_range(dev_dax); > > - struct resource *new_res; > > + struct resource *res; > > char *res_name; > > int numa_node; > > int rc; > > @@ -56,8 +56,8 @@ int dev_dax_kmem_probe(struct device *dev) > > return -ENOMEM; > > > > /* Region is permanently reserved if hotremove fails. */ > > - new_res = request_mem_region(range.start, range_len(&range), res_name); > > - if (!new_res) { > > + res = request_mem_region(range.start, range_len(&range), res_name); > > + if (!res) { > > dev_warn(dev, "could not reserve region [%#llx-%#llx]\n", range.start, range.end); > > kfree(res_name); > > return -EBUSY; > > @@ -69,23 +69,20 @@ int dev_dax_kmem_probe(struct device *dev) > > * inherit flags from the parent since it may set new flags > > * unknown to us that will break add_memory() below. > > */ > > - new_res->flags = IORESOURCE_SYSTEM_RAM; > > + res->flags = IORESOURCE_SYSTEM_RAM; > > > > /* > > * Ensure that future kexec'd kernels will not treat this as RAM > > * automatically. > > */ > > - rc = add_memory_driver_managed(numa_node, new_res->start, > > - resource_size(new_res), kmem_name); > > + rc = add_memory_driver_managed(numa_node, range.start, range_len(&range), kmem_name); > > if (rc) { > > - release_resource(new_res); > > - kfree(new_res); > > + release_mem_region(range.start, range_len(&range)); > > kfree(res_name); > > return rc; > > } > > > > dev_set_drvdata(dev, res_name); > > - dev_dax->dax_kmem_res = new_res; > > > > return 0; > > } > > @@ -95,7 +92,6 @@ static int dev_dax_kmem_remove(struct device *dev) > > { > > struct dev_dax *dev_dax = to_dev_dax(dev); > > struct range range = dax_kmem_range(dev_dax); > > - struct resource *res = dev_dax->dax_kmem_res; > > const char *res_name = dev_get_drvdata(dev); > > int rc; > > > > @@ -114,10 +110,8 @@ static int dev_dax_kmem_remove(struct device *dev) > > } > > > > /* Release and free dax resources */ > > - release_resource(res); > > - kfree(res); > > + release_mem_region(range.start, range_len(&range)); > > Does that work? AFAIKs, > > __release_region(&iomem_resource, (start), (n)) -> __release_region() > > will only remove stuff that is IORESOURCE_BUSY. Oh, crud, indeed. More fallout from futzing with the flags. > Maybe storing it in drvdata is indeed the easiest way to remove it from > struct dax_region. My first choice is still to get the driver out of the layering violation of touching resource flags, but we don't seem to be converging on a way forward there. So, stuffing more tracking into drvdata (especially for the multi-range case) is the next best option.