nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* set_memory_uc() does not work with pmem poison handling
@ 2021-06-15 18:55 Jane Chu
  2021-06-22  0:15 ` Luis Chamberlain
  0 siblings, 1 reply; 6+ messages in thread
From: Jane Chu @ 2021-06-15 18:55 UTC (permalink / raw)
  To: Dan Williams; +Cc: nvdimm

Hi, Dan,

It appears the fact pmem region is of WB memtype renders set_memory_uc()

to fail due to inconsistency between the requested memtype(UC-) and the 
cached

memtype(WB).

# cat /proc/iomem |grep -A 8 -i persist
1840000000-d53fffffff : Persistent Memory
   1840000000-1c3fffffff : namespace0.0
   5740000000-76bfffffff : namespace2.0

# cat /sys/kernel/debug/x86/pat_memtype_list
PAT memtype list:
PAT: [mem 0x0000001840000000-0x0000001c40000000] write-back
PAT: [mem 0x0000005740000000-0x00000076c0000000] write-back

[10683.418072] Memory failure: 0x1850600: recovery action for dax page: 
Recovered
[10683.426147] x86/PAT: fsdax_poison_v1:5018 conflicting memory types 
1850600000-1850601000  uncached-minus<->write-back

cscope search shows that unlike pmem, set_memory_uc() is primarily used 
by drivers dealing with ioremap(),

perhaps the pmem case needs another way to suppress prefetch?

Your thoughts?

thanks!

-jane



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: set_memory_uc() does not work with pmem poison handling
  2021-06-15 18:55 set_memory_uc() does not work with pmem poison handling Jane Chu
@ 2021-06-22  0:15 ` Luis Chamberlain
  2021-06-22 17:21   ` Jane Chu
  0 siblings, 1 reply; 6+ messages in thread
From: Luis Chamberlain @ 2021-06-22  0:15 UTC (permalink / raw)
  To: Jane Chu; +Cc: Dan Williams, nvdimm, mcgrof

On Tue, Jun 15, 2021 at 11:55:19AM -0700, Jane Chu wrote:
> Hi, Dan,
> 
> It appears the fact pmem region is of WB memtype renders set_memory_uc()
> 
> to fail due to inconsistency between the requested memtype(UC-) and the
> cached
> 
> memtype(WB).
> 
> # cat /proc/iomem |grep -A 8 -i persist
> 1840000000-d53fffffff : Persistent Memory
>   1840000000-1c3fffffff : namespace0.0
>   5740000000-76bfffffff : namespace2.0
> 
> # cat /sys/kernel/debug/x86/pat_memtype_list
> PAT memtype list:
> PAT: [mem 0x0000001840000000-0x0000001c40000000] write-back
> PAT: [mem 0x0000005740000000-0x00000076c0000000] write-back
> 
> [10683.418072] Memory failure: 0x1850600: recovery action for dax page:
> Recovered
> [10683.426147] x86/PAT: fsdax_poison_v1:5018 conflicting memory types
> 1850600000-1850601000  uncached-minus<->write-back
> 
> cscope search shows that unlike pmem, set_memory_uc() is primarily used by
> drivers dealing with ioremap(),

Yes, when a driver *knows* the type must follow certain rules, it
requests it.

> perhaps the pmem case needs another way to suppress prefetch?
> 
> Your thoughts?

The way to think about this problem is, who did the ioremap call for the
ioremap'd area? That's the driver that needs changing.

  Luis


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: set_memory_uc() does not work with pmem poison handling
  2021-06-22  0:15 ` Luis Chamberlain
@ 2021-06-22 17:21   ` Jane Chu
  2021-06-25 23:15     ` Dan Williams
  0 siblings, 1 reply; 6+ messages in thread
From: Jane Chu @ 2021-06-22 17:21 UTC (permalink / raw)
  To: Luis Chamberlain; +Cc: Dan Williams, nvdimm, mcgrof


