Re: [x86/copy_mc] a0ac629ebe: fio.read_iops -43.3% regression

2020-08-06 Thread Ingo Molnar


* Dan Williams  wrote:

> On Thu, Aug 6, 2020 at 6:35 AM Ingo Molnar  wrote:
> >
> >
> > * kernel test robot  wrote:
> >
> > > Greeting,
> > >
> > > FYI, we noticed a -43.3% regression of fio.read_iops due to commit:
> > >
> > >
> > > commit: a0ac629ebe7b3d248cb93807782a00d9142fdb98 ("x86/copy_mc: Introduce 
> > > copy_mc_generic()")
> > > url: 
> > > https://github.com/0day-ci/linux/commits/Dan-Williams/Renovate-memcpy_mcsafe-with-copy_mc_to_-user-kernel/20200802-014046
> > >
> > >
> > > in testcase: fio-basic
> > > on test machine: 96 threads Intel(R) Xeon(R) Gold 6252 CPU @ 2.10GHz with 
> > > 256G memory
> > > with following parameters:
> >
> > So this performance regression, if it isn't a spurious result, looks
> > concerning. Is this expected?
> 
> This is not expected and I think delays these patches until I'm back
> from leave in a few weeks. I know that we might lose some inlining
> effect due to replacing native memcpy, but I did not expect it would
> have an impact like this. In my testing I was seeing a performance
> improvement from replacing the careful / open-coded copy with rep;
> mov;, which increases the surprise of this result.

It would be nice to double check this on the kernel-test-robot side as 
well, to make sure it's not a false positive.

Thanks,

Ingo
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


Re: [x86/copy_mc] a0ac629ebe: fio.read_iops -43.3% regression

2020-08-06 Thread Ingo Molnar


* kernel test robot  wrote:

> Greeting,
> 
> FYI, we noticed a -43.3% regression of fio.read_iops due to commit:
> 
> 
> commit: a0ac629ebe7b3d248cb93807782a00d9142fdb98 ("x86/copy_mc: Introduce 
> copy_mc_generic()")
> url: 
> https://github.com/0day-ci/linux/commits/Dan-Williams/Renovate-memcpy_mcsafe-with-copy_mc_to_-user-kernel/20200802-014046
> 
> 
> in testcase: fio-basic
> on test machine: 96 threads Intel(R) Xeon(R) Gold 6252 CPU @ 2.10GHz with 
> 256G memory
> with following parameters:

So this performance regression, if it isn't a spurious result, looks 
concerning. Is this expected?

Thanks,

Ingo
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


Re: [PATCH v4 0/6] Memory Hierarchy: Enable target node lookups for reserved memory

2020-02-13 Thread Ingo Molnar


* Dan Williams  wrote:

> On Tue, Jan 21, 2020 at 7:20 PM Dan Williams  wrote:
> >
> > Changes since v3 [1]:
> > - Cleanup numa_map_to_online_node() to remove redundant "if
> >   (!node_online(node))" (Aneesh)
> >
> > [1]: 
> > http://lore.kernel.org/r/157954696789.2239526.17707265517154476652.st...@dwillia2-desk3.amr.corp.intel.com
> >
> > ---
> >
> > Merge notes:
> >
> > x86 folks: This has an ack from Rafael for ACPI, and Michael for Power.
> > With an x86 ack I plan to take this through the libnvdimm tree provided
> > the x86 touches look ok to you.
> 
> Ping x86 folks. There's no additional changes identified for this
> series. Can I request an ack to take it through libnvdimm.git? Do you
> need a resend?
> 
> x86/mm: Introduce CONFIG_KEEP_NUMA
>     x86/numa: Provide a range-to-target_node lookup facility

If the minor complaints I outlined are addressed:

Reviewed-by: Ingo Molnar 

Thanks,

Ingo
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


Re: [PATCH v4 5/6] x86/numa: Provide a range-to-target_node lookup facility

2020-02-13 Thread Ingo Molnar


* Dan Williams  wrote:

> The DEV_DAX_KMEM facility is a generic mechanism to allow device-dax
> instances, fronting performance-differentiated-memory like pmem, to be
> added to the System RAM pool. The numa node for that hot-added memory is
> derived from the device-dax instance's 'target_node' attribute.
> 
> Recall that the 'target_node' is the ACPI-PXM-to-node translation for
> memory when it comes online whereas the 'numa_node' attribute of the
> device represents the closest online cpu node.
> 
> Presently useful target_node information from the ACPI SRAT is discarded
> with the expectation that "Reserved" memory will never be onlined. Now,
> DEV_DAX_KMEM violates that assumption, there is a need to retain the
> translation. Move, rather than discard, numa_memblk data to a secondary
> array that memory_add_physaddr_to_target_node() may consider at a later
> point in time.
> 
> Cc: Dave Hansen 
> Cc: Andy Lutomirski 
> Cc: Peter Zijlstra 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: "H. Peter Anvin" 
> Cc: 
> Cc: Andrew Morton 
> Cc: David Hildenbrand 
> Cc: Michal Hocko 
> Reported-by: kbuild test robot 
> Signed-off-by: Dan Williams 
> ---
>  arch/x86/mm/numa.c   |   68 
> +++---
>  include/linux/numa.h |8 +-
>  mm/mempolicy.c   |5 
>  3 files changed, 70 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index 5289d9d6799a..f2c8fca36f28 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -26,6 +26,7 @@ struct pglist_data *node_data[MAX_NUMNODES] __read_mostly;
>  EXPORT_SYMBOL(node_data);
>  
>  static struct numa_meminfo numa_meminfo __initdata_numa;
> +static struct numa_meminfo numa_reserved_meminfo __initdata_numa;
>  
>  static int numa_distance_cnt;
>  static u8 *numa_distance;
> @@ -164,6 +165,26 @@ void __init numa_remove_memblk_from(int idx, struct 
> numa_meminfo *mi)
>   (mi->nr_blks - idx) * sizeof(mi->blk[0]));
>  }
>  
> +/**
> + * numa_move_memblk - Move one numa_memblk from one numa_meminfo to another
> + * @dst: numa_meminfo to move block to
> + * @idx: Index of memblk to remove
> + * @src: numa_meminfo to remove memblk from
> + *
> + * If @dst is non-NULL add it at the @dst->nr_blks index and increment
> + * @dst->nr_blks, then remove it from @src.
> + */
> +static void __init numa_move_memblk(struct numa_meminfo *dst, int idx,
> + struct numa_meminfo *src)

