From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753404AbcAaGMQ (ORCPT ); Sun, 31 Jan 2016 01:12:16 -0500 Received: from mail-ob0-f170.google.com ([209.85.214.170]:35149 "EHLO mail-ob0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751052AbcAaGMO convert rfc822-to-8bit (ORCPT ); Sun, 31 Jan 2016 01:12:14 -0500 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (1.0) Subject: Re: [PATCH 2/2] dax: fix bdev NULL pointer dereferences From: Ross Zwisler X-Mailer: iPad Mail (13D15) In-Reply-To: <20160131023247.GZ2948@linux.intel.com> Date: Sat, 30 Jan 2016 23:12:12 -0700 Cc: Dan Williams , linux-nvdimm , Dave Chinner , "linux-kernel@vger.kernel.org" , Christoph Hellwig , Alexander Viro , Jan Kara , linux-fsdevel , Andrew Morton Content-Transfer-Encoding: 8BIT Message-Id: <33D2FA63-0EBC-4E5F-B337-F06A8A846166@gmail.com> References: <1454009704-25959-1-git-send-email-ross.zwisler@linux.intel.com> <1454009704-25959-2-git-send-email-ross.zwisler@linux.intel.com> <20160128213858.GA29114@infradead.org> <20160129182815.GB5224@linux.intel.com> <20160130052833.GY2948@linux.intel.com> <20160131023247.GZ2948@linux.intel.com> To: Matthew Wilcox Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On Jan 30, 2016, at 7:32 PM, Matthew Wilcox wrote: > >> On Fri, Jan 29, 2016 at 10:01:13PM -0800, Dan Williams wrote: >>> On Fri, Jan 29, 2016 at 9:28 PM, Matthew Wilcox wrote: >>> 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? >> >> I deferred the dax tear down code until next cycle as Al rightly >> pointed out some needed re-works: >> >> https://lists.01.org/pipermail/linux-nvdimm/2016-January/003995.html > > Thanks; I eventually found it in my email somewhere over the Pacific. > > I did probably 70% of the work needed to switch the radix tree over to > storing PFNs instead of sectors. It seems viable, though it's a big > change from where we are today: At one point I had kaddrs in the radix tree, so I could just pull the addresses out and flush them. That would save us a pfn -> kaddrs conversion before flush. Is there a reason to store pnfs instead of kaddrs in the radix tree? > > fs/dax.c | 415 +++++++++++++++++++++++---------------------- > include/linux/dax.h | 3 +- > include/linux/pfn_t.h | 33 +++- > include/linux/radix-tree.h | 9 - > 4 files changed, 236 insertions(+), 224 deletions(-) > > I'll try and get that finished off this week. > > One concrete and easily-separable piece is that dax_clear_blocks() has > the wrong signature. It currently takes an inode & block as parameters; > it has no way of finding out the correct block device. It's only two > callers are filesystems (ext2 and xfs). Those filesystems should be > passing the block_device instead of the inode. But without the inode, > we can't convert a block number to a sector number, so we also need > to pass the sector number, not the block number. It still has type > sector_t, annoyingly. > > @@ -63,12 +238,11 @@ static void dax_unmap_atomic(struct block_device *bdev, > * and hence this means the stack from this point must follow GFP_NOFS > * semantics for all operations. > */ > -int dax_clear_blocks(struct inode *inode, sector_t block, long _size) > +int dax_clear_blocks(struct block_device *bdev, sector_t sector, long size) > { > - struct block_device *bdev = inode->i_sb->s_bdev; > struct blk_dax_ctl dax = { > - .sector = block << (inode->i_blkbits - 9), > - .size = _size, > + .sector = sector, > + .size = size, > }; > > might_sleep(); > > but I haven't looked at doing the conversion of xfs or ext2 to use that > new interface. > _______________________________________________ > Linux-nvdimm mailing list > Linux-nvdimm@lists.01.org > https://lists.01.org/mailman/listinfo/linux-nvdimm