Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1500383 > unrolled thread

[PATCH] pmem: report error on clear poison failure

Started byToshi Kani <toshi.kani@hpe.com>
First post2016-10-13 18:00 +0200
Last post2016-10-13 20:50 +0200
Articles 7 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] pmem: report error on clear poison failure Toshi Kani <toshi.kani@hpe.com> - 2016-10-13 18:00 +0200
    Re: [PATCH] pmem: report error on clear poison failure Dan Williams <dan.j.williams@intel.com> - 2016-10-13 18:10 +0200
      Re: [PATCH] pmem: report error on clear poison failure Dan Williams <dan.j.williams@intel.com> - 2016-10-13 19:30 +0200
        Re: [PATCH] pmem: report error on clear poison failure "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2016-10-13 21:00 +0200
          Re: [PATCH] pmem: report error on clear poison failure Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-10-13 21:20 +0200
          Re: [PATCH] pmem: report error on clear poison failure Dan Williams <dan.j.williams@intel.com> - 2016-10-13 21:30 +0200
      Re: [PATCH] pmem: report error on clear poison failure "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2016-10-13 20:50 +0200

#1500383 — [PATCH] pmem: report error on clear poison failure

FromToshi Kani <toshi.kani@hpe.com>
Date2016-10-13 18:00 +0200
Subject[PATCH] pmem: report error on clear poison failure
Message-ID<srQAh-5qL-27@gated-at.bofh.it>
ACPI Clear Uncorrectable Error DSM function may fail or may be
unsupported on a platform.  pmem_clear_poison() returns without
clearing badblocks in such cases, which leads to a silent data
corruption.

Change pmem_do_bvec() and pmem_clear_poison() to return -EIO
so that filesystem can log an error message.

Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
---
 drivers/nvdimm/pmem.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 42b3a82..2461843 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -47,7 +47,7 @@ static struct nd_region *to_region(struct pmem_device *pmem)
 	return to_nd_region(to_dev(pmem)->parent);
 }
 
-static void pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset,
+static int pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset,
 		unsigned int len)
 {
 	struct device *dev = to_dev(pmem);
@@ -62,8 +62,12 @@ static void pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset,
 				__func__, (unsigned long long) sector,
 				cleared / 512, cleared / 512 > 1 ? "s" : "");
 		badblocks_clear(&pmem->bb, sector, cleared / 512);
+	} else {
+		return -EIO;
 	}
+
 	invalidate_pmem(pmem->virt_addr + offset, len);