Nit, this is obviously not how we format function definitions if 
checkpatch complains about the col80 limit.


> +{
> + if (dst) {
> + memcpy(>blk[dst->nr_blks], >blk[idx],
> + sizeof(struct numa_memblk));

This linebreak is actually unnecessary ...

Thanks,

Ingo
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


Re: [PATCH v4 4/6] x86/mm: Introduce CONFIG_KEEP_NUMA

2020-02-13 Thread Ingo Molnar


* Dan Williams  wrote:

> Currently x86 numa_meminfo is marked __initdata in the
> CONFIG_MEMORY_HOTPLUG=n case. In support of a new facility to allow
> drivers to map reserved memory to a 'target_node'
> (phys_to_target_node()), add support for removing the __initdata
> designation for those users. Both memory hotplug and
> phys_to_target_node() users select CONFIG_KEEP_NUMA to tell the arch to
> maintain its physical address to numa mapping infrastructure post init.
> 
> Cc: Dave Hansen 
> Cc: Andy Lutomirski 
> Cc: Peter Zijlstra 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: "H. Peter Anvin" 
> Cc: 
> Cc: Andrew Morton 
> Cc: David Hildenbrand 
> Cc: Michal Hocko 
> Signed-off-by: Dan Williams 
> ---
>  arch/x86/mm/numa.c   |6 +-
>  include/linux/numa.h |6 ++
>  mm/Kconfig   |5 +
>  3 files changed, 12 insertions(+), 5 deletions(-)

The concept and the x86 portions look sane, just a few minor nits:

> 
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index 99f7a68738f0..5289d9d6799a 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -25,11 +25,7 @@ nodemask_t numa_nodes_parsed __initdata;
>  struct pglist_data *node_data[MAX_NUMNODES] __read_mostly;
>  EXPORT_SYMBOL(node_data);
>  
> -static struct numa_meminfo numa_meminfo
> -#ifndef CONFIG_MEMORY_HOTPLUG
> -__initdata
> -#endif
> -;
> +static struct numa_meminfo numa_meminfo __initdata_numa;
>  
>  static int numa_distance_cnt;
>  static u8 *numa_distance;
> diff --git a/include/linux/numa.h b/include/linux/numa.h
> index 20f4e44b186c..c005ed6b807b 100644
> --- a/include/linux/numa.h
> +++ b/include/linux/numa.h
> @@ -13,6 +13,12 @@
>  
>  #define  NUMA_NO_NODE(-1)
>  
> +#ifdef CONFIG_KEEP_NUMA
> +#define __initdata_numa
> +#else
> +#define __initdata_numa __initdata
> +#endif
> +
>  #ifdef CONFIG_NUMA
>  int numa_map_to_online_node(int node);
>  #else
> diff --git a/mm/Kconfig b/mm/Kconfig
> index ab80933be65f..001f1185eadf 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -139,6 +139,10 @@ config HAVE_FAST_GUP
>  config ARCH_KEEP_MEMBLOCK
>   bool
>  
> +# Keep arch numa mapping infrastructure post-init.

s/numa/NUMA

Please also capitalize consistently in the rest of the series.

> +config KEEP_NUMA
> + bool


So most of our recent new NUMA options followed the naming pattern of:

  CONFIG_NUMA_*

Such as CONFIG_NUMA_BALANCING or CONFIG_NUMA_EMU.

So I'd suggesting naming it to CONFIG_NUMA_KEEP, or, a bit more 
descriptively, such as CONFIG_NUMA_KEEP_MAPPING or such?

'Keeping NUMA' is kind of lame - of course we keep NUMA. ;-)

Thanks,

Ingo
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


Re: [PATCH v6 00/13] mm: Teach memory_failure() about ZONE_DEVICE pages

2018-07-24 Thread Ingo Molnar


* Dave Jiang  wrote:

> Ingo,
> Is it possible to ack the x86 bits in this patch series? I'm hoping to
> get this pulled through the libnvdimm tree for 4.19. Thanks!

With the minor typo fixed in the first patch, both patches are looking good to 
me:

  Acked-by: Ingo Molnar 

Assuming that it gets properly tested, etc.

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v6 11/13] x86/mm/pat: Prepare {reserve, free}_memtype() for "decoy" addresses

2018-07-24 Thread Ingo Molnar


* Dan Williams  wrote:

