[RFC PATCH 1/2] block: introduce file_bd_inode()

[RFC PATCH 1/2] block: introduce file_bd_inode()

[Dan Williams]

Similar to the file_inode() helper, provide a helper to lookup the inode for a
raw block device itself.

Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 fs/block_dev.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 073bb57adab1..5277dd83d254 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -147,11 +147,16 @@ blkdev_get_block(struct inode *inode, sector_t iblock,
 	return 0;
 }
 
+static struct inode *file_bd_inode(struct file *file)
+{
+	return file->f_mapping->host;
+}
+
 static ssize_t
 blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t offset)
 {
 	struct file *file = iocb->ki_filp;
-	struct inode *inode = file->f_mapping->host;
+	struct inode *inode = file_bd_inode(file);
 
 	if (IS_DAX(inode))
 		return dax_do_io(iocb, inode, iter, offset, blkdev_get_block,
@@ -329,7 +334,7 @@ static int blkdev_write_end(struct file *file, struct address_space *mapping,
  */
 static loff_t block_llseek(struct file *file, loff_t offset, int whence)
 {
-	struct inode *bd_inode = file->f_mapping->host;
+	struct inode *bd_inode = file_bd_inode(file);
 	loff_t retval;
 
 	mutex_lock(&bd_inode->i_mutex);
@@ -340,7 +345,7 @@ static loff_t block_llseek(struct file *file, loff_t offset, int whence)
 	
 int blkdev_fsync(struct file *filp, loff_t start, loff_t end, int datasync)
 {
-	struct inode *bd_inode = filp->f_mapping->host;
+	struct inode *bd_inode = file_bd_inode(filp);
 	struct block_device *bdev = I_BDEV(bd_inode);
 	int error;
 	
@@ -1579,14 +1584,14 @@ EXPORT_SYMBOL(blkdev_put);
 
 static int blkdev_close(struct inode * inode, struct file * filp)
 {
-	struct block_device *bdev = I_BDEV(filp->f_mapping->host);
+	struct block_device *bdev = I_BDEV(file_bd_inode(filp));
 	blkdev_put(bdev, filp->f_mode);
 	return 0;
 }
 
 static long block_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 {
-	struct block_device *bdev = I_BDEV(file->f_mapping->host);
+	struct block_device *bdev = I_BDEV(file_bd_inode(file));
 	fmode_t mode = file->f_mode;
 
 	/*
@@ -1611,7 +1616,7 @@ static long block_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
 {
 	struct file *file = iocb->ki_filp;
-	struct inode *bd_inode = file->f_mapping->host;
+	struct inode *bd_inode = file_bd_inode(file);
 	loff_t size = i_size_read(bd_inode);
 	struct blk_plug plug;
 	ssize_t ret;
@@ -1643,7 +1648,7 @@ EXPORT_SYMBOL_GPL(blkdev_write_iter);
 ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
 {
 	struct file *file = iocb->ki_filp;
-	struct inode *bd_inode = file->f_mapping->host;
+	struct inode *bd_inode = file_bd_inode(file);
 	loff_t size = i_size_read(bd_inode);
 	loff_t pos = iocb->ki_pos;
 

[RFC PATCH 2/2] block: enable dax for raw block devices

[Dan Williams]

If an application wants exclusive access to all of the persistent memory
provided by an NVDIMM namespace it can use this raw-block-dax facility
to forgo establishing a filesystem.  This capability is targeted
primarily to hypervisors wanting to provision persistent memory for
guests.

Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---

Only lighted tested so far, but seems to work, is the shortest path to a
DAX mapping, and makes it easier to trigger the pmd_fault path (no
fs-block-allocator interactions).

 fs/block_dev.c |   84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 5277dd83d254..498b71455570 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1687,13 +1687,95 @@ static const struct address_space_operations def_blk_aops = {
 	.is_dirty_writeback = buffer_check_dirty_writeback,
 };
 
+#ifdef CONFIG_FS_DAX
+static int blkdev_dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+	struct inode *bd_inode = file_bd_inode(vma->vm_file);
+	struct block_device *bdev = I_BDEV(bd_inode);
+	int ret;
+
+	mutex_lock(&bdev->bd_mutex);
+	ret = __dax_fault(vma, vmf, blkdev_get_block, NULL);
+	mutex_unlock(&bdev->bd_mutex);
+
+	return ret;
+}
+
+static int blkdev_dax_pmd_fault(struct vm_area_struct *vma, unsigned long addr,
+		pmd_t *pmd, unsigned int flags)
+{
+	struct inode *bd_inode = file_bd_inode(vma->vm_file);
+	struct block_device *bdev = I_BDEV(bd_inode);
+	int ret;
+
+	mutex_lock(&bdev->bd_mutex);
+	ret = __dax_pmd_fault(vma, addr, pmd, flags, blkdev_get_block, NULL);
+	mutex_unlock(&bdev->bd_mutex);
+
+	return ret;
+}
+
+static int blkdev_dax_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+	struct inode *bd_inode = file_bd_inode(vma->vm_file);
+	struct block_device *bdev = I_BDEV(bd_inode);
+	int ret;
+
+	mutex_lock(&bdev->bd_mutex);
+	ret = __dax_mkwrite(vma, vmf, blkdev_get_block, NULL);
+	mutex_unlock(&bdev->bd_mutex);
+
+	return ret;
+}
+
+static int blkdev_dax_pfn_mkwrite(struct vm_area_struct *vma,
+		struct vm_fault *vmf)
+{
+	struct inode *bd_inode = file_bd_inode(vma->vm_file);
+	struct block_device *bdev = I_BDEV(bd_inode);
+	int ret = VM_FAULT_NOPAGE;
+	loff_t size;
+
+	/* check that the faulting page hasn't raced with bdev resize */
+	mutex_lock(&bdev->bd_mutex);
+	size = (i_size_read(bd_inode) + PAGE_SIZE - 1) >> PAGE_SHIFT;
+	if (vmf->pgoff >= size)
+		ret = VM_FAULT_SIGBUS;
+	mutex_unlock(&bdev->bd_mutex);
+
+	return ret;
+}
+
+static const struct vm_operations_struct blkdev_dax_vm_ops = {
+	.fault		= blkdev_dax_fault,
+	.pmd_fault	= blkdev_dax_pmd_fault,
+	.page_mkwrite	= blkdev_dax_mkwrite,
+	.pfn_mkwrite	= blkdev_dax_pfn_mkwrite,
+};
+
+static int blkdev_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	struct inode *bd_inode = file_bd_inode(file);
+
+	if (!IS_DAX(bd_inode))
+		return generic_file_mmap(file, vma);
+
+	file_accessed(file);
+	vma->vm_ops = &blkdev_dax_vm_ops;
+	vma->vm_flags |= VM_MIXEDMAP | VM_HUGEPAGE;
+	return 0;
+}
+#else
+#define blkdev_mmap generic_file_mmap
+#endif
+
 const struct file_operations def_blk_fops = {
 	.open		= blkdev_open,
 	.release	= blkdev_close,
 	.llseek		= block_llseek,
 	.read_iter	= blkdev_read_iter,
 	.write_iter	= blkdev_write_iter,
-	.mmap		= generic_file_mmap,
+	.mmap		= blkdev_mmap,
 	.fsync		= blkdev_fsync,
 	.unlocked_ioctl	= block_ioctl,
 #ifdef CONFIG_COMPAT

Re: [RFC PATCH 2/2] block: enable dax for raw block devices

[Ross Zwisler]

On Fri, Oct 16, 2015 at 08:49:41PM -0400, Dan Williams wrote:
> If an application wants exclusive access to all of the persistent memory
> provided by an NVDIMM namespace it can use this raw-block-dax facility
> to forgo establishing a filesystem.  This capability is targeted
> primarily to hypervisors wanting to provision persistent memory for
> guests.
> 
> Cc: Jeff Moyer <jmoyer@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> 
> Only lighted tested so far, but seems to work, is the shortest path to a
> DAX mapping, and makes it easier to trigger the pmd_fault path (no
> fs-block-allocator interactions).
> 
>  fs/block_dev.c |   84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 83 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 5277dd83d254..498b71455570 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -1687,13 +1687,95 @@ static const struct address_space_operations def_blk_aops = {
>  	.is_dirty_writeback = buffer_check_dirty_writeback,
>  };
>  
> +#ifdef CONFIG_FS_DAX
> +static int blkdev_dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> +{
> +	struct inode *bd_inode = file_bd_inode(vma->vm_file);
> +	struct block_device *bdev = I_BDEV(bd_inode);
> +	int ret;
> +
> +	mutex_lock(&bdev->bd_mutex);
> +	ret = __dax_fault(vma, vmf, blkdev_get_block, NULL);
> +	mutex_unlock(&bdev->bd_mutex);
> +
> +	return ret;
> +}

This all looks very straightforward.  The one comment I have is that this
code is missing the calls to sb_[start|end]_pagefault(), and to
file_update_time() that are found in ext[24]/xfs and the generic fault code.

The previous version of this code used the generic fault implementation, and
was calling these functions via filemap_page_mkwrite().

It is possible that they were omitted for a reason - does protection from
filesystem freezing still make sense when talking with a raw block device?
For example, if that block device *has* a mounted filesystem on it that is
frozen, does sb_start_pagefault() prevent against page faults on the raw
device that try and make something writable?  

In any case, the presence of them in filemap_page_mkwrite() tells me that they
at least aren't harmful, and I wanted to make sure they weren't needed before
leaving them out.  If the omission was intentional, should we add a comment to
explain why they are missing?

Re: [RFC PATCH 2/2] block: enable dax for raw block devices

[Dan Williams]

On Sun, Oct 18, 2015 at 8:01 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> On Fri, Oct 16, 2015 at 08:49:41PM -0400, Dan Williams wrote:
>> If an application wants exclusive access to all of the persistent memory
>> provided by an NVDIMM namespace it can use this raw-block-dax facility
>> to forgo establishing a filesystem.  This capability is targeted
>> primarily to hypervisors wanting to provision persistent memory for
>> guests.
>>
>> Cc: Jeff Moyer <jmoyer@redhat.com>
>> Cc: Christoph Hellwig <hch@lst.de>
>> Cc: Al Viro <viro@zeniv.linux.org.uk>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
>> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>>
>> Only lighted tested so far, but seems to work, is the shortest path to a
>> DAX mapping, and makes it easier to trigger the pmd_fault path (no
>> fs-block-allocator interactions).
>>
>>  fs/block_dev.c |   84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 83 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/block_dev.c b/fs/block_dev.c
>> index 5277dd83d254..498b71455570 100644
>> --- a/fs/block_dev.c
>> +++ b/fs/block_dev.c
>> @@ -1687,13 +1687,95 @@ static const struct address_space_operations def_blk_aops = {
>>       .is_dirty_writeback = buffer_check_dirty_writeback,
>>  };
>>
>> +#ifdef CONFIG_FS_DAX
>> +static int blkdev_dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>> +{
>> +     struct inode *bd_inode = file_bd_inode(vma->vm_file);
>> +     struct block_device *bdev = I_BDEV(bd_inode);
>> +     int ret;
>> +
>> +     mutex_lock(&bdev->bd_mutex);
>> +     ret = __dax_fault(vma, vmf, blkdev_get_block, NULL);
>> +     mutex_unlock(&bdev->bd_mutex);
>> +
>> +     return ret;
>> +}
>
> This all looks very straightforward.  The one comment I have is that this
> code is missing the calls to sb_[start|end]_pagefault(), and to
> file_update_time() that are found in ext[24]/xfs and the generic fault code.
>
> The previous version of this code used the generic fault implementation, and
> was calling these functions via filemap_page_mkwrite().
>
> It is possible that they were omitted for a reason - does protection from
> filesystem freezing still make sense when talking with a raw block device?
> For example, if that block device *has* a mounted filesystem on it that is
> frozen, does sb_start_pagefault() prevent against page faults on the raw
> device that try and make something writable?
>
> In any case, the presence of them in filemap_page_mkwrite() tells me that they
> at least aren't harmful, and I wanted to make sure they weren't needed before
> leaving them out.  If the omission was intentional, should we add a comment to
> explain why they are missing?

So, I left them out on purpose and labeled this "RFC" mainly because I
wasn't ready to assert that the new locking using bd_mutex did not
have bad interactions with those paths or the mmap path in general.

The access time and interactions with freezing are different for raw
block device files given that the inode for the per-instance
device-node file is separate from the inode for the block device
itself.  The device node file is typically on a tmpfs filesystem
mounted at /dev.  The access time of the device node doesn't have a
reliable correlation with the access time of the block-device due to
the fact that you can mknod() a new device node anywhere, on any
filesystem, and do i/o.

I'll consider adding them back if they do no harm, but "device special
files" are already sufficiently different than regular files that I'm
not sure it matters.  Either way I agree this could use more
commentary in the code.