On Tue, Sep 15, 2020 at 12:58 PM Mike Snitzer <snitzer(a)redhat.com> wrote:
On Tue, Sep 15 2020 at 3:49pm -0400,
Dan Williams <dan.j.williams(a)intel.com> wrote:
> On Tue, Sep 15, 2020 at 1:01 AM Jan Kara <jack(a)suse.cz> wrote:
> > Hi!
> > On Tue 15-09-20 11:03:29, colyli(a)suse.de wrote:
> > > Could you please to take a look? I am offline in the next two weeks.
> > I just had a look into this. IMHO the justification in 6180bb446a "dax:
> > detection of dax support for non-persistent memory block devices" is just
> > bogus and people got confused by the previous condition
> > if (!dax_dev && !bdev_dax_supported(bdev, blocksize))
> > which was bogus as well. bdev_dax_supported() always returns false for bdev
> > that doesn't have dax_dev (naturally so). So in the original condition
> > there was no point in calling bdev_dax_supported() if we know dax_dev is
> > NULL.
> > Then this was changed to:
> > if (!dax_dev || !bdev_dax_supported(bdev, blocksize))
> > which looks more sensible at the first sight. But only at the first sight -
> > if you look at wider context, __generic_fsdax_supported() is the bulk of
> > code that decides whether a device supports DAX so calling
> > bdev_dax_supported() from it indeed doesn't look as such a great idea. So
> > IMO the condition should be just:
> > if (!dax_dev)
> > I'll send a fix for this.
> If you beat me to it, great, but you might be sleeping now. I agree
> the original condition was bogus and looks to be a result of previous
> non-thorough refactoring on my part. I think we can move that !dax_dev
> into dax_supported(). I'll take a look.
You trimmed the relevant portion of Jan's reply
Vishal already owned up to the process escape.
but: can you also
weigh-in one whether DM is using the wrong function to test for DAX?
That might be the original source of the bug because I would expect DM
to be using dax_supported() which should route to all the component
devices. I'll have a talk with whomever put that call to
generic_fsdax_supported() in DM:
git blame drivers/md/dm-table.c
7bf7eac8d6480 (Dan Williams 2019-05-16 13:26:29 -0700 865)
return generic_fsdax_supported(dev->dax_dev, dev->bdev, blocksize,