Messages in this thread | | | Date | Fri, 2 Feb 2024 09:40:20 -0500 | Subject | Re: [RFC PATCH v3 2/4] dax: Check for data cache aliasing at runtime | From | Mathieu Desnoyers <> |
| |
On 2024-02-01 10:44, Mathieu Desnoyers wrote: [...] >> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c >> index 4e8fdcb3f1c8..b69c9e442cf4 100644 >> --- a/drivers/nvdimm/pmem.c >> +++ b/drivers/nvdimm/pmem.c >> @@ -560,17 +560,19 @@ static int pmem_attach_disk(struct device *dev, >> dax_dev = alloc_dax(pmem, &pmem_dax_ops); >> if (IS_ERR(dax_dev)) { >> rc = PTR_ERR(dax_dev); >> - goto out; >> + if (rc != -EOPNOTSUPP) >> + goto out; > > If I compare the before / after this change, if previously > pmem_attach_disk() was called in a configuration with FS_DAX=n, it would > result in a NULL pointer dereference.
I was wrong. drivers/nvdimm/Kconfig has:
config BLK_DEV_PMEM select DAX
and
drivers/nvdimm/Makefile has:
obj-$(CONFIG_BLK_DEV_PMEM) += nd_pmem.o nd_pmem-y := pmem.o
which means that anything in pmem.c can assume that alloc_dax() is implemented.
[...] >> diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c >> index 4b7ecd4fd431..f911e58a24dd 100644 >> --- a/drivers/s390/block/dcssblk.c >> +++ b/drivers/s390/block/dcssblk.c >> @@ -681,12 +681,14 @@ dcssblk_add_store(struct device *dev, struct >> device_attribute *attr, const char >> if (IS_ERR(dev_info->dax_dev)) { >> rc = PTR_ERR(dev_info->dax_dev); >> dev_info->dax_dev = NULL; >> - goto put_dev; >> + if (rc != -EOPNOTSUPP) >> + goto put_dev; > > config DCSSBLK selects FS_DAX_LIMITED and DAX. > > I'm not sure what selecting DAX is trying to achieve here, because the > Kconfig option is "FS_DAX". > > So depending on the real motivation behind this select, we may want to > consider failure rather than success in the -EOPNOTSUPP case. >
I missed that alloc_dax() is implemented as not supported based on CONFIG_DAX (not CONFIG_FS_DAX).
Therefore DCSSBLK Kconfig does the right thing and always selects DAX, and thus an implemented version of alloc_dax().
This takes care of two of my open questions at least. :)
Thanks,
Mathieu
-- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com
| |