All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Dan Williams <dan.j.williams@intel.com>
Cc: linux-kernel@vger.kernel.org, ross.zwisler@linux.intel.com,
	mcgrof@suse.com, hch@lst.de, linux-nvdimm@lists.01.org
Subject: Re: [PATCH v4 08/10] pmem: convert to generic memremap
Date: Tue, 11 Aug 2015 14:55:56 +0200	[thread overview]
Message-ID: <20150811125556.GD3402@lst.de> (raw)
In-Reply-To: <20150811033833.4987.62222.stgit@otcpl-bsw-rvp-2.jf.intel.com>

On Mon, Aug 10, 2015 at 11:38:33PM -0400, Dan Williams wrote:
> Update memremap_pmem() to query the architecture for the mapping type of
> the given persistent memory range  and then pass those flags to generic
> memremap().  arch_memremap_pmem_flags() is provided an address range to
> evaluate in the event an arch has a need for different mapping types by
> address range.  For example the ACPI NFIT carries EFI mapping types in
> its memory range description table.
> 
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Looks generally good, but a little nipick related to my comment
from two patches ago:

> +unsigned long arch_memremap_pmem_flags(resource_size_t offset, size_t size)
> +{
> +	/*
> +	 * The expectation is that pmem is always WB capable range on
> +	 * x86, i.e. no need to walk the range.
> +	 */
> +	return MEMREMAP_WB;
> +}
> +EXPORT_SYMBOL(arch_memremap_pmem_flags);

Why not just add a 

/*
 * The expectation is that pmem is always WB capable range on x86, i.e. no
 * need to walk the range.
 */
#define ARCH_MEMREMAP_PMEM	MEMREMAP_WB

in io.h, and then kill all these little pmem wrappers and:

>  static inline void __pmem *memremap_pmem(resource_size_t offset,
>  		unsigned long size)
>  {
> +	unsigned long flags;
> +
>  	if (arch_has_pmem_api())
> -		return arch_memremap_pmem(offset, size);
> -	return default_memremap_pmem(offset, size);
> +		flags = arch_memremap_pmem_flags(offset, size);
> +	else
> +		flags = default_memremap_pmem_flags();
> +
> +	return (void __pmem *) memremap(offset, size, flags);

#ifdef ARCH_MEMREMAP_PMEM
	return (void __pmem *) memremap(offset, size, ARCH_MEMREMAP_PMEM);
#else
	return (void __pmem *) memremap(offset, size, MEMREMAP_WT);
#endif


WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de>
To: Dan Williams <dan.j.williams@intel.com>
Cc: linux-kernel@vger.kernel.org, ross.zwisler@linux.intel.com,
	mcgrof@suse.com, hch@lst.de, linux-nvdimm@ml01.01.org
Subject: Re: [PATCH v4 08/10] pmem: convert to generic memremap
Date: Tue, 11 Aug 2015 14:55:56 +0200	[thread overview]
Message-ID: <20150811125556.GD3402@lst.de> (raw)
In-Reply-To: <20150811033833.4987.62222.stgit@otcpl-bsw-rvp-2.jf.intel.com>

On Mon, Aug 10, 2015 at 11:38:33PM -0400, Dan Williams wrote:
> Update memremap_pmem() to query the architecture for the mapping type of
> the given persistent memory range  and then pass those flags to generic
> memremap().  arch_memremap_pmem_flags() is provided an address range to
> evaluate in the event an arch has a need for different mapping types by
> address range.  For example the ACPI NFIT carries EFI mapping types in
> its memory range description table.
> 
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Looks generally good, but a little nipick related to my comment
from two patches ago:

> +unsigned long arch_memremap_pmem_flags(resource_size_t offset, size_t size)
> +{
> +	/*
> +	 * The expectation is that pmem is always WB capable range on
> +	 * x86, i.e. no need to walk the range.
> +	 */
> +	return MEMREMAP_WB;
> +}
> +EXPORT_SYMBOL(arch_memremap_pmem_flags);

Why not just add a 

/*
 * The expectation is that pmem is always WB capable range on x86, i.e. no
 * need to walk the range.
 */
#define ARCH_MEMREMAP_PMEM	MEMREMAP_WB

in io.h, and then kill all these little pmem wrappers and:

>  static inline void __pmem *memremap_pmem(resource_size_t offset,
>  		unsigned long size)
>  {
> +	unsigned long flags;
> +
>  	if (arch_has_pmem_api())
> -		return arch_memremap_pmem(offset, size);
> -	return default_memremap_pmem(offset, size);
> +		flags = arch_memremap_pmem_flags(offset, size);
> +	else
> +		flags = default_memremap_pmem_flags();
> +
> +	return (void __pmem *) memremap(offset, size, flags);

#ifdef ARCH_MEMREMAP_PMEM
	return (void __pmem *) memremap(offset, size, ARCH_MEMREMAP_PMEM);
#else
	return (void __pmem *) memremap(offset, size, MEMREMAP_WT);
#endif


  reply	other threads:[~2015-08-11 12:55 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-11  3:37 [PATCH v4 00/10] memremap for 4.3 Dan Williams
2015-08-11  3:37 ` Dan Williams
2015-08-11  3:37 ` [PATCH v4 01/10] mm: enhance region_is_ram() to region_intersects() Dan Williams
2015-08-11  3:37   ` Dan Williams
2015-08-11  3:38 ` [PATCH v4 02/10] arch, drivers: don't include <asm/io.h> directly, use <linux/io.h> instead Dan Williams
2015-08-11  3:38   ` Dan Williams
2015-08-11  3:38 ` [PATCH v4 03/10] cleanup IORESOURCE_CACHEABLE vs ioremap() Dan Williams
2015-08-11  3:38   ` Dan Williams
2015-08-11  3:38 ` [PATCH v4 04/10] arch: introduce memremap() Dan Williams
2015-08-11  3:38   ` Dan Williams
2015-08-11 12:47   ` Christoph Hellwig
2015-08-11 12:47     ` Christoph Hellwig
2015-08-11  3:38 ` [PATCH v4 05/10] visorbus: switch from ioremap_cache to memremap Dan Williams
2015-08-11  3:38   ` Dan Williams
2015-08-11  3:38 ` [PATCH v4 06/10] libnvdimm, pmem: push call to ioremap_cache out of line Dan Williams
2015-08-11  3:38   ` Dan Williams
2015-08-11 12:51   ` Christoph Hellwig
2015-08-11 12:51     ` Christoph Hellwig
2015-08-11 16:20     ` Dan Williams
2015-08-11 16:20       ` Dan Williams
2015-08-11  3:38 ` [PATCH v4 07/10] pmem: switch from ioremap_wt to memremap Dan Williams
2015-08-11  3:38   ` Dan Williams
2015-08-11 12:52   ` Christoph Hellwig
2015-08-11 12:52     ` Christoph Hellwig
2015-08-11  3:38 ` [PATCH v4 08/10] pmem: convert to generic memremap Dan Williams
2015-08-11  3:38   ` Dan Williams
2015-08-11 12:55   ` Christoph Hellwig [this message]
2015-08-11 12:55     ` Christoph Hellwig
2015-08-11  3:38 ` [PATCH v4 09/10] devres: add devm_memremap Dan Williams
2015-08-11  3:38   ` Dan Williams
2015-08-11  3:38 ` [PATCH v4 10/10] pmem: switch to devm_ allocations Dan Williams
2015-08-11  3:38   ` Dan Williams

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=20150811125556.GD3402@lst.de \
    --to=hch@lst.de \
    --cc=dan.j.williams@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=mcgrof@suse.com \
    --cc=ross.zwisler@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.