[PATCH AUTOSEL 5.14 03/25] cxl: Move cxl_core to new directory

[PATCH AUTOSEL 5.14 03/25] cxl: Move cxl_core to new directory

[Sasha Levin]

From: Ben Widawsky <ben.widawsky@intel.com>

[ Upstream commit 5161a55c069f53d88da49274cbef6e3c74eadea9 ]

CXL core is growing, and it's already arguably unmanageable. To support
future growth, move core functionality to a new directory and rename the
file to represent just bus support. Future work will remove non-bus
functionality.

Note that mem.h is renamed to cxlmem.h to avoid a namespace collision
with the global ARCH=um mem.h header.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://lore.kernel.org/r/162792537866.368511.8915631504621088321.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 Documentation/driver-api/cxl/memory-devices.rst | 2 +-
 drivers/cxl/Makefile                            | 4 +---
 drivers/cxl/core/Makefile                       | 5 +++++
 drivers/cxl/{core.c => core/bus.c}              | 4 ++--
 drivers/cxl/{mem.h => cxlmem.h}                 | 0
 drivers/cxl/pci.c                               | 2 +-
 drivers/cxl/pmem.c                              | 2 +-
 7 files changed, 11 insertions(+), 8 deletions(-)
 create mode 100644 drivers/cxl/core/Makefile
 rename drivers/cxl/{core.c => core/bus.c} (99%)
 rename drivers/cxl/{mem.h => cxlmem.h} (100%)

diff --git a/Documentation/driver-api/cxl/memory-devices.rst b/Documentation/driver-api/cxl/memory-devices.rst
index 487ce4f41d77..a86e2c7c551a 100644
--- a/Documentation/driver-api/cxl/memory-devices.rst
+++ b/Documentation/driver-api/cxl/memory-devices.rst
@@ -36,7 +36,7 @@ CXL Core
 .. kernel-doc:: drivers/cxl/cxl.h
    :internal:
 
-.. kernel-doc:: drivers/cxl/core.c
+.. kernel-doc:: drivers/cxl/core/bus.c
    :doc: cxl core
 
 External Interfaces
diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile
index 32954059b37b..d1aaabc940f3 100644
--- a/drivers/cxl/Makefile
+++ b/drivers/cxl/Makefile
@@ -1,11 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_CXL_BUS) += cxl_core.o
+obj-$(CONFIG_CXL_BUS) += core/
 obj-$(CONFIG_CXL_MEM) += cxl_pci.o
 obj-$(CONFIG_CXL_ACPI) += cxl_acpi.o
 obj-$(CONFIG_CXL_PMEM) += cxl_pmem.o
 
-ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=CXL
-cxl_core-y := core.o
 cxl_pci-y := pci.o
 cxl_acpi-y := acpi.o
 cxl_pmem-y := pmem.o
diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
new file mode 100644
index 000000000000..ad137f96e5c8
--- /dev/null
+++ b/drivers/cxl/core/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_CXL_BUS) += cxl_core.o
+
+ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=CXL -I$(srctree)/drivers/cxl
+cxl_core-y := bus.o
diff --git a/drivers/cxl/core.c b/drivers/cxl/core/bus.c
similarity index 99%
rename from drivers/cxl/core.c
rename to drivers/cxl/core/bus.c
index a2e4d54fc7bc..0815eec23944 100644
--- a/drivers/cxl/core.c
+++ b/drivers/cxl/core/bus.c
@@ -6,8 +6,8 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <linux/idr.h>
-#include "cxl.h"
-#include "mem.h"
+#include <cxlmem.h>
+#include <cxl.h>
 
 /**
  * DOC: cxl core
diff --git a/drivers/cxl/mem.h b/drivers/cxl/cxlmem.h
similarity index 100%
rename from drivers/cxl/mem.h
rename to drivers/cxl/cxlmem.h
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 4cf351a3cf99..a945c5fda292 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -12,9 +12,9 @@
 #include <linux/pci.h>
 #include <linux/io.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
+#include "cxlmem.h"
 #include "pci.h"
 #include "cxl.h"
-#include "mem.h"
 
 /**
  * DOC: cxl pci
diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
index 0088e41dd2f3..9652c3ee41e7 100644
--- a/drivers/cxl/pmem.c
+++ b/drivers/cxl/pmem.c
@@ -6,7 +6,7 @@
 #include <linux/ndctl.h>
 #include <linux/async.h>
 #include <linux/slab.h>
-#include "mem.h"
+#include "cxlmem.h"
 #include "cxl.h"
 
 /*
-- 
2.30.2

[PATCH AUTOSEL 5.14 04/25] cxl/pci: Introduce cdevm_file_operations

[Sasha Levin]

From: Dan Williams <dan.j.williams@intel.com>

[ Upstream commit 9cc238c7a526dba9ee8c210fa2828886fc65db66 ]

In preparation for moving cxl_memdev allocation to the core, introduce
cdevm_file_operations to coordinate file operations shutdown relative to
driver data release.

The motivation for moving cxl_memdev allocation to the core (beyond
better file organization of sysfs attributes in core/ and drivers in
cxl/), is that device lifetime is longer than module lifetime. The cxl_pci
module should be free to come and go without needing to coordinate with
devices that need the text associated with cxl_memdev_release() to stay
resident. The move will fix a use after free bug when looping driver
load / unload with CONFIG_DEBUG_KOBJECT_RELEASE=y.

Another motivation for passing in file_operations to the core cxl_memdev
creation flow is to allow for alternate drivers, like unit test code, to
define their own ioctl backends.

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://lore.kernel.org/r/162792539962.368511.2962268954245340288.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/cxl/cxlmem.h | 15 ++++++++++
 drivers/cxl/pci.c    | 65 ++++++++++++++++++++++++++------------------
 2 files changed, 53 insertions(+), 27 deletions(-)

diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 8f02d02b26b4..0cd463de1342 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -34,6 +34,21 @@
  */
 #define CXL_MEM_MAX_DEVS 65536
 
