All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Matthew Wilcox <matthew.r.wilcox@intel.com>
Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, willy@linux.intel.com
Subject: Re: [PATCH v7 08/22] Replace xip_truncate_page with dax_truncate_page
Date: Wed, 9 Apr 2014 00:17:59 +0200	[thread overview]
Message-ID: <20140408221759.GD26019@quack.suse.cz> (raw)
In-Reply-To: <fd328c564ddc79b41a3a8d754080e6e6e77bbf4f.1395591795.git.matthew.r.wilcox@intel.com>

On Sun 23-03-14 15:08:34, Matthew Wilcox wrote:
> It takes a get_block parameter just like nobh_truncate_page() and
> block_truncate_page()
  The patch looks mostly OK. Some minor comments below.

> 
> Signed-off-by: Matthew Wilcox <matthew.r.wilcox@intel.com>
> ---
>  fs/dax.c           | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
>  fs/ext2/inode.c    |  2 +-
>  include/linux/fs.h |  4 ++--
>  mm/filemap_xip.c   | 40 ----------------------------------------
>  4 files changed, 51 insertions(+), 47 deletions(-)
> 
> diff --git a/fs/dax.c b/fs/dax.c
> index 863749c..7271be0 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -374,13 +374,13 @@ static int do_dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
>  }
>  
>  /**
> - * dax_fault - handle a page fault on an XIP file
> + * dax_fault - handle a page fault on a DAX file
>   * @vma: The virtual memory area where the fault occurred
>   * @vmf: The description of the fault
>   * @get_block: The filesystem method used to translate file offsets to blocks
>   *
>   * When a page fault occurs, filesystems may call this helper in their
> - * fault handler for XIP files.
> + * fault handler for DAX files.
>   */
>  int dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
>  			get_block_t get_block)
> @@ -398,12 +398,12 @@ int dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
>  EXPORT_SYMBOL_GPL(dax_fault);
>  
>  /**
> - * dax_mkwrite - convert a read-only page to read-write in an XIP file
> + * dax_mkwrite - convert a read-only page to read-write in a DAX file
>   * @vma: The virtual memory area where the fault occurred
>   * @vmf: The description of the fault
>   * @get_block: The filesystem method used to translate file offsets to blocks
>   *
> - * XIP handles reads of holes by adding pages full of zeroes into the
> + * DAX handles reads of holes by adding pages full of zeroes into the
>   * mapping.  If the page is subsequenty written to, we have to allocate
>   * the page on media and free the page that was in the cache.
>   */
  Above two hunks belong to the previous patch...