> In preparation for using set_memory_uc() instead set_memory_np() for
> isolating poison from speculation, teach the memtype code to sanitize
> physical addresses vs __PHYSICAL_MASK.
> 
> The motivation for using set_memory_uc() for this case is to allow
> ongoing access to persistent memory pages via the pmem-driver +
> memcpy_mcsafe() until the poison is repaired.
> 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: "H. Peter Anvin" 
> Cc: Tony Luck 
> Cc: Borislav Petkov 
> Cc: 
> Cc: 
> Signed-off-by: Dan Williams 
> ---
>  arch/x86/mm/pat.c |   16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
> index 1555bd7d3449..6788ffa990f8 100644
> --- a/arch/x86/mm/pat.c
> +++ b/arch/x86/mm/pat.c
> @@ -512,6 +512,17 @@ static int free_ram_pages_type(u64 start, u64 end)
>   return 0;
>  }
>  
> +static u64 sanitize_phys(u64 address)
> +{
> + /*
> +  * When changing the memtype for pages containing poison allow
> +  * for a "decoy" virtual address (bit 63 clear) passed to
> +  * set_memory_X(). __pa() on a "decoy" address results in a
> +  * physical address with it 63 set.
> +  */
> + return address & __PHYSICAL_MASK;

s/it/bit

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: use memcpy_mcsafe() for copy_to_iter() (was: Re: [PATCH v3 0/9] Series short description)

2018-05-15 Thread Ingo Molnar

* Dan Williams <dan.j.willi...@intel.com> wrote:

> On Mon, May 14, 2018 at 12:26 AM, Ingo Molnar <mi...@kernel.org> wrote:
> >
> > * Dan Williams <dan.j.willi...@intel.com> wrote:
> >
> >> Ingo, Thomas, Al, any concerns with this series?
> >
> > Yeah, so:
> >
> >"[PATCH v3 0/9] Series short description"
> >
> > ... isn't the catchiest of titles to capture my [all too easily distracted]
> > attention! ;-)
> 
> My bad! After that mistake it became a toss-up between more spam and
> hoping the distraction would not throw you off.
> 
> > I have marked it now for -tip processing. Linus was happy with this and 
> > acked the
> > approach, right?
> 
> I think "happy" is a strong word when it comes to x86 machine check
> handling. My interpretation is that he and Andy acquiesced that this
> is about the best we can do with dax+mce as things stand today.

So, how would you like to go about this series?

To help move it forward I applied the first 5 commits to tip:x86/dax, on a
vanilla v4.17-rc5 base, did some minor edits to the changelogs, tested it
superficially (I don't have DAX so this essentially means build tests) and
pushed out the result.

Barring some later generic-x86 regression (unlikely) this looks good to me - 
feel 
free to cross-pull that branch into your DAX/nvdimm tree.

Or we could apply the remaining changes to -tip too - your call.

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: use memcpy_mcsafe() for copy_to_iter() (was: Re: [PATCH v3 0/9] Series short description)

2018-05-14 Thread Ingo Molnar

* Dan Williams  wrote:

> Ingo, Thomas, Al, any concerns with this series?

Yeah, so:

   "[PATCH v3 0/9] Series short description"

... isn't the catchiest of titles to capture my [all too easily distracted] 
attention! ;-)

I have marked it now for -tip processing. Linus was happy with this and acked 
the 
approach, right?

Thanks,

Ingo

> 
> On Thu, May 3, 2018 at 5:06 PM, Dan Williams  wrote:
> > Changes since v2 [1]:
> >
> > * Fix source address increment in mcsafe_handle_tail() (Mika)
> >
> > * Extend the unit test to inject simulated write faults and validate
> >   that data is properly transferred.
> >
> > * Rename MCSAFE_DEBUG to MCSAFE_TEST
> >
> > [1]: https://lists.01.org/pipermail/linux-nvdimm/2018-May/015583.html
> >
> > ---
> >
> > Currently memcpy_mcsafe() is only deployed in the pmem driver when
> > reading through a /dev/pmemX block device. However, a filesystem in dax
> > mode mounted on a /dev/pmemX block device will bypass the block layer
> > and the driver for reads. The filesystem-dax (fsdax) read case uses
> > dax_direct_access() and copy_to_iter() to bypass the block layer.
> >
> > The result of the bypass is that the kernel treats machine checks during
> > read as system fatal (reboot) when they could simply be flagged as an
> > I/O error, similar to performing reads through the pmem driver. Prevent
> > this fatal condition by deploying memcpy_mcsafe() in the fsdax read
> > path.
> >
> > The main differences between this copy_to_user_mcsafe() and
> > copy_user_generic_unrolled() are:
> >
> > * Typical tail/residue handling after a fault retries the copy
> >   byte-by-byte until the fault happens again. Re-triggering machine
> >   checks is potentially fatal so the implementation uses source alignment
> >   and poison alignment assumptions to avoid re-triggering machine
> >   checks.
> >
> > * SMAP coordination is handled external to the assembly with
> >   __uaccess_begin() and __uaccess_end().
> >
> > * ITER_KVEC and ITER_BVEC can now end prematurely with an error.
> >
> > The new MCSAFE_TEST facility is proposed as a way to unit test the
> > exception handling without requiring an ACPI EINJ capable platform.
> >
> > ---
> >
> > Dan Williams (9):
> >   x86, memcpy_mcsafe: remove loop unrolling
> >   x86, memcpy_mcsafe: add labels for write fault handling
> >   x86, memcpy_mcsafe: return bytes remaining
> >   x86, memcpy_mcsafe: add write-protection-fault handling
> >   x86, memcpy_mcsafe: define copy_to_iter_mcsafe()
> >   dax: introduce a ->copy_to_iter dax operation
> >   dax: report bytes remaining in dax_iomap_actor()
> >   pmem: switch to copy_to_iter_mcsafe()
> >   x86, nfit_test: unit test for memcpy_mcsafe()
> >
> >
> >  arch/x86/Kconfig   |1
> >  arch/x86/Kconfig.debug |3 +
> >  arch/x86/include/asm/mcsafe_test.h |   75 
> >  arch/x86/include/asm/string_64.h   |   10 ++-
> >  arch/x86/include/asm/uaccess_64.h  |   14 +
> >  arch/x86/lib/memcpy_64.S   |  112 
> > +---
> >  arch/x86/lib/usercopy_64.c |   21 +++
> >  drivers/dax/super.c|   10 +++
> >  drivers/md/dm-linear.c |   16 +
> >  drivers/md/dm-log-writes.c |   15 +
> >  drivers/md/dm-stripe.c |   21 +++
> >  drivers/md/dm.c|   25 
> >  drivers/nvdimm/claim.c |3 +
> >  drivers/nvdimm/pmem.c  |   13 +++-
> >  drivers/s390/block/dcssblk.c   |7 ++
> >  fs/dax.c   |   21 ---
> >  include/linux/dax.h|5 ++
> >  include/linux/device-mapper.h  |5 +-
> >  include/linux/string.h |4 +
> >  include/linux/uio.h|   15 +
> >  lib/iov_iter.c |   61 
> >  tools/testing/nvdimm/test/nfit.c   |  104 +
> >  22 files changed, 482 insertions(+), 79 deletions(-)
> >  create mode 100644 arch/x86/include/asm/mcsafe_test.h
> > ___
> > Linux-nvdimm mailing list
> > Linux-nvdimm@lists.01.org
> > https://lists.01.org/mailman/listinfo/linux-nvdimm
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h

