On Mon, Sep 20, 2021 at 3:51 PM Ira Weiny <ira.weiny@xxxxxxxxx> 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.
