diff mbox

[v3,1/2] dax: add dax_get_unmapped_area for pmd mappings

Message ID 1460652511-19636-2-git-send-email-toshi.kani@hpe.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kani, Toshi April 14, 2016, 4:48 p.m. UTC
When CONFIG_FS_DAX_PMD is set, DAX supports mmap() using pmd page
size.  This feature relies on both mmap virtual address and FS
block (i.e. physical address) to be aligned by the pmd page size.
Users can use mkfs options to specify FS to align block allocations.
However, aligning mmap address requires code changes to existing
applications for providing a pmd-aligned address to mmap().

For instance, fio with "ioengine=mmap" performs I/Os with mmap() [1].
It calls mmap() with a NULL address, which needs to be changed to
provide a pmd-aligned address for testing with DAX pmd mappings.
Changing all applications that call mmap() with NULL is undesirable.

Add dax_get_unmapped_area(), which can be called by filesystem's
get_unmapped_area to align an mmap address by the pmd size for
a DAX file.  It calls the default handler, mm->get_unmapped_area(),
to find a range and then aligns it for a DAX file.

[1]: https://github.com/axboe/fio/blob/master/engines/mmap.c
Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Matthew Wilcox <willy@linux.intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
---
 fs/dax.c            |   43 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/dax.h |    3 +++
 2 files changed, 46 insertions(+)

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jan Kara April 18, 2016, 8:47 p.m. UTC | #1
On Thu 14-04-16 10:48:30, Toshi Kani wrote:
> +
> +/**
> + * dax_get_unmapped_area - handle get_unmapped_area for a DAX file
> + * @filp: The file being mmap'd, if not NULL
> + * @addr: The mmap address. If NULL, the kernel assigns the address
> + * @len: The mmap size in bytes
> + * @pgoff: The page offset in the file where the mapping starts from.
> + * @flags: The mmap flags
> + *
> + * This function can be called by a filesystem for get_unmapped_area().
> + * When a target file is a DAX file, it aligns the mmap address at the
> + * beginning of the file by the pmd size.
> + */
> +unsigned long dax_get_unmapped_area(struct file *filp, unsigned long addr,
> +		unsigned long len, unsigned long pgoff, unsigned long flags)
> +{
> +	unsigned long off, off_end, off_pmd, len_pmd, addr_pmd;

I think we need to use 'loff_t' for the offsets for things to work on
32-bits.

> +	if (!IS_ENABLED(CONFIG_FS_DAX_PMD) ||
> +	    !filp || addr || !IS_DAX(filp->f_mapping->host))
> +		goto out;
> +
> +	off = pgoff << PAGE_SHIFT;

And here we need to type to loff_t before the shift...

> +	off_end = off + len;
> +	off_pmd = round_up(off, PMD_SIZE);  /* pmd-aligned offset */
> +
> +	if ((off_end <= off_pmd) || ((off_end - off_pmd) < PMD_SIZE))

None of these parenthesis is actually needed (and IMHO they make the code
less readable, not more).

> +		goto out;
> +
> +	len_pmd = len + PMD_SIZE;
> +	if ((off + len_pmd) < off)
> +		goto out;
> +
> +	addr_pmd = current->mm->get_unmapped_area(filp, addr, len_pmd,
> +						  pgoff, flags);
> +	if (!IS_ERR_VALUE(addr_pmd)) {
> +		addr_pmd += (off - addr_pmd) & (PMD_SIZE - 1);
> +		return addr_pmd;

Otherwise the patch looks good to me.

								Honza
Kani, Toshi April 19, 2016, 2:36 a.m. UTC | #2
On 4/18/2016 4:47 PM, Jan Kara wrote:
> On Thu 14-04-16 10:48:30, Toshi Kani wrote:
>> +
>> +/**
>> + * dax_get_unmapped_area - handle get_unmapped_area for a DAX file
>> + * @filp: The file being mmap'd, if not NULL
>> + * @addr: The mmap address. If NULL, the kernel assigns the address
>> + * @len: The mmap size in bytes
>> + * @pgoff: The page offset in the file where the mapping starts from.
>> + * @flags: The mmap flags
>> + *
>> + * This function can be called by a filesystem for get_unmapped_area().
>> + * When a target file is a DAX file, it aligns the mmap address at the
>> + * beginning of the file by the pmd size.
>> + */
>> +unsigned long dax_get_unmapped_area(struct file *filp, unsigned long addr,
>> +		unsigned long len, unsigned long pgoff, unsigned long flags)
>> +{
>> +	unsigned long off, off_end, off_pmd, len_pmd, addr_pmd;
> I think we need to use 'loff_t' for the offsets for things to work on
> 32-bits.

Agreed. Will change to loff_t.

>> +	if (!IS_ENABLED(CONFIG_FS_DAX_PMD) ||
>> +	    !filp || addr || !IS_DAX(filp->f_mapping->host))
>> +		goto out;
>> +
>> +	off = pgoff << PAGE_SHIFT;
> And here we need to type to loff_t before the shift...

Right.

>> +	off_end = off + len;
>> +	off_pmd = round_up(off, PMD_SIZE);  /* pmd-aligned offset */
>> +
>> +	if ((off_end <= off_pmd) || ((off_end - off_pmd) < PMD_SIZE))
> None of these parenthesis is actually needed (and IMHO they make the code
> less readable, not more).

OK.  Will remove the parenthesis.

>> +		goto out;
>> +
>> +	len_pmd = len + PMD_SIZE;
>> +	if ((off + len_pmd) < off)
>> +		goto out;
>> +
>> +	addr_pmd = current->mm->get_unmapped_area(filp, addr, len_pmd,
>> +						  pgoff, flags);
>> +	if (!IS_ERR_VALUE(addr_pmd)) {
>> +		addr_pmd += (off - addr_pmd) & (PMD_SIZE - 1);
>> +		return addr_pmd;
> Otherwise the patch looks good to me.

Great. Thanks Jan!
-Toshi
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/dax.c b/fs/dax.c
index 75ba46d..f8ddd27 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1158,3 +1158,46 @@  int dax_truncate_page(struct inode *inode, loff_t from, get_block_t get_block)
 	return dax_zero_page_range(inode, from, length, get_block);
 }
 EXPORT_SYMBOL_GPL(dax_truncate_page);
+
+/**
+ * dax_get_unmapped_area - handle get_unmapped_area for a DAX file
+ * @filp: The file being mmap'd, if not NULL
+ * @addr: The mmap address. If NULL, the kernel assigns the address
+ * @len: The mmap size in bytes
+ * @pgoff: The page offset in the file where the mapping starts from.
+ * @flags: The mmap flags
+ *
+ * This function can be called by a filesystem for get_unmapped_area().
+ * When a target file is a DAX file, it aligns the mmap address at the
+ * beginning of the file by the pmd size.
+ */
+unsigned long dax_get_unmapped_area(struct file *filp, unsigned long addr,
+		unsigned long len, unsigned long pgoff, unsigned long flags)
+{
+	unsigned long off, off_end, off_pmd, len_pmd, addr_pmd;
+
+	if (!IS_ENABLED(CONFIG_FS_DAX_PMD) ||
+	    !filp || addr || !IS_DAX(filp->f_mapping->host))
+		goto out;
+
+	off = pgoff << PAGE_SHIFT;
+	off_end = off + len;
+	off_pmd = round_up(off, PMD_SIZE);  /* pmd-aligned offset */
+
+	if ((off_end <= off_pmd) || ((off_end - off_pmd) < PMD_SIZE))
+		goto out;
+
+	len_pmd = len + PMD_SIZE;
+	if ((off + len_pmd) < off)
+		goto out;
+
+	addr_pmd = current->mm->get_unmapped_area(filp, addr, len_pmd,
+						  pgoff, flags);
+	if (!IS_ERR_VALUE(addr_pmd)) {
+		addr_pmd += (off - addr_pmd) & (PMD_SIZE - 1);
+		return addr_pmd;
+	}
+out:
+	return current->mm->get_unmapped_area(filp, addr, len, pgoff, flags);
+}
+EXPORT_SYMBOL_GPL(dax_get_unmapped_area);
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 636dd59..184b171 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -17,12 +17,15 @@  int __dax_fault(struct vm_area_struct *, struct vm_fault *, get_block_t,
 
 #ifdef CONFIG_FS_DAX
 struct page *read_dax_sector(struct block_device *bdev, sector_t n);
+unsigned long dax_get_unmapped_area(struct file *filp, unsigned long addr,
+		unsigned long len, unsigned long pgoff, unsigned long flags);
 #else
 static inline struct page *read_dax_sector(struct block_device *bdev,
 		sector_t n)
 {
 	return ERR_PTR(-ENXIO);
 }
+#define dax_get_unmapped_area	NULL
 #endif
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE