On Mon, Sep 20, 2021 at 3:51 PM Ira Weiny <[email protected]> wrote: > > On Mon, Sep 20, 2021 at 09:27:25AM +0200, Christoph Hellwig wrote: > > ... > > > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c > > index ef4950f808326..bbeb3f46db157 100644 > > --- a/drivers/nvdimm/pmem.c > > +++ b/drivers/nvdimm/pmem.c > > @@ -328,6 +328,49 @@ static const struct dax_operations pmem_dax_ops = { > > .zero_page_range = pmem_dax_zero_page_range, > > }; > > > > +static ssize_t write_cache_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + struct pmem_device *pmem = dev_to_disk(dev)->private_data; > > I want to say this should be dax_get_private()... However, looking at the use
No, this wants to do from @dev to @dax_dev. dax_get_private() assumes that @dax_dev is already known. Also, in this case @dev is the gendisk device, so this is a gendisk-to-dax-device with special knowledge that the gendisk is for a pmem device. > of dax_get_private() not a single caller checks for NULL! :-( All the callers are correctly assuming that their usage is before kill_dax(). > > So now I wonder why dax_get_private() exists... :-/ It exists so that the definition of 'struct dax_device' can remain private, as no one should be directly mucking with dax_device internals outside of the provided APIs. > A quick history search does not make anything apparent. When the DAXDEV_ALIVE > check was added to dax_get_private() no callers were changed to account for a > potential NULL return. > > Dan? I double checked, but this all looks ok to me.