+	return 0;
 }
 
 static void write_pmem(void *pmem_addr, struct page *page,
@@ -123,7 +127,7 @@ static int pmem_do_bvec(struct pmem_device *pmem, struct page *page,
 		flush_dcache_page(page);
 		write_pmem(pmem_addr, page, off, len);
 		if (unlikely(bad_pmem)) {
-			pmem_clear_poison(pmem, pmem_off, len);
+			rc = pmem_clear_poison(pmem, pmem_off, len);
 			write_pmem(pmem_addr, page, off, len);
 		}
 	}

[toc] | [next] | [standalone]

#1500405

FromDan Williams <dan.j.williams@intel.com>
Date2016-10-13 18:10 +0200
Message-ID<srQJX-5Jj-17@gated-at.bofh.it>
In reply to#1500383
On Thu, Oct 13, 2016 at 8:54 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> ACPI Clear Uncorrectable Error DSM function may fail or may be
> unsupported on a platform.  pmem_clear_poison() returns without
> clearing badblocks in such cases, which leads to a silent data
> corruption.
>
> Change pmem_do_bvec() and pmem_clear_poison() to return -EIO
> so that filesystem can log an error message.

What's the silent data corruption scenario?  If the clear poison fails
I'm assuming that the poison will still be notified on the next read.

[toc] | [prev] | [next] | [standalone]

#1500473

FromDan Williams <dan.j.williams@intel.com>
Date2016-10-13 19:30 +0200
Message-ID<srRZo-6yi-17@gated-at.bofh.it>
In reply to#1500405
On Thu, Oct 13, 2016 at 9:08 AM, Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
> On Thu, 2016-10-13 at 09:01 -0700, Dan Williams wrote:
>> On Thu, Oct 13, 2016 at 8:54 AM, Toshi Kani <toshi.kani@hpe.com>
>> wrote:
>> >
>> > ACPI Clear Uncorrectable Error DSM function may fail or may be
>> > unsupported on a platform.  pmem_clear_poison() returns without
>> > clearing badblocks in such cases, which leads to a silent data
>> > corruption.
>> >
>> > Change pmem_do_bvec() and pmem_clear_poison() to return -EIO
>> > so that filesystem can log an error message.
>>
>> What's the silent data corruption scenario?  If the clear poison
>> fails I'm assuming that the poison will still be notified on the next
>> read.
>
> I agree that the data is eventually read, but there is no guranteed
> that when it is read soon enough, i.e. user might not access to the
> data for a long time.

...but that's the same behavior for errors that we don't yet know
about.  That said, we indeed know that the write failed.  I'd feel
better about this patch if the justification / impact was clearer in
the changelog, because "silent data corruption" is not the impact.

[toc] | [prev] | [next] | [standalone]

#1500499

From"Kani, Toshimitsu" <toshi.kani@hpe.com>
Date2016-10-13 21:00 +0200
Message-ID<srTot-7lC-1@gated-at.bofh.it>
In reply to#1500473
On Thu, 2016-10-13 at 10:22 -0700, Dan Williams wrote:
> On Thu, Oct 13, 2016 at 9:08 AM, Kani, Toshimitsu <toshi.kani@hpe.com
> > wrote:
> > 
> > On Thu, 2016-10-13 at 09:01 -0700, Dan Williams wrote:
> > > 
> > > On Thu, Oct 13, 2016 at 8:54 AM, Toshi Kani <toshi.kani@hpe.com>
> > > wrote:
> > > > 
> > > > 
> > > > ACPI Clear Uncorrectable Error DSM function may fail or may be
> > > > unsupported on a platform.  pmem_clear_poison() returns without
> > > > clearing badblocks in such cases, which leads to a silent data
> > > > corruption.
> > > > 
> > > > Change pmem_do_bvec() and pmem_clear_poison() to return -EIO
> > > > so that filesystem can log an error message.
> > > 
> > > What's the silent data corruption scenario?  If the clear poison
> > > fails I'm assuming that the poison will still be notified on the
> > > next
> > > read.
> > 
> > I agree that the data is eventually read, but there is no guranteed
> > that when it is read soon enough, i.e. user might not access to the
> > data for a long time.
> 
> ...but that's the same behavior for errors that we don't yet know
> about.  That said, we indeed know that the write failed.  I'd feel
> better about this patch if the justification / impact was clearer in
> the changelog, because "silent data corruption" is not the impact.

Agreed.  How about the following descritpion?

===
ACPI Clear Uncorrectable Error DSM function may fail or may be
unsupported on a platform.  pmem_clear_poison() returns without
clearing badblocks in such cases.  This failure is detected at
the next read (-EIO).

This behavior can lead to an issue when user keeps writing but
does not read immedicately.  For instance, flight recorder file
may be only read when it is necessary for troubleshooting.

Change pmem_do_bvec() and pmem_clear_poison() to return -EIO
so that filesystem can log an error message on a write error.
===

Thanks,
-Toshi

[toc] | [prev] | [next] | [standalone]

#1500507

FromRoss Zwisler <ross.zwisler@linux.intel.com>
Date2016-10-13 21:20 +0200
Message-ID<srTHP-7Il-3@gated-at.bofh.it>
In reply to#1500499
On Thu, Oct 13, 2016 at 06:16:29PM +0000, Kani, Toshimitsu wrote:
> On Thu, 2016-10-13 at 10:22 -0700, Dan Williams wrote:
> > On Thu, Oct 13, 2016 at 9:08 AM, Kani, Toshimitsu <toshi.kani@hpe.com
> > > wrote:
> > > 
> > > On Thu, 2016-10-13 at 09:01 -0700, Dan Williams wrote:
> > > > 
> > > > On Thu, Oct 13, 2016 at 8:54 AM, Toshi Kani <toshi.kani@hpe.com>
> > > > wrote:
> > > > > 
> > > > > 
> > > > > ACPI Clear Uncorrectable Error DSM function may fail or may be
> > > > > unsupported on a platform.  pmem_clear_poison() returns without
> > > > > clearing badblocks in such cases, which leads to a silent data
> > > > > corruption.
> > > > > 
> > > > > Change pmem_do_bvec() and pmem_clear_poison() to return -EIO
> > > > > so that filesystem can log an error message.
> > > > 
> > > > What's the silent data corruption scenario?  If the clear poison
> > > > fails I'm assuming that the poison will still be notified on the
> > > > next
> > > > read.
> > > 
> > > I agree that the data is eventually read, but there is no guranteed
> > > that when it is read soon enough, i.e. user might not access to the
> > > data for a long time.
> > 
> > ...but that's the same behavior for errors that we don't yet know
> > about.  That said, we indeed know that the write failed.  I'd feel
> > better about this patch if the justification / impact was clearer in
> > the changelog, because "silent data corruption" is not the impact.
> 
> Agreed.  How about the following descritpion?
> 
> ===
> ACPI Clear Uncorrectable Error DSM function may fail or may be
> unsupported on a platform.  pmem_clear_poison() returns without
> clearing badblocks in such cases.  This failure is detected at
> the next read (-EIO).
> 
> This behavior can lead to an issue when user keeps writing but
> does not read immedicately.  For instance, flight recorder file
		immediately

> may be only read when it is necessary for troubleshooting.
> 
> Change pmem_do_bvec() and pmem_clear_poison() to return -EIO
> so that filesystem can log an error message on a write error.
> ===
> 
> Thanks,
> -Toshi
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm

[toc] | [prev] | [next] | [standalone]

#1500511

FromDan Williams <dan.j.williams@intel.com>
Date2016-10-13 21:30 +0200
Message-ID<srTRw-7Mi-21@gated-at.bofh.it>
In reply to#1500499
On Thu, Oct 13, 2016 at 11:16 AM, Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
> On Thu, 2016-10-13 at 10:22 -0700, Dan Williams wrote:
>> On Thu, Oct 13, 2016 at 9:08 AM, Kani, Toshimitsu <toshi.kani@hpe.com
>> > wrote:
>> >
>> > On Thu, 2016-10-13 at 09:01 -0700, Dan Williams wrote:
>> > >
>> > > On Thu, Oct 13, 2016 at 8:54 AM, Toshi Kani <toshi.kani@hpe.com>
>> > > wrote:
>> > > >
>> > > >
>> > > > ACPI Clear Uncorrectable Error DSM function may fail or may be
>> > > > unsupported on a platform.  pmem_clear_poison() returns without
>> > > > clearing badblocks in such cases, which leads to a silent data
>> > > > corruption.
>> > > >
>> > > > Change pmem_do_bvec() and pmem_clear_poison() to return -EIO
>> > > > so that filesystem can log an error message.
>> > >
>> > > What's the silent data corruption scenario?  If the clear poison
>> > > fails I'm assuming that the poison will still be notified on the
>> > > next
>> > > read.
>> >
>> > I agree that the data is eventually read, but there is no guranteed
>> > that when it is read soon enough, i.e. user might not access to the
>> > data for a long time.
>>
>> ...but that's the same behavior for errors that we don't yet know
>> about.  That said, we indeed know that the write failed.  I'd feel
>> better about this patch if the justification / impact was clearer in
>> the changelog, because "silent data corruption" is not the impact.
>
> Agreed.  How about the following descritpion?
>
> ===
> ACPI Clear Uncorrectable Error DSM function may fail or may be
> unsupported on a platform.  pmem_clear_poison() returns without
> clearing badblocks in such cases.  This failure is detected at
> the next read (-EIO).
>
> This behavior can lead to an issue when user keeps writing but
> does not read immedicately.  For instance, flight recorder file
> may be only read when it is necessary for troubleshooting.
>
> Change pmem_do_bvec() and pmem_clear_poison() to return -EIO
> so that filesystem can log an error message on a write error.
> ===

Looks good, thanks Toshi.  I'll update the nvdimm.git branches after
-rc1 is out.

[toc] | [prev] | [next] | [standalone]

#1500498

From"Kani, Toshimitsu" <toshi.kani@hpe.com>
Date2016-10-13 20:50 +0200
Message-ID<srRZo-6yi-19@gated-at.bofh.it>
In reply to#1500405
On Thu, 2016-10-13 at 09:01 -0700, Dan Williams wrote:
> On Thu, Oct 13, 2016 at 8:54 AM, Toshi Kani <toshi.kani@hpe.com>
> wrote:
> > 
> > ACPI Clear Uncorrectable Error DSM function may fail or may be
> > unsupported on a platform.  pmem_clear_poison() returns without
> > clearing badblocks in such cases, which leads to a silent data
> > corruption.
> > 
> > Change pmem_do_bvec() and pmem_clear_poison() to return -EIO
> > so that filesystem can log an error message.
> 
> What's the silent data corruption scenario?  If the clear poison
> fails I'm assuming that the poison will still be notified on the next
> read.

I agree that the data is eventually read, but there is no guranteed
that when it is read soon enough, i.e. user might not access to the
data for a long time.

Thanks,
-Toshi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web