2017-07-27 Thread Ingo Molnar

* Dan Williams <dan.j.willi...@intel.com> wrote:

> On Wed, Jul 26, 2017 at 10:19 AM, Ingo Molnar <mi...@kernel.org> wrote:
> >
> > * Dan Williams <dan.j.willi...@intel.com> wrote:
> >
> >> On Wed, Jul 26, 2017 at 4:29 AM, Ingo Molnar <mi...@kernel.org> wrote:
> >> >
> >> > * Dan Williams <dan.j.willi...@intel.com> wrote:
> >> >
> >> >> On Tue, Jul 25, 2017 at 4:55 PM, Arnaldo Carvalho de Melo
> >> >> <a...@kernel.org> wrote:
> >> >> > Em Tue, Jul 25, 2017 at 03:36:26PM -0700, Dan Williams escreveu:
> >> >> >> Replace the ccan implementation of list primitives, bitmap helpers 
> >> >> >> and
> >> >> >> small utility macros with the common definitions available in
> >> >> >> tool/include/linux.
> >> >> >
> >> >> > You should first add what you need in separate patches, paving the way
> >> >> > to then use it, and some stuff are already there, see below:
> >> >>
> >> >> Ok, I'll break out those changes separately.
> >> >
> >> > BTW., another general observation I have is that ndctl uses autotools - 
> >> > while perf
> >> > uses its own build system, some of which is abstracted out into 
> >> > tools/build/ and
> >> > reused by other tooling projects as well.
> >> >
> >> > I despise autotools with a passion, it's slow, bloated, and encourages 
> >> > all sorts
> >> > of bad API/ABI practices that plagues many OSS projects. I know that 
> >> > Linus
> >> > explicitly did a Makefile based build system for Git for (I think) 
> >> > similar
> >> > reasons.
> >> >
> >> > It might be a good idea to not let autotools into the kernel tooling 
> >> > tree, not
> >> > because ndctl's use of autotools is bad in any fashion (it appears to be 
> >> > a fairly
> >> > straightforward use), but to generally encourage good API/ABI practices 
> >> > in our
> >> > tooling space, and to encourage enhancements to the tools/build/ 
> >> > infrastructure.
> >>
> >> That's a fair point. Regardless, autotools will be in the git history,
> >> but if you'd like to see the final merge product eliminate its use, I
> >> can't really argue otherwise. I was originally not concerned because
> >> tools/usb/usbip/ was an existing in tree autotools user. In any event
> >> if you want the autotools removal to be done out-of-tree I'll need to
> >> put this effort on the back burner until 4.15.
> >
> > So that was another thing I wanted to suggest: why not import the current 
> > ndctl
> > version as a single commit?
> >
> > I had a quick look, and there's quite a few of commits in the ndctl history 
> > that
> > don't conform to kernel standards, such as:
> >
> >   ce881c1e78f6: ndctl: seed tracking
> >
> > which doesn't have any Signed-off-by tags.
> >
> > There's also commits with ambiguous titles that would be confusing in the 
> > kernel
> > context - for example:
> >
> >   796b6f373dec: clarify copyright and license information
> >
> > ... which on the surface could be misunderstood as something talking about 
> > the
> > kernel copyright ...
> >
> > Or:
> >
> >   e38bd36e5d0a: completion: updates for file name completion
> >
> > which I could initially mistake for a commit about scheduler completions ;-)
> >
> > Or:
> >
> >   2ad6a39c9ae9: Fix attribute sizes to match NFIT 0.8s2
> >   cc7cb44385d3: Import initial infrastructure
> >
> > etc.
> >
> > I suppose all that could be corrected, SOBs added, titles clarified and 
> > prefixed
> > with tools/ndctl, but then it wouldn't really be unmodified history anymore,
> > right?
> >
> > At that point we might as well do a clean start - and not import ~500 extra
> > commits into the kernel tree?
> 
> I think keeping the history is worth it, similar reasoning to why we
> kept the btrfs history.

Well, there's two key differences:

 - btrfs _is_ kernel code and as such was developed as a kernel module,
   albeit out of tree. ndctl is a standalone tooling project.

 - all the old btrfs commit titles are either prefixed with 'btrfs:', or have it
   as a string in the title, which made the merge unproblematic and unambiguous.

> [...] Also, since the history is linear and already rewritten by 'git 
> filter-branch' to move everything to tools/ndctl/ it wouldn't be that much 
> more 
> work to go clean up these few commits that are problematic.

So I think that's where to draw the line - grafting two kernel tree commit 
histories like for btrfs might be OK (just the repositories were incompatible), 
but if commit logs need to be rewritten manually then that's essentially 
whitewashing of history - which is a big no-no in Git flows.

Anyway, it's up to Linus I guess.

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h

2017-07-26 Thread Ingo Molnar

* Dan Williams <dan.j.willi...@intel.com> wrote:

> On Wed, Jul 26, 2017 at 4:29 AM, Ingo Molnar <mi...@kernel.org> wrote:
> >
> > * Dan Williams <dan.j.willi...@intel.com> wrote:
> >
> >> On Tue, Jul 25, 2017 at 4:55 PM, Arnaldo Carvalho de Melo
> >> <a...@kernel.org> wrote:
> >> > Em Tue, Jul 25, 2017 at 03:36:26PM -0700, Dan Williams escreveu:
> >> >> Replace the ccan implementation of list primitives, bitmap helpers and
> >> >> small utility macros with the common definitions available in
> >> >> tool/include/linux.
> >> >
> >> > You should first add what you need in separate patches, paving the way
> >> > to then use it, and some stuff are already there, see below:
> >>
> >> Ok, I'll break out those changes separately.
> >
> > BTW., another general observation I have is that ndctl uses autotools - 
> > while perf
> > uses its own build system, some of which is abstracted out into 
> > tools/build/ and
> > reused by other tooling projects as well.
> >
> > I despise autotools with a passion, it's slow, bloated, and encourages all 
> > sorts
> > of bad API/ABI practices that plagues many OSS projects. I know that Linus
> > explicitly did a Makefile based build system for Git for (I think) similar
> > reasons.
> >
> > It might be a good idea to not let autotools into the kernel tooling tree, 
> > not
> > because ndctl's use of autotools is bad in any fashion (it appears to be a 
> > fairly
> > straightforward use), but to generally encourage good API/ABI practices in 
> > our
> > tooling space, and to encourage enhancements to the tools/build/ 
> > infrastructure.
> 
> That's a fair point. Regardless, autotools will be in the git history,
> but if you'd like to see the final merge product eliminate its use, I
> can't really argue otherwise. I was originally not concerned because
> tools/usb/usbip/ was an existing in tree autotools user. In any event
> if you want the autotools removal to be done out-of-tree I'll need to
> put this effort on the back burner until 4.15.

So that was another thing I wanted to suggest: why not import the current ndctl 
version as a single commit?

I had a quick look, and there's quite a few of commits in the ndctl history 
that 
don't conform to kernel standards, such as:

  ce881c1e78f6: ndctl: seed tracking

which doesn't have any Signed-off-by tags.

There's also commits with ambiguous titles that would be confusing in the 
kernel 
context - for example:

  796b6f373dec: clarify copyright and license information

... which on the surface could be misunderstood as something talking about the 
kernel copyright ...

Or:

  e38bd36e5d0a: completion: updates for file name completion

which I could initially mistake for a commit about scheduler completions ;-)

Or:

  2ad6a39c9ae9: Fix attribute sizes to match NFIT 0.8s2
  cc7cb44385d3: Import initial infrastructure

etc.

I suppose all that could be corrected, SOBs added, titles clarified and 
prefixed 
with tools/ndctl, but then it wouldn't really be unmodified history anymore, 
right?

At that point we might as well do a clean start - and not import ~500 extra 
commits into the kernel tree?

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h

2017-07-26 Thread Ingo Molnar

* Dan Williams  wrote:

> On Tue, Jul 25, 2017 at 4:55 PM, Arnaldo Carvalho de Melo
>  wrote:
> > Em Tue, Jul 25, 2017 at 03:36:26PM -0700, Dan Williams escreveu:
> >> Replace the ccan implementation of list primitives, bitmap helpers and
> >> small utility macros with the common definitions available in
> >> tool/include/linux.
> >
> > You should first add what you need in separate patches, paving the way
> > to then use it, and some stuff are already there, see below:
> 
> Ok, I'll break out those changes separately.

BTW., another general observation I have is that ndctl uses autotools - while 
perf 
uses its own build system, some of which is abstracted out into tools/build/ 
and 
reused by other tooling projects as well.

I despise autotools with a passion, it's slow, bloated, and encourages all 
sorts 
of bad API/ABI practices that plagues many OSS projects. I know that Linus 
explicitly did a Makefile based build system for Git for (I think) similar 
reasons.

It might be a good idea to not let autotools into the kernel tooling tree, not 
because ndctl's use of autotools is bad in any fashion (it appears to be a 
fairly 
straightforward use), but to generally encourage good API/ABI practices in our 
tooling space, and to encourage enhancements to the tools/build/ infrastructure.

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: Moving ndctl development into the kernel tree?

2017-07-21 Thread Ingo Molnar

* Dan Williams  wrote:

> [...]
> 
> * Like perf, ndctl borrows the sub-command architecture and option
> parsing from git. So, this code could be refactored into something
> shared / generic, i.e. the bits in tools/perf/util/.

Just as a side note, stacktool (tools/stacktool/) is using the Git sub-command 
and 
options parsing code as well, and it's already sharing it with perf, via the 
tools/lib/subcmd/ library.

ndctl could use that as well.

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v2] x86, uaccess: introduce copy_from_iter_wt for pmem / writethrough operations

2017-05-07 Thread Ingo Molnar

* Dan Williams <dan.j.willi...@intel.com> wrote:

> On Sat, May 6, 2017 at 2:46 AM, Ingo Molnar <mi...@kernel.org> wrote:
> >
> > * Dan Williams <dan.j.willi...@intel.com> wrote:
> >
> >> On Fri, May 5, 2017 at 3:44 PM, Kani, Toshimitsu <toshi.k...@hpe.com> 
> >> wrote:
> >> > On Fri, 2017-05-05 at 15:25 -0700, Dan Williams wrote:
> >> >> On Fri, May 5, 2017 at 1:39 PM, Kani, Toshimitsu <toshi.k...@hpe.com>
> >> >> wrote:
> >> >  :
> >> >> > > ---
> >> >> > > Changes since the initial RFC:
> >> >> > > * s/writethru/wt/ since we already have ioremap_wt(),
> >> >> > > set_memory_wt(), etc. (Ingo)
> >> >> >
> >> >> > Sorry I should have said earlier, but I think the term "wt" is
> >> >> > misleading.  Non-temporal stores used in memcpy_wt() provide WC
> >> >> > semantics, not WT semantics.
> >> >>
> >> >> The non-temporal stores do, but memcpy_wt() is using a combination of
> >> >> non-temporal stores and explicit cache flushing.
> >> >>
> >> >> > How about using "nocache" as it's been
> >> >> > used in __copy_user_nocache()?
> >> >>
> >> >> The difference in my mind is that the "_nocache" suffix indicates
> >> >> opportunistic / optional cache pollution avoidance whereas "_wt"
> >> >> strictly arranges for caches not to contain dirty data upon
> >> >> completion of the routine. For example, non-temporal stores on older
> >> >> x86 cpus could potentially leave dirty data in the cache, so
> >> >> memcpy_wt on those cpus would need to use explicit cache flushing.
> >> >
> >> > I see.  I agree that its behavior is different from the existing one
> >> > with "_nocache".   That said, I think "wt" or "write-through" generally
> >> > means that writes allocate cachelines and keep them clean by writing to
> >> > memory.  So, subsequent reads to the destination will hit the
> >> > cachelines.  This is not the case with this interface.
> >>
> >> True... maybe _nocache_strict()? Or, leave it _wt() until someone
> >> comes along and is surprised that the cache is not warm for reads
> >> after memcpy_wt(), at which point we can ask "why not just use plain
> >> memcpy then?", or set the page-attributes to WT.
> >
> > Perhaps a _nocache_flush() postfix, to signal both that it's non-temporal 
> > and that
> > no cache line is left around afterwards (dirty or clean)?
> 
> Yes, I think "flush" belongs in the name, and to make it easily
> grep-able separate from _nocache we can call it _flushcache? An
> efficient implementation will use _nocache / non-temporal stores
> internally, but external consumers just care about the state of the
> cache after the call.

_flushcache() works for me too.

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v2] x86, uaccess: introduce copy_from_iter_wt for pmem / writethrough operations

2017-05-06 Thread Ingo Molnar

* Dan Williams  wrote:

> On Fri, May 5, 2017 at 3:44 PM, Kani, Toshimitsu  wrote:
> > On Fri, 2017-05-05 at 15:25 -0700, Dan Williams wrote:
> >> On Fri, May 5, 2017 at 1:39 PM, Kani, Toshimitsu 
> >> wrote:
> >  :
> >> > > ---
> >> > > Changes since the initial RFC:
> >> > > * s/writethru/wt/ since we already have ioremap_wt(),
> >> > > set_memory_wt(), etc. (Ingo)
> >> >
> >> > Sorry I should have said earlier, but I think the term "wt" is
> >> > misleading.  Non-temporal stores used in memcpy_wt() provide WC
> >> > semantics, not WT semantics.
> >>
> >> The non-temporal stores do, but memcpy_wt() is using a combination of
> >> non-temporal stores and explicit cache flushing.
> >>
> >> > How about using "nocache" as it's been
> >> > used in __copy_user_nocache()?
> >>
> >> The difference in my mind is that the "_nocache" suffix indicates
> >> opportunistic / optional cache pollution avoidance whereas "_wt"
> >> strictly arranges for caches not to contain dirty data upon
> >> completion of the routine. For example, non-temporal stores on older
> >> x86 cpus could potentially leave dirty data in the cache, so
> >> memcpy_wt on those cpus would need to use explicit cache flushing.
> >
> > I see.  I agree that its behavior is different from the existing one
> > with "_nocache".   That said, I think "wt" or "write-through" generally
> > means that writes allocate cachelines and keep them clean by writing to
> > memory.  So, subsequent reads to the destination will hit the
> > cachelines.  This is not the case with this interface.
> 
> True... maybe _nocache_strict()? Or, leave it _wt() until someone
> comes along and is surprised that the cache is not warm for reads
> after memcpy_wt(), at which point we can ask "why not just use plain
> memcpy then?", or set the page-attributes to WT.

Perhaps a _nocache_flush() postfix, to signal both that it's non-temporal and 
that 
no cache line is left around afterwards (dirty or clean)?

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v2] x86, uaccess: introduce copy_from_iter_wt for pmem / writethrough operations

2017-05-05 Thread Ingo Molnar

* Dan Williams <dan.j.willi...@intel.com> wrote:

> The pmem driver has a need to transfer data with a persistent memory
> destination and be able to rely on the fact that the destination writes
> are not cached. It is sufficient for the writes to be flushed to a
> cpu-store-buffer (non-temporal / "movnt" in x86 terms), as we expect
> userspace to call fsync() to ensure data-writes have reached a
> power-fail-safe zone in the platform. The fsync() triggers a REQ_FUA or
> REQ_FLUSH to the pmem driver which will turn around and fence previous
> writes with an "sfence".
> 
> Implement a __copy_from_user_inatomic_wt, memcpy_page_wt, and memcpy_wt,
> that guarantee that the destination buffer is not dirty in the cpu cache
> on completion. The new copy_from_iter_wt and sub-routines will be used
> to replace the "pmem api" (include/linux/pmem.h +
> arch/x86/include/asm/pmem.h). The availability of copy_from_iter_wt()
> and memcpy_wt() are gated by the CONFIG_ARCH_HAS_UACCESS_WT config
> symbol, and fallback to copy_from_iter_nocache() and plain memcpy()
> otherwise.
> 
> This is meant to satisfy the concern from Linus that if a driver wants
> to do something beyond the normal nocache semantics it should be
> something private to that driver [1], and Al's concern that anything
> uaccess related belongs with the rest of the uaccess code [2].
> 
> [1]: https://lists.01.org/pipermail/linux-nvdimm/2017-January/008364.html
> [2]: https://lists.01.org/pipermail/linux-nvdimm/2017-April/009942.html
> 
> Cc: <x...@kernel.org>
> Cc: Jan Kara <j...@suse.cz>
> Cc: Jeff Moyer <jmo...@redhat.com>
> Cc: Ingo Molnar <mi...@redhat.com>
> Cc: Christoph Hellwig <h...@lst.de>
> Cc: "H. Peter Anvin" <h...@zytor.com>
> Cc: Al Viro <v...@zeniv.linux.org.uk>
> Cc: Thomas Gleixner <t...@linutronix.de>
> Cc: Matthew Wilcox <mawil...@microsoft.com>
> Cc: Ross Zwisler <ross.zwis...@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> ---
> Changes since the initial RFC:
> * s/writethru/wt/ since we already have ioremap_wt(), set_memory_wt(),
>   etc. (Ingo)

Looks good to me. I suspect you'd like to carry this in the nvdimm tree?

Acked-by: Ingo Molnar <mi...@kernel.org>

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [RFC PATCH] x86, uaccess, pmem: introduce copy_from_iter_writethru for dax + pmem

2017-04-27 Thread Ingo Molnar

* Dan Williams  wrote:

> +#ifdef CONFIG_ARCH_HAS_UACCESS_WRITETHRU
> +#define __HAVE_ARCH_MEMCPY_WRITETHRU 1
> +void memcpy_writethru(void *dst, const void *src, size_t cnt);
> +#endif

This should be named memcpy_wt(), which is the well-known postfix for 
write-through.

We already have ioremap_wt(), set_memory_wt(), etc. - no need to introduce a 
longer variant with uncommon spelling.

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH] x86: fix kaslr and memmap collision

2016-11-22 Thread Ingo Molnar

* Dave Jiang  wrote:

> CONFIG_RANDOMIZE_BASE relocates the kernel to a random base address.
> However it does not take into account the memmap= parameter passed in from
> the kernel commandline.

memmap= parameters are often used as a list.

> [...] This results in the kernel sometimes being put in the middle of the 
> user 
> memmap. [...]

What does this mean? If memmap= is used to re-define the memory map then the 
kernel getting in the middle of a RAM area is what we want, isn't it? What we 
don't want is for the kernel to get into reserved areas, right?

> [...] Check has been added in the kaslr in order to avoid the region marked 
> by 
> memmap.

What does this mean?

> Signed-off-by: Dave Jiang 
> ---
>  arch/x86/boot/boot.h |2 ++
>  arch/x86/boot/compressed/kaslr.c |   45 
> ++
>  arch/x86/boot/string.c   |   25 +
>  3 files changed, 72 insertions(+)
> 
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index e5612f3..0d5fe5b 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -332,6 +332,8 @@ int strncmp(const char *cs, const char *ct, size_t count);
>  size_t strnlen(const char *s, size_t maxlen);
>  unsigned int atou(const char *s);
>  unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int 
> base);
> +unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base);
> +long simple_strtol(const char *cp, char **endp, unsigned int base);
>  size_t strlen(const char *s);
>  
>  /* tty.c */
> diff --git a/arch/x86/boot/compressed/kaslr.c 
> b/arch/x86/boot/compressed/kaslr.c
> index a66854d..6fb8f1ec 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -11,6 +11,7 @@
>   */
>  #include "misc.h"
>  #include "error.h"
> +#include "../boot.h"
>  
>  #include 
>  #include 
> @@ -61,6 +62,7 @@ enum mem_avoid_index {
>   MEM_AVOID_INITRD,
>   MEM_AVOID_CMDLINE,
>   MEM_AVOID_BOOTPARAMS,
> + MEM_AVOID_MEMMAP,
>   MEM_AVOID_MAX,
>  };
>  
> @@ -77,6 +79,37 @@ static bool mem_overlaps(struct mem_vector *one, struct 
> mem_vector *two)
>   return true;
>  }
>  
> +#include "../../../../lib/cmdline.c"
> +
> +static int
> +parse_memmap(char *p, unsigned long long *start, unsigned long long *size)
> +{
> + char *oldp;
> +
> + if (!p)
> + return -EINVAL;
> +
> + /* we don't care about this option here */
> + if (!strncmp(p, "exactmap", 8))
> + return -EINVAL;
> +
> + oldp = p;
> + *size = memparse(p, );
> + if (p == oldp)
> + return -EINVAL;
> +
> + switch (*p) {
> + case '@':
> + case '#':
> + case '$':
> + case '!':
> + *start = memparse(p+1, );
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
>  /*
>   * In theory, KASLR can put the kernel anywhere in the range of [16M, 64T).
>   * The mem_avoid array is used to store the ranges that need to be avoided
> @@ -158,6 +191,8 @@ static void mem_avoid_init(unsigned long input, unsigned 
> long input_size,
>   u64 initrd_start, initrd_size;
>   u64 cmd_line, cmd_line_size;
>   char *ptr;
> + char arg[38];

Where does the magic '38' come from?

> + unsigned long long memmap_start, memmap_size;
>  
>   /*
>* Avoid the region that is unsafe to overlap during
> @@ -195,6 +230,16 @@ static void mem_avoid_init(unsigned long input, unsigned 
> long input_size,
>   add_identity_map(mem_avoid[MEM_AVOID_BOOTPARAMS].start,
>mem_avoid[MEM_AVOID_BOOTPARAMS].size);
>  
> + /* see if we have any memmap areas */
> + if (cmdline_find_option("memmap", arg, sizeof(arg)) > 0) {
> + int rc = parse_memmap(arg, _start, _size);
> +
> + if (!rc) {
> + mem_avoid[MEM_AVOID_MEMMAP].start = memmap_start;
> + mem_avoid[MEM_AVOID_MEMMAP].size = memmap_size;
> + }
> + }
> +

This only handles a single (first) memmap argument, is that sufficient?

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [RFC PATCH 2/2] x86: wire up mincore2()

2016-09-13 Thread Ingo Molnar

* Dan Williams  wrote:

> Add the new the mincore2() symbol to the x86 syscall tables.

Could you please send the patch against -tip? We have this (new) commit in the 
x86 
tree:

  f9afc6197e9b x86: Wire up protection keys system calls

... which created a new conflict.

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v2 16/17] x86/insn: remove pcommit

2016-07-23 Thread Ingo Molnar

* Dan Williams <dan.j.willi...@intel.com> wrote:

> On Fri, Jul 22, 2016 at 9:52 AM, Ingo Molnar <mi...@kernel.org> wrote:
> >
> > * Dan Williams <dan.j.willi...@intel.com> wrote:
> >
> >> On Tue, Jul 12, 2016 at 3:12 PM, Dan Williams <dan.j.willi...@intel.com> 
> >> wrote:
> >> > On Tue, Jul 12, 2016 at 7:57 AM, Peter Zijlstra <pet...@infradead.org> 
> >> > wrote:
> >> >> On Sat, Jul 09, 2016 at 08:25:54PM -0700, Dan Williams wrote:
> >> >>> The pcommit instruction is being deprecated in favor of either ADR
> >> >>> (asynchronous DRAM refresh: flush-on-power-fail) at the platform 
> >> >>> level, or
> >> >>> posted-write-queue flush addresses as defined by the ACPI 6.x NFIT 
> >> >>> (NVDIMM
> >> >>> Firmware Interface Table).
> >> >>
> >> >>>  arch/x86/include/asm/cpufeatures.h |1
> >> >>>  arch/x86/include/asm/special_insns.h   |   46 
> >> >>> 
> >> >>>  arch/x86/lib/x86-opcode-map.txt|2 -
> >> >>>  tools/objtool/arch/x86/insn/x86-opcode-map.txt |2 -
> >> >>>  tools/perf/arch/x86/tests/insn-x86-dat-32.c|2 -
> >> >>>  tools/perf/arch/x86/tests/insn-x86-dat-64.c|2 -
> >> >>>  tools/perf/arch/x86/tests/insn-x86-dat-src.c   |4 --
> >> >>
> >> >> Just deprecated, or is it completely eradicated, removed from history,
> >> >> will never ever happen and we'll reissue the opcode for something else?
> >> >>
> >> >> Because if its only deprecated then removing it from the instruction
> >> >> decoders seems wrong, old binaries might still contain the opcode.
> >> >
> >> > Eradicated.
> >> >
> >> > "The new instructions like CLWB and CLFLUSHOPT will be rolled into the
> >> > SDM but PCOMMIT will be removed from the Extensions doc and not rolled
> >> > into the SDM." [1]
> >> >
> >> > Existing binaries are already gating their usage on the presence of
> >> > the cpu id flag, that flag and the instruction opcode are reserved
> >> > going forward.
> >> >
> >> > [1]: https://lists.01.org/pipermail/linux-nvdimm/2016-June/005923.html
> >>
> >> x86 maintainers, I have the other patches in this series queued in -next. 
> >> Please
> >> ack this one and I'll add it for v4.8-rc1, or otherwise let me know how 
> >> you want
> >> to handle this patch.
> >
> > Since it's just a removal AFAICS that the rest of your series should not 
> > depend
> > on, can you submit it to the x86 tree?
> 
> This patch depends on the previous patches in the series removing
> calls to pcommit_sfence().

Ok, and the patch looks harmless:

Acked-by: Ingo Molnar <mi...@kernel.org>

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v2 16/17] x86/insn: remove pcommit

