Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Frederic Barrat <fbarrat@linux.ibm.com>
To: "Alastair D'Silva" <alastair@au1.ibm.com>, alastair@d-silva.org
Cc: "Benjamin Herrenschmidt" <benh@kernel.crashing.org>,
	"Paul Mackerras" <paulus@samba.org>,
	"Michael Ellerman" <mpe@ellerman.id.au>,
	"Andrew Donnellan" <ajd@linux.ibm.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Vishal Verma" <vishal.l.verma@intel.com>,
	"Dave Jiang" <dave.jiang@intel.com>,
	"Keith Busch" <keith.busch@intel.com>,
	"Ira Weiny" <ira.weiny@intel.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Mauro Carvalho Chehab" <mchehab+samsung@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	"Rob Herring" <robh@kernel.org>,
	"Anton Blanchard" <anton@ozlabs.org>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"Mahesh Salgaonkar" <mahesh@linux.vnet.ibm.com>,
	"Madhavan Srinivasan" <maddy@linux.vnet.ibm.com>,
	"Cédric Le Goater" <clg@kaod.org>,
	"Anju T Sudhakar" <anju@linux.vnet.ibm.com>,
	"Hari Bathini" <hbathini@linux.ibm.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Greg Kurz" <groug@kaod.org>,
	"Nicholas Piggin" <npiggin@gmail.com>,
	"Masahiro Yamada" <yamada.masahiro@socionext.com>,
	"Alexey Kardashevskiy" <aik@ozlabs.ru>,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-nvdimm@lists.01.org, linux-mm@kvack.org
Subject: Re: [PATCH v2 09/27] ocxl: Free detached contexts in ocxl_context_detach_all()
Date: Thu, 9 Jan 2020 15:54:35 +0100
Message-ID: <4c9da9a0-55f4-6cf4-53b8-e8a69744bf98@linux.ibm.com> (raw)
In-Reply-To: <20191203034655.51561-10-alastair@au1.ibm.com>



Le 03/12/2019 à 04:46, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> ocxl_context_detach_all() is called from ocxl_function_close(), so
> there is no reason to leave the contexts allocated, as the caller
> can do nothing useful with them at that point.
> 
> This also has the side-effect of freeing any allocated IRQs
> within the context.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---


I think this is wrong and probably unneeded. In ocxl (and I would assume 
most drivers), we separate pretty clearly what is setup by the driver 
framework when a device is probed, and what is allocated by the users 
(userland or scm). Contexts are allocated by the users. So they should 
be freed by them only. That separation is also why we have some 
reference counting on the afu and function structs, to make sure the 
core data remains valid for as long as required.
Though it's a bit asking for troubles, it can be seen when unbinding a 
function from the driver through sysfs. That will end up calling 
ocxl_function_close() and therefore ocxl_context_detach_all(). However 
it's possible for a user process to still have a file descriptor opened. 
The context is detached and marked as CLOSED, so any interaction with it 
from the user will fail, but it should still be allocated so that it is 
valid if the user process makes a system call to the driver. The context 
will be freed when the file descriptor is closed.
I don't think this is needed for scm either, since you've now added the 
context detach and free call in free_scm()
I would just drop this patch.

   Fred



>   drivers/misc/ocxl/context.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
> index 994563a078eb..6cb36ef96e09 100644
> --- a/drivers/misc/ocxl/context.c
> +++ b/drivers/misc/ocxl/context.c
> @@ -259,10 +259,11 @@ void ocxl_context_detach_all(struct ocxl_afu *afu)
>   {
>   	struct ocxl_context *ctx;
>   	int tmp;
> +	int rc;
>   
>   	mutex_lock(&afu->contexts_lock);
>   	idr_for_each_entry(&afu->contexts_idr, ctx, tmp) {
> -		ocxl_context_detach(ctx);
> +		rc = ocxl_context_detach(ctx);
>   		/*
>   		 * We are force detaching - remove any active mmio
>   		 * mappings so userspace cannot interfere with the
> @@ -274,6 +275,9 @@ void ocxl_context_detach_all(struct ocxl_afu *afu)
>   		if (ctx->mapping)
>   			unmap_mapping_range(ctx->mapping, 0, 0, 1);
>   		mutex_unlock(&ctx->mapping_lock);
> +
> +		if (rc != -EBUSY)
> +			ocxl_context_free(ctx);
>   	}
>   	mutex_unlock(&afu->contexts_lock);
>   }
> 



  reply index

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-03  3:46 [PATCH v2 00/27] Add support for OpenCAPI SCM devices Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 01/27] memory_hotplug: Add a bounds check to __add_pages Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 02/27] nvdimm: remove prototypes for nonexistent functions Alastair D'Silva
2019-12-03  4:47   ` Andrew Donnellan
2019-12-04  0:10   ` Dan Williams
2020-01-23 21:49     ` Dan Williams
2019-12-03  3:46 ` [PATCH v2 03/27] powerpc: Add OPAL calls for LPC memory alloc/release Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 04/27] mm/memory_hotplug: Allow check_hotplug_memory_addressable to be called from drivers Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 05/27] powerpc: Map & release OpenCAPI LPC memory Alastair D'Silva
2020-01-09 14:41   ` Frederic Barrat
2020-01-21  6:46   ` Andrew Donnellan
2020-01-21  7:11     ` Greg Kurz
2020-02-14 11:09   ` Frederic Barrat
2020-02-18 23:44     ` Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 06/27] ocxl: Tally up the LPC memory on a link & allow it to be mapped Alastair D'Silva
2020-01-09 14:48   ` Frederic Barrat
2020-02-03 12:37   ` Jonathan Cameron
2020-02-19  0:01     ` Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 07/27] ocxl: Add functions to map/unmap LPC memory Alastair D'Silva
2020-01-09 14:49   ` Frederic Barrat
2020-02-03 12:49   ` Jonathan Cameron
2020-02-19  2:39     ` Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 08/27] ocxl: Save the device serial number in ocxl_fn Alastair D'Silva
2020-02-03 12:53   ` Jonathan Cameron
2020-02-19  4:03     ` Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 09/27] ocxl: Free detached contexts in ocxl_context_detach_all() Alastair D'Silva
2020-01-09 14:54   ` Frederic Barrat [this message]
2019-12-03  3:46 ` [PATCH v2 10/27] nvdimm: Add driver for OpenCAPI Storage Class Memory Alastair D'Silva
2019-12-03  5:05   ` Alastair D'Silva
2020-02-03 13:20   ` Jonathan Cameron
2020-02-19  4:40     ` Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 11/27] nvdimm/ocxl: Add register addresses & status values to header Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 12/27] nvdimm/ocxl: Read the capability registers & wait for device ready Alastair D'Silva
2020-02-03 13:23   ` Jonathan Cameron
2020-02-19  4:46     ` Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 13/27] nvdimm/ocxl: Add support for Admin commands Alastair D'Silva
2020-02-03 14:18   ` Jonathan Cameron
2020-02-19  5:00     ` Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 14/27] nvdimm/ocxl: Add support for near storage commands Alastair D'Silva
2020-02-03 14:22   ` Jonathan Cameron
2020-02-19  4:54     ` Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 15/27] nvdimm/ocxl: Register a character device for userspace to interact with Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 16/27] nvdimm/ocxl: Implement the Read Error Log command Alastair D'Silva
2019-12-05  3:42   ` Alastair D'Silva
2019-12-05 19:34   ` kbuild test robot
2019-12-03  3:46 ` [PATCH v2 17/27] nvdimm/ocxl: Add controller dump IOCTLs Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 18/27] nvdimm/ocxl: Add an IOCTL to report controller statistics Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 19/27] nvdimm/ocxl: Forward events to userspace Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 20/27] nvdimm/ocxl: Add an IOCTL to request controller health & perf data Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 21/27] nvdimm/ocxl: Support firmware update via sysfs Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 22/27] nvdimm/ocxl: Implement the heartbeat command Alastair D'Silva
2020-02-03 15:11   ` Jonathan Cameron
2020-02-19  5:02     ` Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 23/27] nvdimm/ocxl: Add debug IOCTLs Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 24/27] nvdimm/ocxl: Implement Overwrite Alastair D'Silva
2020-02-03 15:10   ` Jonathan Cameron
2020-02-19  5:13     ` Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 25/27] nvdimm/ocxl: Expose SMART data via ndctl Alastair D'Silva
2019-12-16  0:15   ` Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 26/27] powerpc: Enable OpenCAPI Storage Class Memory driver on bare metal Alastair D'Silva
2019-12-03  4:54   ` Andrew Donnellan
2019-12-03  4:57     ` Alastair D'Silva
2019-12-03  3:46 ` [PATCH v2 27/27] MAINTAINERS: Add myself & nvdimm/ocxl to ocxl Alastair D'Silva
2019-12-03  3:50 ` [PATCH v2 00/27] Add support for OpenCAPI SCM devices Matthew Wilcox
2019-12-03  4:01   ` Alastair D'Silva
2019-12-03 12:42     ` Matthew Wilcox
2019-12-04  0:15       ` Dan Williams

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4c9da9a0-55f4-6cf4-53b8-e8a69744bf98@linux.ibm.com \
    --to=fbarrat@linux.ibm.com \
    --cc=aik@ozlabs.ru \
    --cc=ajd@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=alastair@au1.ibm.com \
    --cc=alastair@d-silva.org \
    --cc=anju@linux.vnet.ibm.com \
    --cc=anton@ozlabs.org \
    --cc=arnd@arndb.de \
    --cc=benh@kernel.crashing.org \
    --cc=clg@kaod.org \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=davem@davemloft.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=groug@kaod.org \
    --cc=hbathini@linux.ibm.com \
    --cc=ira.weiny@intel.com \
    --cc=keith.busch@intel.com \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.vnet.ibm.com \
    --cc=mahesh@linux.vnet.ibm.com \
    --cc=mchehab+samsung@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=paulus@samba.org \
    --cc=robh@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=vishal.l.verma@intel.com \
    --cc=yamada.masahiro@socionext.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git