On 6/21/2021 5:15 PM, Luis Chamberlain wrote:
> On Tue, Jun 15, 2021 at 11:55:19AM -0700, Jane Chu wrote:
>> Hi, Dan,
>>
>> It appears the fact pmem region is of WB memtype renders set_memory_uc()
>>
>> to fail due to inconsistency between the requested memtype(UC-) and the
>> cached
>>
>> memtype(WB).
>>
>> # cat /proc/iomem |grep -A 8 -i persist
>> 1840000000-d53fffffff : Persistent Memory
>>    1840000000-1c3fffffff : namespace0.0
>>    5740000000-76bfffffff : namespace2.0
>>
>> # cat /sys/kernel/debug/x86/pat_memtype_list
>> PAT memtype list:
>> PAT: [mem 0x0000001840000000-0x0000001c40000000] write-back
>> PAT: [mem 0x0000005740000000-0x00000076c0000000] write-back
>>
>> [10683.418072] Memory failure: 0x1850600: recovery action for dax page:
>> Recovered
>> [10683.426147] x86/PAT: fsdax_poison_v1:5018 conflicting memory types
>> 1850600000-1850601000  uncached-minus<->write-back
>>
>> cscope search shows that unlike pmem, set_memory_uc() is primarily used by
>> drivers dealing with ioremap(),
> 
> Yes, when a driver *knows* the type must follow certain rules, it
> requests it.
> 
>> perhaps the pmem case needs another way to suppress prefetch?
>>
>> Your thoughts?
> 
> The way to think about this problem is, who did the ioremap call for the
> ioremap'd area? That's the driver that needs changing.

I'm not sure if the pmem driver is doing something wrong, but rather the
pmem memory failure handler.

thanks,
-jane

> 
>    Luis
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: set_memory_uc() does not work with pmem poison handling
  2021-06-22 17:21   ` Jane Chu
@ 2021-06-25 23:15     ` Dan Williams
  2021-06-25 23:21       ` Luck, Tony
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Williams @ 2021-06-25 23:15 UTC (permalink / raw)
  To: Jane Chu; +Cc: Luis Chamberlain, Linux NVDIMM, Luis R. Rodriguez, Luck, Tony

[ add Tony ]

Hi Jane,

I was out for a few days...

On Tue, Jun 22, 2021 at 10:22 AM Jane Chu <jane.chu@oracle.com> wrote:
>
>
> On 6/21/2021 5:15 PM, Luis Chamberlain wrote:
> > On Tue, Jun 15, 2021 at 11:55:19AM -0700, Jane Chu wrote:
> >> Hi, Dan,
> >>
> >> It appears the fact pmem region is of WB memtype renders set_memory_uc()
> >>
> >> to fail due to inconsistency between the requested memtype(UC-) and the
> >> cached
> >>
> >> memtype(WB).
> >>
> >> # cat /proc/iomem |grep -A 8 -i persist
> >> 1840000000-d53fffffff : Persistent Memory
> >>    1840000000-1c3fffffff : namespace0.0
> >>    5740000000-76bfffffff : namespace2.0
> >>
> >> # cat /sys/kernel/debug/x86/pat_memtype_list
> >> PAT memtype list:
> >> PAT: [mem 0x0000001840000000-0x0000001c40000000] write-back
> >> PAT: [mem 0x0000005740000000-0x00000076c0000000] write-back
> >>
> >> [10683.418072] Memory failure: 0x1850600: recovery action for dax page:
> >> Recovered
> >> [10683.426147] x86/PAT: fsdax_poison_v1:5018 conflicting memory types
> >> 1850600000-1850601000  uncached-minus<->write-back
> >>
> >> cscope search shows that unlike pmem, set_memory_uc() is primarily used by
> >> drivers dealing with ioremap(),
> >
> > Yes, when a driver *knows* the type must follow certain rules, it
> > requests it.
> >
> >> perhaps the pmem case needs another way to suppress prefetch?
> >>
> >> Your thoughts?
> >
> > The way to think about this problem is, who did the ioremap call for the
> > ioremap'd area? That's the driver that needs changing.
>
> I'm not sure if the pmem driver is doing something wrong, but rather the
> pmem memory failure handler.

This is a fixup that got lost in the shuffle. The issue is that the
memtype tracking needs to be updated before the set_memory_uc(), or
the memtype tracking needs to be bypassed.

You'll notice that this commit:

284ce4011ba6 x86/memory_failure: Introduce {set, clear}_mce_nospec()

...effectively replaced set_memory_np() with set_memory_uc(). However,
set_memory_np() does not check memtype and set_memory_uc() does.

I believe it should be sufficient to do the following, i.e. skip the
memtype sanity checking because the memory-failure code should have
already shot down all the conflicting WB mappings:

diff --git a/arch/x86/include/asm/set_memory.h
b/arch/x86/include/asm/set_memory.h
index 43fa081a1adb..0bf2274c5186 100644
--- a/arch/x86/include/asm/set_memory.h
+++ b/arch/x86/include/asm/set_memory.h
@@ -114,8 +114,13 @@ static inline int set_mce_nospec(unsigned long
pfn, bool unmap)

        if (unmap)
                rc = set_memory_np(decoy_addr, 1);
-       else
-               rc = set_memory_uc(decoy_addr, 1);
+       else {
+               /*
+                * Bypass memtype checks since memory-failure has shot
+                * down mappings.
+                */
+               rc = _set_memory_uc(decoy_addr, 1);
+       }
        if (rc)
                pr_warn("Could not invalidate pfn=0x%lx from 1:1 map\n", pfn);
        return rc;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: set_memory_uc() does not work with pmem poison handling
  2021-06-25 23:15     ` Dan Williams
@ 2021-06-25 23:21       ` Luck, Tony
  2021-06-25 23:49         ` Dan Williams
  0 siblings, 1 reply; 6+ messages in thread
From: Luck, Tony @ 2021-06-25 23:21 UTC (permalink / raw)
  To: Williams, Dan J, Jane Chu
  Cc: Luis Chamberlain, Linux NVDIMM, Luis R. Rodriguez

-       else
-               rc = set_memory_uc(decoy_addr, 1);
+       else {
+               /*
+                * Bypass memtype checks since memory-failure has shot
+                * down mappings.
+                */
+               rc = _set_memory_uc(decoy_addr, 1);
+       }

Does pmem "fix" poison addresses yet? If it does (or will) does it matter that
you skip the memtype_reserve() call?

-Tony

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: set_memory_uc() does not work with pmem poison handling
  2021-06-25 23:21       ` Luck, Tony
@ 2021-06-25 23:49         ` Dan Williams
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Williams @ 2021-06-25 23:49 UTC (permalink / raw)
  To: Luck, Tony; +Cc: Jane Chu, Luis Chamberlain, Linux NVDIMM, Luis R. Rodriguez

On Fri, Jun 25, 2021 at 4:22 PM Luck, Tony <tony.luck@intel.com> wrote:
>
> -       else
> -               rc = set_memory_uc(decoy_addr, 1);
> +       else {
> +               /*
> +                * Bypass memtype checks since memory-failure has shot
> +                * down mappings.
> +                */
> +               rc = _set_memory_uc(decoy_addr, 1);
> +       }
>
> Does pmem "fix" poison addresses yet? If it does (or will) does it matter that
> you skip the memtype_reserve() call?

It does fix them via clear_mce_nospec(), but that also looks like it
should be using the _set_memory_wb() version to bypass the memory type
reservation twiddling.

That said, I don't understand why set_memory_wb(), non "_" version,
thinks it can unconditionally delete any memtype reservation that
might exist for that pfn? The set_memory_* apis seem disjoint (wrt
memtype) for a reason that eludes me.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-06-25 23:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-15 18:55 set_memory_uc() does not work with pmem poison handling Jane Chu
2021-06-22  0:15 ` Luis Chamberlain
2021-06-22 17:21   ` Jane Chu
2021-06-25 23:15     ` Dan Williams
2021-06-25 23:21       ` Luck, Tony
2021-06-25 23:49         ` Dan Williams

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).