> @@ -421,3 +421,47 @@ int dax_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
>  	return result;
>  }
>  EXPORT_SYMBOL_GPL(dax_mkwrite);
> +
> +/**
> + * dax_truncate_page - handle a partial page being truncated in a DAX file
> + * @inode: The file being truncated
> + * @from: The file offset that is being truncated to
> + * @get_block: The filesystem method used to translate file offsets to blocks
> + *
> + * Similar to block_truncate_page(), this function can be called by a
> + * filesystem when it is truncating an DAX file to handle the partial page.
> + *
> + * We work in terms of PAGE_CACHE_SIZE here for commonality with
> + * block_truncate_page(), but we could go down to PAGE_SIZE if the filesystem
> + * took care of disposing of the unnecessary blocks.  Even if the filesystem
> + * block size is smaller than PAGE_SIZE, we have to zero the rest of the page
> + * since the file might be mmaped.
  Well, DAX mmap support pretty much relies on PAGE_CACHE_SIZE == block
size (we cannot really map only a part of a physical page directly...). So
the comment seems somewhat misleading.

> + */
> +int dax_truncate_page(struct inode *inode, loff_t from, get_block_t get_block)
> +{
> +	struct buffer_head bh;
> +	pgoff_t index = from >> PAGE_CACHE_SHIFT;
> +	unsigned offset = from & (PAGE_CACHE_SIZE-1);
> +	unsigned length = PAGE_CACHE_ALIGN(from) - from;
> +	int err;
> +
  Can we WARN_ON_ONCE here if PAGE_CACHE_SHIFT != inode->i_blkbits? Just to
catch bugs early.

> +	/* Block boundary? Nothing to do */
> +	if (!length)
> +		return 0;
> +
> +	memset(&bh, 0, sizeof(bh));
> +	bh.b_size = PAGE_CACHE_SIZE;
> +	err = get_block(inode, index, &bh, 0);
> +	if (err < 0)
> +		return err;
> +	if (buffer_written(&bh)) {
> +		void *addr;
> +		err = dax_get_addr(inode, &bh, &addr);
> +		if (err)
> +			return err;
> +		memset(addr + offset, 0, length);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(dax_truncate_page);

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz>
To: Matthew Wilcox <matthew.r.wilcox@intel.com>
Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, willy@linux.intel.com
Subject: Re: [PATCH v7 08/22] Replace xip_truncate_page with dax_truncate_page
Date: Wed, 9 Apr 2014 00:17:59 +0200	[thread overview]
Message-ID: <20140408221759.GD26019@quack.suse.cz> (raw)
In-Reply-To: <fd328c564ddc79b41a3a8d754080e6e6e77bbf4f.1395591795.git.matthew.r.wilcox@intel.com>

On Sun 23-03-14 15:08:34, Matthew Wilcox wrote:
> It takes a get_block parameter just like nobh_truncate_page() and
> block_truncate_page()
  The patch looks mostly OK. Some minor comments below.

> 
> Signed-off-by: Matthew Wilcox <matthew.r.wilcox@intel.com>
> ---
>  fs/dax.c           | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
>  fs/ext2/inode.c    |  2 +-
>  include/linux/fs.h |  4 ++--
>  mm/filemap_xip.c   | 40 ----------------------------------------
>  4 files changed, 51 insertions(+), 47 deletions(-)
> 
> diff --git a/fs/dax.c b/fs/dax.c
> index 863749c..7271be0 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -374,13 +374,13 @@ static int do_dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
>  }
>  
>  /**
> - * dax_fault - handle a page fault on an XIP file
> + * dax_fault - handle a page fault on a DAX file
>   * @vma: The virtual memory area where the fault occurred
>   * @vmf: The description of the fault
>   * @get_block: The filesystem method used to translate file offsets to blocks
>   *
>   * When a page fault occurs, filesystems may call this helper in their
> - * fault handler for XIP files.
> + * fault handler for DAX files.
>   */
>  int dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
>  			get_block_t get_block)
> @@ -398,12 +398,12 @@ int dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
>  EXPORT_SYMBOL_GPL(dax_fault);
>  
>  /**
> - * dax_mkwrite - convert a read-only page to read-write in an XIP file
> + * dax_mkwrite - convert a read-only page to read-write in a DAX file
>   * @vma: The virtual memory area where the fault occurred
>   * @vmf: The description of the fault
>   * @get_block: The filesystem method used to translate file offsets to blocks
>   *
> - * XIP handles reads of holes by adding pages full of zeroes into the
> + * DAX handles reads of holes by adding pages full of zeroes into the
>   * mapping.  If the page is subsequenty written to, we have to allocate
>   * the page on media and free the page that was in the cache.
>   */
  Above two hunks belong to the previous patch...

> @@ -421,3 +421,47 @@ int dax_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
>  	return result;
>  }
>  EXPORT_SYMBOL_GPL(dax_mkwrite);
> +
> +/**
> + * dax_truncate_page - handle a partial page being truncated in a DAX file
> + * @inode: The file being truncated
> + * @from: The file offset that is being truncated to
> + * @get_block: The filesystem method used to translate file offsets to blocks
> + *
> + * Similar to block_truncate_page(), this function can be called by a
> + * filesystem when it is truncating an DAX file to handle the partial page.
> + *
> + * We work in terms of PAGE_CACHE_SIZE here for commonality with
> + * block_truncate_page(), but we could go down to PAGE_SIZE if the filesystem
> + * took care of disposing of the unnecessary blocks.  Even if the filesystem
> + * block size is smaller than PAGE_SIZE, we have to zero the rest of the page
> + * since the file might be mmaped.
  Well, DAX mmap support pretty much relies on PAGE_CACHE_SIZE == block
size (we cannot really map only a part of a physical page directly...). So
the comment seems somewhat misleading.

> + */
> +int dax_truncate_page(struct inode *inode, loff_t from, get_block_t get_block)
> +{
> +	struct buffer_head bh;
> +	pgoff_t index = from >> PAGE_CACHE_SHIFT;
> +	unsigned offset = from & (PAGE_CACHE_SIZE-1);
> +	unsigned length = PAGE_CACHE_ALIGN(from) - from;
> +	int err;
> +
  Can we WARN_ON_ONCE here if PAGE_CACHE_SHIFT != inode->i_blkbits? Just to
catch bugs early.

> +	/* Block boundary? Nothing to do */
> +	if (!length)
> +		return 0;
> +
> +	memset(&bh, 0, sizeof(bh));
> +	bh.b_size = PAGE_CACHE_SIZE;
> +	err = get_block(inode, index, &bh, 0);
> +	if (err < 0)
> +		return err;
> +	if (buffer_written(&bh)) {
> +		void *addr;
> +		err = dax_get_addr(inode, &bh, &addr);
> +		if (err)
> +			return err;
> +		memset(addr + offset, 0, length);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(dax_truncate_page);

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

  reply	other threads:[~2014-04-08 22:17 UTC|newest]

Thread overview: 180+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-23 19:08 [PATCH v7 00/22] Support ext4 on NV-DIMMs Matthew Wilcox
2014-03-23 19:08 ` Matthew Wilcox
2014-03-23 19:08 ` [PATCH v7 01/22] Fix XIP fault vs truncate race Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-03-29 15:57   ` Jan Kara
2014-03-29 15:57     ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 02/22] Allow page fault handlers to perform the COW Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-08 16:34   ` Jan Kara
2014-04-08 16:34     ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 03/22] axonram: Fix bug in direct_access Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-03-29 16:22   ` Jan Kara
2014-03-29 16:22     ` Jan Kara
2014-04-02 19:24     ` Matthew Wilcox
2014-04-02 19:24       ` Matthew Wilcox
2014-03-23 19:08 ` [PATCH v7 04/22] Change direct_access calling convention Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-03-29 16:30   ` Jan Kara
2014-03-29 16:30     ` Jan Kara
2014-04-02 19:27     ` Matthew Wilcox
2014-04-02 19:27       ` Matthew Wilcox
2014-03-23 19:08 ` [PATCH v7 05/22] Introduce IS_DAX(inode) Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-08 15:32   ` Jan Kara
2014-04-08 15:32     ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 06/22] Replace XIP read and write with DAX I/O Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-08 17:56   ` Jan Kara
2014-04-08 17:56     ` Jan Kara
2014-04-08 20:21     ` Matthew Wilcox
2014-04-08 20:21       ` Matthew Wilcox
2014-04-09  9:14       ` Jan Kara
2014-04-09  9:14         ` Jan Kara
2014-04-09 15:19         ` Matthew Wilcox
2014-04-09 15:19           ` Matthew Wilcox
2014-04-09 20:55           ` Jan Kara
2014-04-09 20:55             ` Jan Kara
2014-04-13 18:05             ` Matthew Wilcox
2014-04-13 18:05               ` Matthew Wilcox
2014-04-09 12:04   ` Jan Kara
2014-04-09 12:04     ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 07/22] Replace the XIP page fault handler with the DAX page fault handler Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-08 22:05   ` Jan Kara
2014-04-08 22:05     ` Jan Kara
2014-04-09 20:48     ` Matthew Wilcox
2014-04-09 20:48       ` Matthew Wilcox
2014-04-09 21:12       ` Jan Kara
2014-04-09 21:12         ` Jan Kara
2014-04-13 11:21         ` Matthew Wilcox
2014-04-13 11:21           ` Matthew Wilcox
2014-04-14 16:04           ` Jan Kara
2014-04-14 16:04             ` Jan Kara
2014-04-09 10:27   ` Jan Kara
2014-04-09 10:27     ` Jan Kara
2014-04-09 20:51     ` Matthew Wilcox
2014-04-09 20:51       ` Matthew Wilcox
2014-04-09 21:43       ` Jan Kara
2014-04-09 21:43         ` Jan Kara
2014-04-13 18:03         ` Matthew Wilcox
2014-04-13 18:03           ` Matthew Wilcox
2014-07-29 12:12         ` Matthew Wilcox
2014-07-29 12:12           ` Matthew Wilcox
2014-07-29 21:04           ` Jan Kara
2014-07-29 21:04             ` Jan Kara
2014-07-29 21:23             ` Matthew Wilcox
2014-07-29 21:23               ` Matthew Wilcox
2014-07-30  9:52               ` Jan Kara
2014-07-30  9:52                 ` Jan Kara
2014-07-30 21:02                 ` Matthew Wilcox
2014-07-30 21:02                   ` Matthew Wilcox
2014-08-09 11:00                 ` Matthew Wilcox
2014-08-09 11:00                   ` Matthew Wilcox
2014-08-11  8:51                   ` Jan Kara
2014-08-11  8:51                     ` Jan Kara
2014-08-11 14:13                     ` Matthew Wilcox
2014-08-11 14:13                       ` Matthew Wilcox
2014-08-11 14:35                       ` Jan Kara
2014-08-11 14:35                         ` Jan Kara
2014-08-11 15:02                         ` Matthew Wilcox
2014-08-11 15:02                           ` Matthew Wilcox
2014-08-11 15:25                           ` Jan Kara
2014-08-11 15:25                             ` Jan Kara
2014-05-21 20:35   ` Toshi Kani
2014-05-21 20:35     ` Toshi Kani
2014-06-05 22:38     ` Toshi Kani
2014-06-05 22:38       ` Toshi Kani
2014-03-23 19:08 ` [PATCH v7 08/22] Replace xip_truncate_page with dax_truncate_page Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-08 22:17   ` Jan Kara [this message]
2014-04-08 22:17     ` Jan Kara
2014-04-09  9:26     ` Jan Kara
2014-04-09  9:26       ` Jan Kara
2014-04-13 19:07       ` Matthew Wilcox
2014-04-13 19:07         ` Matthew Wilcox
2014-03-23 19:08 ` [PATCH v7 09/22] Remove mm/filemap_xip.c Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-08 18:21   ` Jan Kara
2014-04-08 18:21     ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 10/22] Remove get_xip_mem Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-08 18:20   ` Jan Kara
2014-04-08 18:20     ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 11/22] Replace ext2_clear_xip_target with dax_clear_blocks Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-09  9:46   ` Jan Kara
2014-04-09  9:46     ` Jan Kara
2014-04-10 14:16     ` Matthew Wilcox
2014-04-10 14:16       ` Matthew Wilcox
2014-04-10 18:31       ` Jan Kara
2014-04-10 18:31         ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 12/22] ext2: Remove ext2_xip_verify_sb() Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-09  9:52   ` Jan Kara
2014-04-09  9:52     ` Jan Kara
2014-04-10 14:22     ` Matthew Wilcox
2014-04-10 14:22       ` Matthew Wilcox
2014-04-10 18:35       ` Jan Kara
2014-04-10 18:35         ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 13/22] ext2: Remove ext2_use_xip Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-09  9:55   ` Jan Kara
2014-04-09  9:55     ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 14/22] ext2: Remove xip.c and xip.h Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-09  9:59   ` Jan Kara
2014-04-09  9:59     ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 15/22] Remove CONFIG_EXT2_FS_XIP and rename CONFIG_FS_XIP to CONFIG_FS_DAX Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-09  9:59   ` Jan Kara
2014-04-09  9:59     ` Jan Kara
2014-04-10 14:23     ` Matthew Wilcox
2014-04-10 14:23       ` Matthew Wilcox
2014-03-23 19:08 ` [PATCH v7 16/22] ext2: Remove ext2_aops_xip Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-09 10:02   ` Jan Kara
2014-04-09 10:02     ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 17/22] Get rid of most mentions of XIP in ext2 Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-09 10:04   ` Jan Kara
2014-04-09 10:04     ` Jan Kara
2014-04-10 14:26     ` Matthew Wilcox
2014-04-10 14:26       ` Matthew Wilcox
2014-04-10 18:40       ` Jan Kara
2014-04-10 18:40         ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 18/22] xip: Add xip_zero_page_range Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-09 10:15   ` Jan Kara
2014-04-09 10:15     ` Jan Kara
2014-04-10 14:27     ` Matthew Wilcox
2014-04-10 14:27       ` Matthew Wilcox
2014-04-10 18:43       ` Jan Kara
2014-04-10 18:43         ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 19/22] ext4: Make ext4_block_zero_page_range static Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-03-24 19:11   ` tytso
2014-03-24 19:11     ` tytso
2014-03-23 19:08 ` [PATCH v7 20/22] ext4: Add DAX functionality Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-09 12:17   ` Jan Kara
2014-04-09 12:17     ` Jan Kara
2014-03-23 19:08 ` [PATCH v7 21/22] ext4: Fix typos Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-03-24 19:16   ` tytso
2014-03-24 19:16     ` tytso
2014-03-23 19:08 ` [PATCH v7 22/22] brd: Rename XIP to DAX Matthew Wilcox
2014-03-23 19:08   ` Matthew Wilcox
2014-04-09 10:07   ` Jan Kara
2014-04-09 10:07     ` Jan Kara
2014-05-18 14:58 ` [PATCH v7 00/22] Support ext4 on NV-DIMMs Boaz Harrosh
2014-05-18 14:58   ` Boaz Harrosh
2014-05-18 23:24   ` Matthew Wilcox
2014-05-18 23:24     ` Matthew Wilcox
2014-06-17 18:11 ` Boaz Harrosh
2014-06-17 18:11   ` Boaz Harrosh
2014-06-17 18:19   ` Matthew Wilcox
2014-06-17 18:19     ` Matthew Wilcox
2014-06-17 18:39     ` Boaz Harrosh
2014-06-17 18:39       ` Boaz Harrosh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140408221759.GD26019@quack.suse.cz \
    --to=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=matthew.r.wilcox@intel.com \
    --cc=willy@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.