+/**
+ * struct cdevm_file_operations - devm coordinated cdev file operations
+ * @fops: file operations that are synchronized against @shutdown
+ * @shutdown: disconnect driver data
+ *
+ * @shutdown is invoked in the devres release path to disconnect any
+ * driver instance data from @dev. It assumes synchronization with any
+ * fops operation that requires driver data. After @shutdown an
+ * operation may only reference @device data.
+ */
+struct cdevm_file_operations {
+	struct file_operations fops;
+	void (*shutdown)(struct device *dev);
+};
+
 /**
  * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device
  * @dev: driver core device object
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index a945c5fda292..f7a5ad5e1f4a 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -806,13 +806,30 @@ static int cxl_memdev_release_file(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static const struct file_operations cxl_memdev_fops = {
-	.owner = THIS_MODULE,
-	.unlocked_ioctl = cxl_memdev_ioctl,
-	.open = cxl_memdev_open,
-	.release = cxl_memdev_release_file,
-	.compat_ioctl = compat_ptr_ioctl,
-	.llseek = noop_llseek,
+static struct cxl_memdev *to_cxl_memdev(struct device *dev)
+{
+	return container_of(dev, struct cxl_memdev, dev);
+}
+
+static void cxl_memdev_shutdown(struct device *dev)
+{
+	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+
+	down_write(&cxl_memdev_rwsem);
+	cxlmd->cxlm = NULL;
+	up_write(&cxl_memdev_rwsem);
+}
+
+static const struct cdevm_file_operations cxl_memdev_fops = {
+	.fops = {
+		.owner = THIS_MODULE,
+		.unlocked_ioctl = cxl_memdev_ioctl,
+		.open = cxl_memdev_open,
+		.release = cxl_memdev_release_file,
+		.compat_ioctl = compat_ptr_ioctl,
+		.llseek = noop_llseek,
+	},
+	.shutdown = cxl_memdev_shutdown,
 };
 
 static inline struct cxl_mem_command *cxl_mem_find_command(u16 opcode)
@@ -1161,11 +1178,6 @@ static int cxl_mem_setup_regs(struct cxl_mem *cxlm)
 	return ret;
 }
 
-static struct cxl_memdev *to_cxl_memdev(struct device *dev)
-{
-	return container_of(dev, struct cxl_memdev, dev);
-}
-
 static void cxl_memdev_release(struct device *dev)
 {
 	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
@@ -1281,24 +1293,22 @@ static const struct device_type cxl_memdev_type = {
 	.groups = cxl_memdev_attribute_groups,
 };
 
-static void cxl_memdev_shutdown(struct cxl_memdev *cxlmd)
-{
-	down_write(&cxl_memdev_rwsem);
-	cxlmd->cxlm = NULL;
-	up_write(&cxl_memdev_rwsem);
-}
-
 static void cxl_memdev_unregister(void *_cxlmd)
 {
 	struct cxl_memdev *cxlmd = _cxlmd;
 	struct device *dev = &cxlmd->dev;
+	struct cdev *cdev = &cxlmd->cdev;
+	const struct cdevm_file_operations *cdevm_fops;
+
+	cdevm_fops = container_of(cdev->ops, typeof(*cdevm_fops), fops);
+	cdevm_fops->shutdown(dev);
 
 	cdev_device_del(&cxlmd->cdev, dev);
-	cxl_memdev_shutdown(cxlmd);
 	put_device(dev);
 }
 
-static struct cxl_memdev *cxl_memdev_alloc(struct cxl_mem *cxlm)
+static struct cxl_memdev *cxl_memdev_alloc(struct cxl_mem *cxlm,
+					   const struct file_operations *fops)
 {
 	struct pci_dev *pdev = cxlm->pdev;
 	struct cxl_memdev *cxlmd;
@@ -1324,7 +1334,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_mem *cxlm)
 	device_set_pm_not_required(dev);
 
 	cdev = &cxlmd->cdev;
-	cdev_init(cdev, &cxl_memdev_fops);
+	cdev_init(cdev, fops);
 	return cxlmd;
 
 err:
@@ -1332,15 +1342,16 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_mem *cxlm)
 	return ERR_PTR(rc);
 }
 
-static struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
-					      struct cxl_mem *cxlm)
+static struct cxl_memdev *
+devm_cxl_add_memdev(struct device *host, struct cxl_mem *cxlm,
+		    const struct cdevm_file_operations *cdevm_fops)
 {
 	struct cxl_memdev *cxlmd;
 	struct device *dev;
 	struct cdev *cdev;
 	int rc;
 
-	cxlmd = cxl_memdev_alloc(cxlm);
+	cxlmd = cxl_memdev_alloc(cxlm, &cdevm_fops->fops);
 	if (IS_ERR(cxlmd))
 		return cxlmd;
 
@@ -1370,7 +1381,7 @@ static struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
 	 * The cdev was briefly live, shutdown any ioctl operations that
 	 * saw that state.
 	 */
-	cxl_memdev_shutdown(cxlmd);
+	cdevm_fops->shutdown(dev);
 	put_device(dev);
 	return ERR_PTR(rc);
 }
@@ -1611,7 +1622,7 @@ static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		return rc;
 
-	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlm);
+	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlm, &cxl_memdev_fops);
 	if (IS_ERR(cxlmd))
 		return PTR_ERR(cxlmd);
 
-- 
2.30.2

Re: [PATCH AUTOSEL 5.14 03/25] cxl: Move cxl_core to new directory

[Jonathan Cameron]

On Mon, 13 Sep 2021 18:33:17 -0400
Sasha Levin <sashal@kernel.org> wrote:

> From: Ben Widawsky <ben.widawsky@intel.com>
> 
> [ Upstream commit 5161a55c069f53d88da49274cbef6e3c74eadea9 ]
> 
> CXL core is growing, and it's already arguably unmanageable. To support
> future growth, move core functionality to a new directory and rename the
> file to represent just bus support. Future work will remove non-bus
> functionality.
> 
> Note that mem.h is renamed to cxlmem.h to avoid a namespace collision
> with the global ARCH=um mem.h header.

Not a fix...

I'm guessing this got picked up on the basis of the Reported-by: tag?
I think that was added for a minor tweak as this went through review rather
than referring to the whole patch.

Jonathan


> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Link: https://lore.kernel.org/r/162792537866.368511.8915631504621088321.stgit@dwillia2-desk3.amr.corp.intel.com
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  Documentation/driver-api/cxl/memory-devices.rst | 2 +-
>  drivers/cxl/Makefile                            | 4 +---
>  drivers/cxl/core/Makefile                       | 5 +++++
>  drivers/cxl/{core.c => core/bus.c}              | 4 ++--
>  drivers/cxl/{mem.h => cxlmem.h}                 | 0
>  drivers/cxl/pci.c                               | 2 +-
>  drivers/cxl/pmem.c                              | 2 +-
>  7 files changed, 11 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/cxl/core/Makefile
>  rename drivers/cxl/{core.c => core/bus.c} (99%)
>  rename drivers/cxl/{mem.h => cxlmem.h} (100%)
> 
> diff --git a/Documentation/driver-api/cxl/memory-devices.rst b/Documentation/driver-api/cxl/memory-devices.rst
> index 487ce4f41d77..a86e2c7c551a 100644
> --- a/Documentation/driver-api/cxl/memory-devices.rst
> +++ b/Documentation/driver-api/cxl/memory-devices.rst
> @@ -36,7 +36,7 @@ CXL Core
>  .. kernel-doc:: drivers/cxl/cxl.h
>     :internal:
>  
> -.. kernel-doc:: drivers/cxl/core.c
> +.. kernel-doc:: drivers/cxl/core/bus.c
>     :doc: cxl core
>  
>  External Interfaces
> diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile
> index 32954059b37b..d1aaabc940f3 100644
> --- a/drivers/cxl/Makefile
> +++ b/drivers/cxl/Makefile
> @@ -1,11 +1,9 @@
>  # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_CXL_BUS) += cxl_core.o
> +obj-$(CONFIG_CXL_BUS) += core/
>  obj-$(CONFIG_CXL_MEM) += cxl_pci.o
>  obj-$(CONFIG_CXL_ACPI) += cxl_acpi.o
>  obj-$(CONFIG_CXL_PMEM) += cxl_pmem.o
>  
> -ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=CXL
> -cxl_core-y := core.o
>  cxl_pci-y := pci.o
>  cxl_acpi-y := acpi.o
>  cxl_pmem-y := pmem.o
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> new file mode 100644
> index 000000000000..ad137f96e5c8
> --- /dev/null
> +++ b/drivers/cxl/core/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_CXL_BUS) += cxl_core.o
> +
> +ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=CXL -I$(srctree)/drivers/cxl
> +cxl_core-y := bus.o
> diff --git a/drivers/cxl/core.c b/drivers/cxl/core/bus.c
> similarity index 99%
> rename from drivers/cxl/core.c
> rename to drivers/cxl/core/bus.c
> index a2e4d54fc7bc..0815eec23944 100644
> --- a/drivers/cxl/core.c
> +++ b/drivers/cxl/core/bus.c
> @@ -6,8 +6,8 @@
>  #include <linux/pci.h>
>  #include <linux/slab.h>
>  #include <linux/idr.h>
> -#include "cxl.h"
> -#include "mem.h"
> +#include <cxlmem.h>
> +#include <cxl.h>
>  
>  /**
>   * DOC: cxl core
> diff --git a/drivers/cxl/mem.h b/drivers/cxl/cxlmem.h
> similarity index 100%
> rename from drivers/cxl/mem.h
> rename to drivers/cxl/cxlmem.h
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 4cf351a3cf99..a945c5fda292 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -12,9 +12,9 @@
>  #include <linux/pci.h>
>  #include <linux/io.h>
>  #include <linux/io-64-nonatomic-lo-hi.h>
> +#include "cxlmem.h"
>  #include "pci.h"
>  #include "cxl.h"
> -#include "mem.h"
>  
>  /**
>   * DOC: cxl pci
> diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
> index 0088e41dd2f3..9652c3ee41e7 100644
> --- a/drivers/cxl/pmem.c
> +++ b/drivers/cxl/pmem.c
> @@ -6,7 +6,7 @@
>  #include <linux/ndctl.h>
>  #include <linux/async.h>
>  #include <linux/slab.h>
> -#include "mem.h"
> +#include "cxlmem.h"
>  #include "cxl.h"
>  
>  /*

Re: [PATCH AUTOSEL 5.14 03/25] cxl: Move cxl_core to new directory

[Jonathan Cameron]

On Tue, 14 Sep 2021 09:56:23 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:

> On Mon, 13 Sep 2021 18:33:17 -0400
> Sasha Levin <sashal@kernel.org> wrote:
> 
> > From: Ben Widawsky <ben.widawsky@intel.com>
> > 
> > [ Upstream commit 5161a55c069f53d88da49274cbef6e3c74eadea9 ]
> > 
> > CXL core is growing, and it's already arguably unmanageable. To support
> > future growth, move core functionality to a new directory and rename the
> > file to represent just bus support. Future work will remove non-bus
> > functionality.
> > 
> > Note that mem.h is renamed to cxlmem.h to avoid a namespace collision
> > with the global ARCH=um mem.h header.  
> 
> Not a fix...
> 
> I'm guessing this got picked up on the basis of the Reported-by: tag?
> I think that was added for a minor tweak as this went through review rather
> than referring to the whole patch.
Or possibly because it was a precursor to the fix in the next patch.

Hmm.  Ben, Dan, does it make sense for these two to go into stable?

Jonathan

> 
> Jonathan
> 
> 
> > 
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Link: https://lore.kernel.org/r/162792537866.368511.8915631504621088321.stgit@dwillia2-desk3.amr.corp.intel.com
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > ---
> >  Documentation/driver-api/cxl/memory-devices.rst | 2 +-
> >  drivers/cxl/Makefile                            | 4 +---
> >  drivers/cxl/core/Makefile                       | 5 +++++
> >  drivers/cxl/{core.c => core/bus.c}              | 4 ++--
> >  drivers/cxl/{mem.h => cxlmem.h}                 | 0
> >  drivers/cxl/pci.c                               | 2 +-
> >  drivers/cxl/pmem.c                              | 2 +-
> >  7 files changed, 11 insertions(+), 8 deletions(-)
> >  create mode 100644 drivers/cxl/core/Makefile
> >  rename drivers/cxl/{core.c => core/bus.c} (99%)
> >  rename drivers/cxl/{mem.h => cxlmem.h} (100%)
> > 
> > diff --git a/Documentation/driver-api/cxl/memory-devices.rst b/Documentation/driver-api/cxl/memory-devices.rst
> > index 487ce4f41d77..a86e2c7c551a 100644
> > --- a/Documentation/driver-api/cxl/memory-devices.rst
> > +++ b/Documentation/driver-api/cxl/memory-devices.rst
> > @@ -36,7 +36,7 @@ CXL Core
> >  .. kernel-doc:: drivers/cxl/cxl.h
> >     :internal:
> >  
> > -.. kernel-doc:: drivers/cxl/core.c
> > +.. kernel-doc:: drivers/cxl/core/bus.c
> >     :doc: cxl core
> >  
> >  External Interfaces
> > diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile
> > index 32954059b37b..d1aaabc940f3 100644
> > --- a/drivers/cxl/Makefile
> > +++ b/drivers/cxl/Makefile
> > @@ -1,11 +1,9 @@
> >  # SPDX-License-Identifier: GPL-2.0
> > -obj-$(CONFIG_CXL_BUS) += cxl_core.o
> > +obj-$(CONFIG_CXL_BUS) += core/
> >  obj-$(CONFIG_CXL_MEM) += cxl_pci.o
> >  obj-$(CONFIG_CXL_ACPI) += cxl_acpi.o
> >  obj-$(CONFIG_CXL_PMEM) += cxl_pmem.o
> >  
> > -ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=CXL
> > -cxl_core-y := core.o
> >  cxl_pci-y := pci.o
> >  cxl_acpi-y := acpi.o
> >  cxl_pmem-y := pmem.o
> > diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> > new file mode 100644
> > index 000000000000..ad137f96e5c8
> > --- /dev/null
> > +++ b/drivers/cxl/core/Makefile
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +obj-$(CONFIG_CXL_BUS) += cxl_core.o
> > +
> > +ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=CXL -I$(srctree)/drivers/cxl
> > +cxl_core-y := bus.o
> > diff --git a/drivers/cxl/core.c b/drivers/cxl/core/bus.c
> > similarity index 99%
> > rename from drivers/cxl/core.c
> > rename to drivers/cxl/core/bus.c
> > index a2e4d54fc7bc..0815eec23944 100644
> > --- a/drivers/cxl/core.c
> > +++ b/drivers/cxl/core/bus.c
> > @@ -6,8 +6,8 @@
> >  #include <linux/pci.h>
> >  #include <linux/slab.h>
> >  #include <linux/idr.h>
> > -#include "cxl.h"
> > -#include "mem.h"
> > +#include <cxlmem.h>
> > +#include <cxl.h>
> >  
> >  /**
> >   * DOC: cxl core
> > diff --git a/drivers/cxl/mem.h b/drivers/cxl/cxlmem.h
> > similarity index 100%
> > rename from drivers/cxl/mem.h
> > rename to drivers/cxl/cxlmem.h
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index 4cf351a3cf99..a945c5fda292 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -12,9 +12,9 @@
> >  #include <linux/pci.h>
> >  #include <linux/io.h>
> >  #include <linux/io-64-nonatomic-lo-hi.h>
> > +#include "cxlmem.h"
> >  #include "pci.h"
> >  #include "cxl.h"
> > -#include "mem.h"
> >  
> >  /**
> >   * DOC: cxl pci
> > diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
> > index 0088e41dd2f3..9652c3ee41e7 100644
> > --- a/drivers/cxl/pmem.c
> > +++ b/drivers/cxl/pmem.c
> > @@ -6,7 +6,7 @@
> >  #include <linux/ndctl.h>
> >  #include <linux/async.h>
> >  #include <linux/slab.h>
> > -#include "mem.h"
> > +#include "cxlmem.h"
> >  #include "cxl.h"
> >  
> >  /*  
> 

Re: [PATCH AUTOSEL 5.14 03/25] cxl: Move cxl_core to new directory

[Ben Widawsky]

On 21-09-14 09:57:49, Jonathan Cameron wrote:
> On Tue, 14 Sep 2021 09:56:23 +0100
> Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> 
> > On Mon, 13 Sep 2021 18:33:17 -0400
> > Sasha Levin <sashal@kernel.org> wrote:
> > 
> > > From: Ben Widawsky <ben.widawsky@intel.com>
> > > 
> > > [ Upstream commit 5161a55c069f53d88da49274cbef6e3c74eadea9 ]
> > > 
> > > CXL core is growing, and it's already arguably unmanageable. To support
> > > future growth, move core functionality to a new directory and rename the
> > > file to represent just bus support. Future work will remove non-bus
> > > functionality.
> > > 
> > > Note that mem.h is renamed to cxlmem.h to avoid a namespace collision
> > > with the global ARCH=um mem.h header.  
> > 
> > Not a fix...
> > 
> > I'm guessing this got picked up on the basis of the Reported-by: tag?
> > I think that was added for a minor tweak as this went through review rather
> > than referring to the whole patch.
> Or possibly because it was a precursor to the fix in the next patch.
> 
> Hmm.  Ben, Dan, does it make sense for these two to go into stable?
> 
> Jonathan

As of now, no, but having this will make future fixes much easier to cherry
pick.

> 
> > 
> > Jonathan
> > 
> > 
> > > 
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > Link: https://lore.kernel.org/r/162792537866.368511.8915631504621088321.stgit@dwillia2-desk3.amr.corp.intel.com
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > > ---
> > >  Documentation/driver-api/cxl/memory-devices.rst | 2 +-
> > >  drivers/cxl/Makefile                            | 4 +---
> > >  drivers/cxl/core/Makefile                       | 5 +++++
> > >  drivers/cxl/{core.c => core/bus.c}              | 4 ++--
> > >  drivers/cxl/{mem.h => cxlmem.h}                 | 0
> > >  drivers/cxl/pci.c                               | 2 +-
> > >  drivers/cxl/pmem.c                              | 2 +-
> > >  7 files changed, 11 insertions(+), 8 deletions(-)
> > >  create mode 100644 drivers/cxl/core/Makefile
> > >  rename drivers/cxl/{core.c => core/bus.c} (99%)
> > >  rename drivers/cxl/{mem.h => cxlmem.h} (100%)
> > > 
> > > diff --git a/Documentation/driver-api/cxl/memory-devices.rst b/Documentation/driver-api/cxl/memory-devices.rst
> > > index 487ce4f41d77..a86e2c7c551a 100644
> > > --- a/Documentation/driver-api/cxl/memory-devices.rst
> > > +++ b/Documentation/driver-api/cxl/memory-devices.rst
> > > @@ -36,7 +36,7 @@ CXL Core
> > >  .. kernel-doc:: drivers/cxl/cxl.h
> > >     :internal:
> > >  
> > > -.. kernel-doc:: drivers/cxl/core.c
> > > +.. kernel-doc:: drivers/cxl/core/bus.c
> > >     :doc: cxl core
> > >  
> > >  External Interfaces
> > > diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile
> > > index 32954059b37b..d1aaabc940f3 100644
> > > --- a/drivers/cxl/Makefile
> > > +++ b/drivers/cxl/Makefile
> > > @@ -1,11 +1,9 @@
> > >  # SPDX-License-Identifier: GPL-2.0
> > > -obj-$(CONFIG_CXL_BUS) += cxl_core.o
> > > +obj-$(CONFIG_CXL_BUS) += core/
> > >  obj-$(CONFIG_CXL_MEM) += cxl_pci.o
> > >  obj-$(CONFIG_CXL_ACPI) += cxl_acpi.o
> > >  obj-$(CONFIG_CXL_PMEM) += cxl_pmem.o
> > >  
> > > -ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=CXL
> > > -cxl_core-y := core.o
> > >  cxl_pci-y := pci.o
> > >  cxl_acpi-y := acpi.o
> > >  cxl_pmem-y := pmem.o
> > > diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> > > new file mode 100644
> > > index 000000000000..ad137f96e5c8
> > > --- /dev/null
> > > +++ b/drivers/cxl/core/Makefile
> > > @@ -0,0 +1,5 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +obj-$(CONFIG_CXL_BUS) += cxl_core.o
> > > +
> > > +ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=CXL -I$(srctree)/drivers/cxl
> > > +cxl_core-y := bus.o
> > > diff --git a/drivers/cxl/core.c b/drivers/cxl/core/bus.c
> > > similarity index 99%
> > > rename from drivers/cxl/core.c
> > > rename to drivers/cxl/core/bus.c
> > > index a2e4d54fc7bc..0815eec23944 100644
> > > --- a/drivers/cxl/core.c
> > > +++ b/drivers/cxl/core/bus.c
> > > @@ -6,8 +6,8 @@
> > >  #include <linux/pci.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/idr.h>
> > > -#include "cxl.h"
> > > -#include "mem.h"
> > > +#include <cxlmem.h>
> > > +#include <cxl.h>
> > >  
> > >  /**
> > >   * DOC: cxl core
> > > diff --git a/drivers/cxl/mem.h b/drivers/cxl/cxlmem.h
> > > similarity index 100%
> > > rename from drivers/cxl/mem.h
> > > rename to drivers/cxl/cxlmem.h
> > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > > index 4cf351a3cf99..a945c5fda292 100644
> > > --- a/drivers/cxl/pci.c
> > > +++ b/drivers/cxl/pci.c
> > > @@ -12,9 +12,9 @@
> > >  #include <linux/pci.h>
> > >  #include <linux/io.h>
> > >  #include <linux/io-64-nonatomic-lo-hi.h>
> > > +#include "cxlmem.h"
> > >  #include "pci.h"
> > >  #include "cxl.h"
> > > -#include "mem.h"
> > >  
> > >  /**
> > >   * DOC: cxl pci
> > > diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
> > > index 0088e41dd2f3..9652c3ee41e7 100644
> > > --- a/drivers/cxl/pmem.c
> > > +++ b/drivers/cxl/pmem.c
> > > @@ -6,7 +6,7 @@
> > >  #include <linux/ndctl.h>
> > >  #include <linux/async.h>
> > >  #include <linux/slab.h>
> > > -#include "mem.h"
> > > +#include "cxlmem.h"
> > >  #include "cxl.h"
> > >  
> > >  /*  
> > 
> 

Re: [PATCH AUTOSEL 5.14 03/25] cxl: Move cxl_core to new directory

[Dan Williams]

On Tue, Sep 14, 2021 at 8:06 AM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> On 21-09-14 09:57:49, Jonathan Cameron wrote:
> > On Tue, 14 Sep 2021 09:56:23 +0100
> > Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> >
> > > On Mon, 13 Sep 2021 18:33:17 -0400
> > > Sasha Levin <sashal@kernel.org> wrote:
> > >
> > > > From: Ben Widawsky <ben.widawsky@intel.com>
> > > >
> > > > [ Upstream commit 5161a55c069f53d88da49274cbef6e3c74eadea9 ]
> > > >
> > > > CXL core is growing, and it's already arguably unmanageable. To support
> > > > future growth, move core functionality to a new directory and rename the
> > > > file to represent just bus support. Future work will remove non-bus
> > > > functionality.
> > > >
> > > > Note that mem.h is renamed to cxlmem.h to avoid a namespace collision
> > > > with the global ARCH=um mem.h header.
> > >
> > > Not a fix...
> > >
> > > I'm guessing this got picked up on the basis of the Reported-by: tag?
> > > I think that was added for a minor tweak as this went through review rather
> > > than referring to the whole patch.
> > Or possibly because it was a precursor to the fix in the next patch.
> >
> > Hmm.  Ben, Dan, does it make sense for these two to go into stable?
> >
> > Jonathan
>
> As of now, no, but having this will make future fixes much easier to cherry
> pick.

Sasha, please drop this. The CXL subsystem is still in major feature
development. I would rather manually backport small fixes rather than
backport major code movement just to make small fix backport easier.

Let me know if there was a specific fix autosel was trying to resolve
and we'll take a look at whether it makes sense to do a custom
backport for -stable.

Re: [PATCH AUTOSEL 5.14 04/25] cxl/pci: Introduce cdevm_file_operations

[Dan Williams]

On Mon, Sep 13, 2021 at 3:33 PM Sasha Levin <sashal@kernel.org> wrote:
>
> From: Dan Williams <dan.j.williams@intel.com>
>
> [ Upstream commit 9cc238c7a526dba9ee8c210fa2828886fc65db66 ]
>
> In preparation for moving cxl_memdev allocation to the core, introduce
> cdevm_file_operations to coordinate file operations shutdown relative to
> driver data release.
>
> The motivation for moving cxl_memdev allocation to the core (beyond
> better file organization of sysfs attributes in core/ and drivers in
> cxl/), is that device lifetime is longer than module lifetime. The cxl_pci
> module should be free to come and go without needing to coordinate with
> devices that need the text associated with cxl_memdev_release() to stay
> resident. The move will fix a use after free bug when looping driver
> load / unload with CONFIG_DEBUG_KOBJECT_RELEASE=y.
>
> Another motivation for passing in file_operations to the core cxl_memdev
> creation flow is to allow for alternate drivers, like unit test code, to
> define their own ioctl backends.

Hi Sasha,

Please drop this. It's not a fix, it's just a reorganization for
easing the addition of new features and capabilities.

Re: [PATCH AUTOSEL 5.14 03/25] cxl: Move cxl_core to new directory

[Sasha Levin]

On Tue, Sep 14, 2021 at 08:05:58AM -0700, Ben Widawsky wrote:
>On 21-09-14 09:57:49, Jonathan Cameron wrote:
>> On Tue, 14 Sep 2021 09:56:23 +0100
>> Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
>>
>> > On Mon, 13 Sep 2021 18:33:17 -0400
>> > Sasha Levin <sashal@kernel.org> wrote:
>> >
>> > > From: Ben Widawsky <ben.widawsky@intel.com>
>> > >
>> > > [ Upstream commit 5161a55c069f53d88da49274cbef6e3c74eadea9 ]
>> > >
>> > > CXL core is growing, and it's already arguably unmanageable. To support
>> > > future growth, move core functionality to a new directory and rename the
>> > > file to represent just bus support. Future work will remove non-bus
>> > > functionality.
>> > >
>> > > Note that mem.h is renamed to cxlmem.h to avoid a namespace collision
>> > > with the global ARCH=um mem.h header.
>> >
>> > Not a fix...
>> >
>> > I'm guessing this got picked up on the basis of the Reported-by: tag?
>> > I think that was added for a minor tweak as this went through review rather
>> > than referring to the whole patch.
>> Or possibly because it was a precursor to the fix in the next patch.
>>
>> Hmm.  Ben, Dan, does it make sense for these two to go into stable?
>>
>> Jonathan
>
>As of now, no, but having this will make future fixes much easier to cherry

It was picked because the next patch depends on it. I'll just drop
both if you don't want the next one. Thanks!

-- 
Thanks,
Sasha

Re: [PATCH AUTOSEL 5.14 04/25] cxl/pci: Introduce cdevm_file_operations

[Sasha Levin]

On Tue, Sep 14, 2021 at 08:42:04AM -0700, Dan Williams wrote:
>On Mon, Sep 13, 2021 at 3:33 PM Sasha Levin <sashal@kernel.org> wrote:
>>
>> From: Dan Williams <dan.j.williams@intel.com>
>>
>> [ Upstream commit 9cc238c7a526dba9ee8c210fa2828886fc65db66 ]
>>
>> In preparation for moving cxl_memdev allocation to the core, introduce
>> cdevm_file_operations to coordinate file operations shutdown relative to
>> driver data release.
>>
>> The motivation for moving cxl_memdev allocation to the core (beyond
>> better file organization of sysfs attributes in core/ and drivers in
>> cxl/), is that device lifetime is longer than module lifetime. The cxl_pci
>> module should be free to come and go without needing to coordinate with
>> devices that need the text associated with cxl_memdev_release() to stay
>> resident. The move will fix a use after free bug when looping driver
>> load / unload with CONFIG_DEBUG_KOBJECT_RELEASE=y.
>>
>> Another motivation for passing in file_operations to the core cxl_memdev
>> creation flow is to allow for alternate drivers, like unit test code, to
>> define their own ioctl backends.
>
>Hi Sasha,
>
>Please drop this. It's not a fix, it's just a reorganization for
>easing the addition of new features and capabilities.

I'll drop it, but just to satisfy my curiousity: the description says it
fixes a use-after-free bug in the existing code, is it not the case?

-- 
Thanks,
Sasha

Re: [PATCH AUTOSEL 5.14 04/25] cxl/pci: Introduce cdevm_file_operations

[Dan Williams]

On Tue, Sep 14, 2021 at 10:01 AM Sasha Levin <sashal@kernel.org> wrote:
>
> On Tue, Sep 14, 2021 at 08:42:04AM -0700, Dan Williams wrote:
> >On Mon, Sep 13, 2021 at 3:33 PM Sasha Levin <sashal@kernel.org> wrote:
> >>
> >> From: Dan Williams <dan.j.williams@intel.com>
> >>
> >> [ Upstream commit 9cc238c7a526dba9ee8c210fa2828886fc65db66 ]
> >>
> >> In preparation for moving cxl_memdev allocation to the core, introduce
> >> cdevm_file_operations to coordinate file operations shutdown relative to
> >> driver data release.
> >>
> >> The motivation for moving cxl_memdev allocation to the core (beyond
> >> better file organization of sysfs attributes in core/ and drivers in
> >> cxl/), is that device lifetime is longer than module lifetime. The cxl_pci
> >> module should be free to come and go without needing to coordinate with
> >> devices that need the text associated with cxl_memdev_release() to stay
> >> resident. The move will fix a use after free bug when looping driver
> >> load / unload with CONFIG_DEBUG_KOBJECT_RELEASE=y.
> >>
> >> Another motivation for passing in file_operations to the core cxl_memdev
> >> creation flow is to allow for alternate drivers, like unit test code, to
> >> define their own ioctl backends.
> >
> >Hi Sasha,
> >
> >Please drop this. It's not a fix, it's just a reorganization for
> >easing the addition of new features and capabilities.
>
> I'll drop it, but just to satisfy my curiousity: the description says it
> fixes a use-after-free bug in the existing code, is it not the case?

It does fix a problem if the final put_device() happens after the
module text has been unloaded. However, I am only aware of the
artificial trigger for that (CONFIG_DEBUG_KOBJECT_RELEASE=y). I.e. if
CONFIG_DEBUG_KOBJECT_RELEASE=n I am not aware of any agent that will
hold a device reference besides the driver itself. That was the
rationale for not tagging this for -stable.