All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Jan Kara <jack@suse.cz>, Matthew Wilcox <mawilcox@microsoft.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	linux-nvdimm@lists.01.org, Ingo Molnar <mingo@redhat.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	linux-fsdevel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH 05/13] x86, dax: replace clear_pmem() with open coded memset + dax_ops->flush
Date: Fri, 20 Jan 2017 11:27:17 +0100	[thread overview]
Message-ID: <20170120102717.GI14115@quack2.suse.cz> (raw)
In-Reply-To: <148488423988.37913.16814081637297710444.stgit@dwillia2-desk3.amr.corp.intel.com>

On Thu 19-01-17 19:50:39, Dan Williams wrote:
> The clear_pmem() helper simply combines a memset() plus a cache flush.
> Now that the flush routine is optionally provided by the dax device
> driver we can avoid unnecessary cache management on dax devices fronting
> volatile memory.
> 
> With clear_pmem() gone we can follow on with a patch to make pmem cache
> management completely defined within the pmem driver.
...
> diff --git a/fs/dax.c b/fs/dax.c
> index 160024e403f6..8883ce4d391e 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -986,6 +986,7 @@ static bool dax_range_is_aligned(struct block_device *bdev,
>  int __dax_zero_page_range(struct block_device *bdev, sector_t sector,
>  		unsigned int offset, unsigned int length)
>  {
> +	const struct dax_operations *dax_ops = to_dax_ops(bdev);
>  	struct blk_dax_ctl dax = {
>  		.sector		= sector,
>  		.size		= PAGE_SIZE,
> @@ -999,7 +1000,9 @@ int __dax_zero_page_range(struct block_device *bdev, sector_t sector,
>  	} else {
>  		if (dax_map_atomic(bdev, &dax) < 0)
>  			return PTR_ERR(dax.addr);
> -		clear_pmem(dax.addr + offset, length);
> +		memset(dax.addr + offset, 0, length);
> +		if (dax_ops->flush)
> +			dax_ops->flush(dax.addr + offset, length);
>  		dax_unmap_atomic(bdev, &dax);
>  	}
>  	return 0;

Shouldn't we rather have some callback in dax_ops for clearing memory?
If we had all accesses to persistent memory inside DAX code wrapped inside
appropriate device wrappers that can report errors, we can have proper
error handling for the case we hit MCE, can't we?

									Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz>
To: Dan Williams <dan.j.williams@intel.com>
Cc: linux-nvdimm@lists.01.org, Jan Kara <jack@suse.cz>,
	Matthew Wilcox <mawilcox@microsoft.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	Christoph Hellwig <hch@lst.de>, Jeff Moyer <jmoyer@redhat.com>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	linux-fsdevel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Ross Zwisler <ross.zwisler@linux.intel.com>
Subject: Re: [PATCH 05/13] x86, dax: replace clear_pmem() with open coded memset + dax_ops->flush
Date: Fri, 20 Jan 2017 11:27:17 +0100	[thread overview]
Message-ID: <20170120102717.GI14115@quack2.suse.cz> (raw)
In-Reply-To: <148488423988.37913.16814081637297710444.stgit@dwillia2-desk3.amr.corp.intel.com>

On Thu 19-01-17 19:50:39, Dan Williams wrote:
> The clear_pmem() helper simply combines a memset() plus a cache flush.
> Now that the flush routine is optionally provided by the dax device
> driver we can avoid unnecessary cache management on dax devices fronting
> volatile memory.
> 
> With clear_pmem() gone we can follow on with a patch to make pmem cache
> management completely defined within the pmem driver.
...
> diff --git a/fs/dax.c b/fs/dax.c
> index 160024e403f6..8883ce4d391e 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -986,6 +986,7 @@ static bool dax_range_is_aligned(struct block_device *bdev,
>  int __dax_zero_page_range(struct block_device *bdev, sector_t sector,
>  		unsigned int offset, unsigned int length)
>  {
> +	const struct dax_operations *dax_ops = to_dax_ops(bdev);
>  	struct blk_dax_ctl dax = {
>  		.sector		= sector,
>  		.size		= PAGE_SIZE,
> @@ -999,7 +1000,9 @@ int __dax_zero_page_range(struct block_device *bdev, sector_t sector,
>  	} else {
>  		if (dax_map_atomic(bdev, &dax) < 0)
>  			return PTR_ERR(dax.addr);
> -		clear_pmem(dax.addr + offset, length);
> +		memset(dax.addr + offset, 0, length);
> +		if (dax_ops->flush)
> +			dax_ops->flush(dax.addr + offset, length);
>  		dax_unmap_atomic(bdev, &dax);
>  	}
>  	return 0;

Shouldn't we rather have some callback in dax_ops for clearing memory?
If we had all accesses to persistent memory inside DAX code wrapped inside
appropriate device wrappers that can report errors, we can have proper
error handling for the case we hit MCE, can't we?

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

WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz>
To: Dan Williams <dan.j.williams@intel.com>
Cc: linux-nvdimm@ml01.01.org, Jan Kara <jack@suse.cz>,
	Matthew Wilcox <mawilcox@microsoft.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	Christoph Hellwig <hch@lst.de>, Jeff Moyer <jmoyer@redhat.com>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	linux-fsdevel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Ross Zwisler <ross.zwisler@linux.intel.com>
Subject: Re: [PATCH 05/13] x86, dax: replace clear_pmem() with open coded memset + dax_ops->flush
Date: Fri, 20 Jan 2017 11:27:17 +0100	[thread overview]
Message-ID: <20170120102717.GI14115@quack2.suse.cz> (raw)
In-Reply-To: <148488423988.37913.16814081637297710444.stgit@dwillia2-desk3.amr.corp.intel.com>

On Thu 19-01-17 19:50:39, Dan Williams wrote:
> The clear_pmem() helper simply combines a memset() plus a cache flush.
> Now that the flush routine is optionally provided by the dax device
> driver we can avoid unnecessary cache management on dax devices fronting
> volatile memory.
> 
> With clear_pmem() gone we can follow on with a patch to make pmem cache
> management completely defined within the pmem driver.
...
> diff --git a/fs/dax.c b/fs/dax.c
> index 160024e403f6..8883ce4d391e 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -986,6 +986,7 @@ static bool dax_range_is_aligned(struct block_device *bdev,
>  int __dax_zero_page_range(struct block_device *bdev, sector_t sector,
>  		unsigned int offset, unsigned int length)
>  {
> +	const struct dax_operations *dax_ops = to_dax_ops(bdev);
>  	struct blk_dax_ctl dax = {
>  		.sector		= sector,
>  		.size		= PAGE_SIZE,
> @@ -999,7 +1000,9 @@ int __dax_zero_page_range(struct block_device *bdev, sector_t sector,
>  	} else {
>  		if (dax_map_atomic(bdev, &dax) < 0)
>  			return PTR_ERR(dax.addr);
> -		clear_pmem(dax.addr + offset, length);
> +		memset(dax.addr + offset, 0, length);
> +		if (dax_ops->flush)
> +			dax_ops->flush(dax.addr + offset, length);
>  		dax_unmap_atomic(bdev, &dax);
>  	}
>  	return 0;

Shouldn't we rather have some callback in dax_ops for clearing memory?
If we had all accesses to persistent memory inside DAX code wrapped inside
appropriate device wrappers that can report errors, we can have proper
error handling for the case we hit MCE, can't we?

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

  reply	other threads:[~2017-01-20 10:27 UTC|newest]

Thread overview: 126+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-20  3:50 [PATCH 00/13] dax, pmem: move cpu cache maintenance to libnvdimm Dan Williams
2017-01-20  3:50 ` Dan Williams
2017-01-20  3:50 ` Dan Williams
2017-01-20  3:50 ` Dan Williams
2017-01-20  3:50 ` [PATCH 01/13] x86, dax, pmem: remove indirection around memcpy_from_pmem() Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20  3:50 ` [PATCH 02/13] block, dax: introduce dax_operations Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20  3:50   ` Dan Williams
     [not found]   ` <148488422405.37913.13366670089124790849.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-01-20 17:28     ` Dan Williams
2017-01-20 17:28       ` Dan Williams
2017-01-20 17:28       ` Dan Williams
2017-01-20 17:28       ` Dan Williams
2017-01-20  3:50 ` [PATCH 03/13] x86, dax, pmem: introduce 'copy_from_iter' dax operation Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-02-03  1:52   ` [lkp-robot] [x86, dax, pmem] 2e12109d1c: fio.write_bw_MBps -75% regression kernel test robot
2017-02-03  1:52     ` kernel test robot
2017-02-03  1:52     ` kernel test robot
2017-02-03  1:52     ` kernel test robot
2017-02-17  3:52   ` [PATCH 03/13] x86, dax, pmem: introduce 'copy_from_iter' dax operation Ross Zwisler
2017-02-17  3:52     ` Ross Zwisler
2017-02-17  3:52     ` Ross Zwisler
2017-02-17  3:56     ` Dan Williams
2017-02-17  3:56       ` Dan Williams
2017-02-17  3:56       ` Dan Williams
2017-01-20  3:50 ` [PATCH 04/13] dax, pmem: introduce an optional 'flush' " Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20  3:50 ` [PATCH 05/13] x86, dax: replace clear_pmem() with open coded memset + dax_ops->flush Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20 10:27   ` Jan Kara [this message]
2017-01-20 10:27     ` Jan Kara
2017-01-20 10:27     ` Jan Kara
2017-01-20 15:33     ` Dan Williams
2017-01-20 15:33       ` Dan Williams
2017-01-20 15:33       ` Dan Williams
2017-01-20  3:50 ` [PATCH 06/13] x86, dax, libnvdimm: move wb_cache_pmem() to libnvdimm Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20  3:50 ` [PATCH 07/13] x86, libnvdimm, pmem: move arch_invalidate_pmem() " Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20  3:50 ` [PATCH 08/13] x86, libnvdimm, dax: stop abusing __copy_user_nocache Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-01-20  3:50   ` Dan Williams
2017-03-28 16:21   ` Ross Zwisler
2017-03-28 16:21     ` Ross Zwisler
2017-03-28 16:21     ` Ross Zwisler
2017-03-28 16:26     ` Dan Williams
2017-03-28 16:26       ` Dan Williams
2017-03-28 16:26       ` Dan Williams
2017-01-20  3:51 ` [PATCH 09/13] libnvdimm, pmem: implement cache bypass for all copy_from_iter() operations Dan Williams
2017-01-20  3:51   ` Dan Williams
2017-01-20  3:51   ` Dan Williams
2017-01-20  3:51 ` [PATCH 10/13] libnvdimm, pmem: fix persistence warning Dan Williams
2017-01-20  3:51   ` Dan Williams
2017-01-20  3:51   ` Dan Williams
2017-01-20  3:51 ` [PATCH 11/13] libnvdimm, nfit: enable support for volatile ranges Dan Williams
2017-01-20  3:51   ` Dan Williams
2017-01-20  3:51   ` Dan Williams
2017-01-20  3:51 ` [PATCH 12/13] libnvdimm, pmem: disable dax flushing when pmem is fronting a volatile region Dan Williams
2017-01-20  3:51   ` Dan Williams
2017-01-20  3:51   ` Dan Williams
2017-01-20  3:51 ` [PATCH 13/13] libnvdimm, pmem: disable dax flushing for 'cache flush on fail' platforms Dan Williams
2017-01-20  3:51   ` Dan Williams
2017-01-20  3:51   ` Dan Williams
     [not found] ` <148488421301.37913.12835362165895864897.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-01-21 16:28   ` [PATCH 00/13] dax, pmem: move cpu cache maintenance to libnvdimm Matthew Wilcox
2017-01-21 16:28     ` Matthew Wilcox
2017-01-21 17:52     ` Christoph Hellwig
2017-01-21 17:52       ` Christoph Hellwig
2017-01-21 17:52       ` Christoph Hellwig
2017-01-21 17:52       ` Christoph Hellwig
     [not found]       ` <20170121175212.GA28180-jcswGhMUV9g@public.gmane.org>
2017-01-22 15:43         ` Matthew Wilcox
2017-01-22 15:43           ` Matthew Wilcox
2017-01-22 15:43           ` Matthew Wilcox
     [not found]           ` <BY2PR21MB00367799FE7B7E8302A99260CB730-vtcBUbTck+B5JOYzoceCCc1VXTxX1y3OvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-01-22 16:29             ` Christoph Hellwig
2017-01-22 16:29               ` Christoph Hellwig
2017-01-22 16:29               ` Christoph Hellwig
2017-01-22 16:29               ` Christoph Hellwig
2017-01-22 18:19               ` Matthew Wilcox
2017-01-22 18:19                 ` Matthew Wilcox
2017-01-22 18:30                 ` Christoph Hellwig
2017-01-22 18:30                   ` Christoph Hellwig
2017-01-22 18:30                   ` Christoph Hellwig
2017-01-22 18:30                   ` Christoph Hellwig
     [not found]                   ` <20170122183046.GA7359-jcswGhMUV9g@public.gmane.org>
2017-01-22 18:39                     ` Matthew Wilcox
2017-01-22 18:39                       ` Matthew Wilcox
2017-01-22 18:39                       ` Matthew Wilcox
     [not found]                       ` <BY2PR21MB0036CC7935BFE438EA001763CB730-vtcBUbTck+B5JOYzoceCCc1VXTxX1y3OvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-01-22 18:44                         ` Christoph Hellwig
2017-01-22 18:44                           ` Christoph Hellwig
2017-01-22 18:44                           ` Christoph Hellwig
2017-01-22 18:44                           ` Christoph Hellwig
2017-01-23  6:37                           ` Matthew Wilcox
2017-01-23  6:37                             ` Matthew Wilcox
     [not found]                             ` <BY2PR21MB0036CA85562DDD21814C0B27CB720-vtcBUbTck+B5JOYzoceCCc1VXTxX1y3OvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-01-23  7:10                               ` Dan Williams
2017-01-23  7:10                                 ` Dan Williams
2017-01-23  7:10                                 ` Dan Williams
2017-01-23  7:10                                 ` Dan Williams
2017-01-23 16:00                                 ` Christoph Hellwig
2017-01-23 16:00                                   ` Christoph Hellwig
2017-01-23 16:00                                   ` Christoph Hellwig
2017-01-23 17:14                                   ` Dan Williams
2017-01-23 17:14                                     ` Dan Williams
2017-01-23 17:14                                     ` Dan Williams
     [not found]                                     ` <CAPcyv4gAbwS9yKNgAN9ytpDg7Jqh1FubZbGSfbFP0f-DdXPpCg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-01-23 18:03                                       ` Christoph Hellwig
2017-01-23 18:03                                         ` Christoph Hellwig
2017-01-23 18:03                                         ` Christoph Hellwig
2017-01-23 18:03                                         ` Christoph Hellwig
     [not found]                                         ` <20170123180314.GA23073-jcswGhMUV9g@public.gmane.org>
2017-01-23 18:31                                           ` Dan Williams
2017-01-23 18:31                                             ` Dan Williams
2017-01-23 18:31                                             ` Dan Williams
2017-01-23 18:31                                             ` Dan Williams
2017-01-23 15:58                             ` Christoph Hellwig
2017-01-23 15:58                               ` Christoph Hellwig
2017-01-23 15:58                               ` Christoph Hellwig
2017-01-22 17:30         ` Dan Williams
2017-01-22 17:30           ` Dan Williams
2017-01-22 17:30           ` Dan Williams
2017-01-22 17:30           ` Dan Williams
     [not found]           ` <CAPcyv4jEXsjw_Mo3aLRFmJr8ThqLPJPjdPjz7Q3ZS0ZC-AaDBw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-01-23 16:01             ` Christoph Hellwig
2017-01-23 16:01               ` Christoph Hellwig
2017-01-23 16:01               ` Christoph Hellwig
2017-01-23 16:01               ` Christoph Hellwig

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=20170120102717.GI14115@quack2.suse.cz \
    --to=jack@suse.cz \
    --cc=dan.j.williams@intel.com \
    --cc=hch@lst.de \
    --cc=hpa@zytor.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=mawilcox@microsoft.com \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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.