From: Toshi Kani <toshi.kani@hpe.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
Borislav Petkov <bp@suse.de>,
Ross Zwisler <ross.zwisler@linux.intel.com>,
Vishal L Verma <vishal.l.verma@intel.com>,
micah.parrish@hpe.com, brian.boylston@hpe.com,
X86 ML <x86@kernel.org>,
"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/2] Fix BTT data corruptions after crash
Date: Thu, 28 Jan 2016 13:43:45 -0700 [thread overview]
Message-ID: <1454013825.2576.31.camel@hpe.com> (raw)
In-Reply-To: <CAPcyv4gc_wL6Bb8NxU42CAPN6v05MYnVVZiyijAQt9XZ6VNL_g@mail.gmail.com>
On Thu, 2016-01-28 at 12:12 -0800, Dan Williams wrote:
> On Thu, Jan 28, 2016 at 10:12 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> > Data corruption issues were observed in tests which initiated a system
> > crash/reset while accessing BTT devices. This problem is reproducible.
> >
> > The BTT driver calls pmem_rw_bytes() to update data in pmem devices.
> > This interface calls __copy_user_nocache(), which uses non-temporal
> > stores so that the stores to pmem are persistent.
> >
> > __copy_user_nocache() uses non-temporal stores when a request size is
> > 8 bytes or larger (and is aligned by 8 bytes). The BTT driver updates
> > the BTT map table, which entry size is 4 bytes. Therefore, updates to
> > the map table entries remain cached, and are not written to pmem after
> > a crash. Since the BTT driver makes previous blocks free and uses them
> > for subsequent writes, the map table ends up pointing to blocks
> > allocated for other LBAs after a crash.
> >
> > Patch 1 extends __copy_user_nocache() to use non-temporal store for
> > 4 byte copy. This patch fixes the BTT data corruption issue.
> >
>
> Nice find!
:-)
> > Patch 2 changes arch_memcpy_to_pmem() to flush processor caches when
> > a request is not naturally aligned or is less than 4 bytes. This is
> > defensive change.
>
> I'm wondering if we should just document that this routine does not
> support unaligned transfers? Maybe backed by a debug mode that does
> the alignment check.
Yes, I agree. For this debug mode, do you have something in mind? Or
should we add a new CONFIG option like CONFIG_PMEM_DEBUG?
Thanks,
-Toshi
WARNING: multiple messages have this Message-ID (diff)
From: Toshi Kani <toshi.kani@hpe.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
Borislav Petkov <bp@suse.de>,
Ross Zwisler <ross.zwisler@linux.intel.com>,
Vishal L Verma <vishal.l.verma@intel.com>,
micah.parrish@hpe.com, brian.boylston@hpe.com,
X86 ML <x86@kernel.org>,
"linux-nvdimm@lists.01.org" <linux-nvdimm@ml01.01.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/2] Fix BTT data corruptions after crash
Date: Thu, 28 Jan 2016 13:43:45 -0700 [thread overview]
Message-ID: <1454013825.2576.31.camel@hpe.com> (raw)
In-Reply-To: <CAPcyv4gc_wL6Bb8NxU42CAPN6v05MYnVVZiyijAQt9XZ6VNL_g@mail.gmail.com>
On Thu, 2016-01-28 at 12:12 -0800, Dan Williams wrote:
> On Thu, Jan 28, 2016 at 10:12 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> > Data corruption issues were observed in tests which initiated a system
> > crash/reset while accessing BTT devices. This problem is reproducible.
> >
> > The BTT driver calls pmem_rw_bytes() to update data in pmem devices.
> > This interface calls __copy_user_nocache(), which uses non-temporal
> > stores so that the stores to pmem are persistent.
> >
> > __copy_user_nocache() uses non-temporal stores when a request size is
> > 8 bytes or larger (and is aligned by 8 bytes). The BTT driver updates
> > the BTT map table, which entry size is 4 bytes. Therefore, updates to
> > the map table entries remain cached, and are not written to pmem after
> > a crash. Since the BTT driver makes previous blocks free and uses them
> > for subsequent writes, the map table ends up pointing to blocks
> > allocated for other LBAs after a crash.
> >
> > Patch 1 extends __copy_user_nocache() to use non-temporal store for
> > 4 byte copy. This patch fixes the BTT data corruption issue.
> >
>
> Nice find!
:-)
> > Patch 2 changes arch_memcpy_to_pmem() to flush processor caches when
> > a request is not naturally aligned or is less than 4 bytes. This is
> > defensive change.
>
> I'm wondering if we should just document that this routine does not
> support unaligned transfers? Maybe backed by a debug mode that does
> the alignment check.
Yes, I agree. For this debug mode, do you have something in mind? Or
should we add a new CONFIG option like CONFIG_PMEM_DEBUG?
Thanks,
-Toshi
next prev parent reply other threads:[~2016-01-28 20:43 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-28 18:12 [PATCH 0/2] Fix BTT data corruptions after crash Toshi Kani
2016-01-28 18:12 ` Toshi Kani
2016-01-28 18:12 ` [PATCH 1/2] x86/lib/copy_user_64.S: Handle 4-byte uncached copy Toshi Kani
2016-01-28 18:12 ` Toshi Kani
2016-01-29 8:27 ` Ingo Molnar
2016-01-29 8:27 ` Ingo Molnar
2016-01-29 14:56 ` Toshi Kani
2016-01-29 14:56 ` Toshi Kani
2016-01-28 18:12 ` [PATCH 2/2] pmem: Flush cache on unaligned request Toshi Kani
2016-01-28 18:12 ` Toshi Kani
2016-01-28 20:12 ` [PATCH 0/2] Fix BTT data corruptions after crash Dan Williams
2016-01-28 20:12 ` Dan Williams
2016-01-28 20:43 ` Toshi Kani [this message]
2016-01-28 20:43 ` Toshi Kani
2016-01-30 17:44 ` Dan Williams
2016-01-30 17:44 ` Dan Williams
2016-02-01 19:50 ` Toshi Kani
2016-02-01 19:50 ` Toshi Kani
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=1454013825.2576.31.camel@hpe.com \
--to=toshi.kani@hpe.com \
--cc=bp@suse.de \
--cc=brian.boylston@hpe.com \
--cc=dan.j.williams@intel.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvdimm@lists.01.org \
--cc=micah.parrish@hpe.com \
--cc=mingo@redhat.com \
--cc=ross.zwisler@linux.intel.com \
--cc=tglx@linutronix.de \
--cc=vishal.l.verma@intel.com \
--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.