Re: [PATCH 1/2] dax: pass bdev argument to dax_clear_blocks()

From: Ross Zwisler <zwisler@gmail.com>
To: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>,
	Theodore Ts'o <tytso@mit.edu>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
	Dave Chinner <david@fromorbit.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	XFS Developers <xfs@oss.sgi.com>, Linux MM <linux-mm@kvack.org>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Jan Kara <jack@suse.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-ext4 <linux-ext4@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 1/2] dax: pass bdev argument to dax_clear_blocks()
Date: Sun, 7 Feb 2016 21:29:38 -0700	[thread overview]
Message-ID: <00FE872A-9B2A-4492-A83C-59025ACB1F4A@gmail.com> (raw)
In-Reply-To: <20160208014601.GB2343@linux.intel.com>

> On Feb 7, 2016, at 6:46 PM, Ross Zwisler <ross.zwisler@linux.intel.com> wrote:
> 
>> On Sun, Feb 07, 2016 at 10:19:29AM -0800, Dan Williams wrote:
>> On Sat, Feb 6, 2016 at 11:19 PM, Ross Zwisler
>> <ross.zwisler@linux.intel.com> wrote:
>>> dax_clear_blocks() needs a valid struct block_device and previously it was
>>> using inode->i_sb->s_bdev in all cases.  This is correct for normal inodes
>>> on mounted ext2, ext4 and XFS filesystems, but is incorrect for DAX raw
>>> block devices and for XFS real-time devices.
>>> 
>>> Instead, have the caller pass in a struct block_device pointer which it
>>> knows to be correct.
>>> 
>>> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>>> ---
>>> fs/dax.c               | 4 ++--
>>> fs/ext2/inode.c        | 5 +++--
>>> fs/xfs/xfs_aops.c      | 2 +-
>>> fs/xfs/xfs_aops.h      | 1 +
>>> fs/xfs/xfs_bmap_util.c | 4 +++-
>>> include/linux/dax.h    | 3 ++-
>>> 6 files changed, 12 insertions(+), 7 deletions(-)
>>> 
>>> diff --git a/fs/dax.c b/fs/dax.c
>>> index 227974a..4592241 100644
>>> --- a/fs/dax.c
>>> +++ b/fs/dax.c
>>> @@ -83,9 +83,9 @@ struct page *read_dax_sector(struct block_device *bdev, sector_t n)
>>>  * 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 inode *inode, struct block_device *bdev,
>>> +               sector_t block, long _size)
>> 
>> Since this is a bdev relative routine we should also resolve the
>> sector, i.e. the signature should drop the inode:
>> 
>> int dax_clear_sectors(struct block_device *bdev, sector_t sector, long _size)
> 
> The inode is still needed because dax_clear_blocks() needs inode->i_blkbits.
> Unless there is some easy way to get this from the bdev that I'm not seeing?

Never mind, you are passing in the sector, not the block.  Sure, this seems better - I'll fix this for v2.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>