2016-07-22 Thread Ingo Molnar

* Dan Williams  wrote:

> On Tue, Jul 12, 2016 at 3:12 PM, Dan Williams  
> wrote:
> > On Tue, Jul 12, 2016 at 7:57 AM, Peter Zijlstra  
> > wrote:
> >> On Sat, Jul 09, 2016 at 08:25:54PM -0700, Dan Williams wrote:
> >>> The pcommit instruction is being deprecated in favor of either ADR
> >>> (asynchronous DRAM refresh: flush-on-power-fail) at the platform level, or
> >>> posted-write-queue flush addresses as defined by the ACPI 6.x NFIT (NVDIMM
> >>> Firmware Interface Table).
> >>
> >>>  arch/x86/include/asm/cpufeatures.h |1
> >>>  arch/x86/include/asm/special_insns.h   |   46 
> >>> 
> >>>  arch/x86/lib/x86-opcode-map.txt|2 -
> >>>  tools/objtool/arch/x86/insn/x86-opcode-map.txt |2 -
> >>>  tools/perf/arch/x86/tests/insn-x86-dat-32.c|2 -
> >>>  tools/perf/arch/x86/tests/insn-x86-dat-64.c|2 -
> >>>  tools/perf/arch/x86/tests/insn-x86-dat-src.c   |4 --
> >>
> >> Just deprecated, or is it completely eradicated, removed from history,
> >> will never ever happen and we'll reissue the opcode for something else?
> >>
> >> Because if its only deprecated then removing it from the instruction
> >> decoders seems wrong, old binaries might still contain the opcode.
> >
> > Eradicated.
> >
> > "The new instructions like CLWB and CLFLUSHOPT will be rolled into the
> > SDM but PCOMMIT will be removed from the Extensions doc and not rolled
> > into the SDM." [1]
> >
> > Existing binaries are already gating their usage on the presence of
> > the cpu id flag, that flag and the instruction opcode are reserved
> > going forward.
> >
> > [1]: https://lists.01.org/pipermail/linux-nvdimm/2016-June/005923.html
> 
> x86 maintainers, I have the other patches in this series queued in -next. 
> Please 
> ack this one and I'll add it for v4.8-rc1, or otherwise let me know how you 
> want 
> to handle this patch.

Since it's just a removal AFAICS that the rest of your series should not depend 
on, can you submit it to the x86 tree?

Thanks,

Ingo
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm