Re: [PATCH 2/2] dax: fix bdev NULL pointer dereferences

From: Matthew Wilcox <willy@linux.intel.com>
To: Jan Kara <jack@suse.cz>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>,
	Christoph Hellwig <hch@infradead.org>,
	linux-kernel@vger.kernel.org,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dan Williams <dan.j.williams@intel.com>,
	Dave Chinner <david@fromorbit.com>, Jan Kara <jack@suse.com>,
	linux-fsdevel@vger.kernel.org, linux-nvdimm@ml01.01.org
Subject: Re: [PATCH 2/2] dax: fix bdev NULL pointer dereferences
Date: Mon, 1 Feb 2016 15:49:39 -0500	[thread overview]
Message-ID: <20160201204939.GG2948@linux.intel.com> (raw)
In-Reply-To: <20160201145147.GD13740@quack.suse.cz>

On Mon, Feb 01, 2016 at 03:51:47PM +0100, Jan Kara wrote:
> On Sat 30-01-16 00:28:33, Matthew Wilcox wrote:
> > I think we're doing it wrong by storing the sector in the radix tree; we'd
> > really need to store both the sector and the bdev which is too much data.
> > 
> > If we store the PFN of the underlying page instead, we don't have this
> > problem.  Instead, we have a different problem; of the device going
> > away under us.  I'm trying to find the code which tears down PTEs when
> > the device goes away, and I'm not seeing it.  What do we do about user
> > mappings of the device?
> 
> So I don't have a strong opinion whether storing PFN or sector is better.
> Maybe PFN is somewhat more generic but OTOH turning DAX off for special
> cases like inodes on XFS RT devices would be IMHO fine.

I'm not sure that's such a great idea.  RT devices might be the best
way to get aligned pages on storage.

> I'm somewhat concerned that there are several things in flight (page fault
> rework, invalidation on device removal, issues with DAX access to block
> devices Ross found) and this is IMHO the smallest trouble we have and changing
> this seems relatively invasive.

This isn't the minimal change to convert us from storing sectors to
storing PFNs.  This is a wholescale rework based around using the page
cache radix tree as the primary data structure instead of buffer heads.

> So could we settle the fault code and
> similar stuff first and look into this somewhat later? Because frankly I
> have some trouble following how all the pieces are going to fit together
> and I'm afraid we'll introduce some non-trivial bugs when several
> fundamental things are in flux in parallel.

We can, of course, do a much smaller patch that will use the radix tree
much less centrally.  And that might be the right way to go for 4.5.
With the extra couple of months we'll have, I hope that this redesign
can be the basis for the DAX code in 4.6.