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

Groups > linux.kernel > #1661570 > unrolled thread

[PATCH v2 2/2] acpi/nfit: Issue Start ARS to retrieve existing records

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 2/2] acpi/nfit: Issue Start ARS to retrieve existing records Toshi Kani <toshi.kani@hpe.com> - 2017-06-08 20:40 +0200
    Re: [PATCH v2 2/2] acpi/nfit: Issue Start ARS to retrieve existing records Dan Williams <dan.j.williams@intel.com> - 2017-06-16 00:10 +0200
      RE: [PATCH v2 2/2] acpi/nfit: Issue Start ARS to retrieve existing  records "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-06-17 02:10 +0200
        Re: [PATCH v2 2/2] acpi/nfit: Issue Start ARS to retrieve existing records Dan Williams <dan.j.williams@intel.com> - 2017-06-17 02:20 +0200
          RE: [PATCH v2 2/2] acpi/nfit: Issue Start ARS to retrieve existing  records "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-06-17 02:50 +0200

#1661570 — [PATCH v2 2/2] acpi/nfit: Issue Start ARS to retrieve existing records

ACPI 6.2 defines in section 9.20.7.2 that the OSPM may call a Start
ARS with Flags Bit [1] set upon receiving the 0x81 notification.

  Upon receiving the notification, the OSPM may decide to issue
  a Start ARS with Flags Bit [1] set to prepare for the retrieval
  of existing records and issue the Query ARS Status function to
  retrieve the records.

Add support to call a Start ARS from acpi_nfit_uc_error_notify()
with ND_ARS_RETURN_PREV_DATA set when HW_ERROR_SCRUB_ON is not set.

Link: http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Linda Knippers <linda.knippers@hpe.com>
---
 drivers/acpi/nfit/core.c   |   14 +++++++++++---
 drivers/acpi/nfit/mce.c    |    2 +-
 drivers/acpi/nfit/nfit.h   |    3 ++-
 include/uapi/linux/ndctl.h |    1 +
 4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index cc22778..d820d72 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -1031,7 +1031,7 @@ static ssize_t scrub_store(struct device *dev,
 	if (nd_desc) {
 		struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
 
-		rc = acpi_nfit_ars_rescan(acpi_desc);
+		rc = acpi_nfit_ars_rescan(acpi_desc, 0);
 	}
 	device_unlock(dev);
 	if (rc)
@@ -2057,6 +2057,10 @@ static int ars_start(struct acpi_nfit_desc *acpi_desc, struct nfit_spa *nfit_spa
 		ars_start.type = ND_ARS_VOLATILE;
 	else
 		return -ENOTTY;
+	if (nfit_spa->ars_prev_data) {
+		ars_start.flags |= ND_ARS_RETURN_PREV_DATA;
+		nfit_spa->ars_prev_data = 0;
+	}
 
 	rc = nd_desc->ndctl(nd_desc, NULL, ND_CMD_ARS_START, &ars_start,
 			sizeof(ars_start), &cmd_rc);
@@ -2817,7 +2821,7 @@ static int acpi_nfit_clear_to_send(struct nvdimm_bus_descriptor *nd_desc,
 	return 0;
 }
 
-int acpi_nfit_ars_rescan(struct acpi_nfit_desc *acpi_desc)
+int acpi_nfit_ars_rescan(struct acpi_nfit_desc *acpi_desc, int flags)
 {
 	struct device *dev = acpi_desc->dev;
 	struct nfit_spa *nfit_spa;
@@ -2838,6 +2842,8 @@ int acpi_nfit_ars_rescan(struct acpi_nfit_desc *acpi_desc)
 			continue;
 
 		nfit_spa->ars_required = 1;
+		if (flags & ND_ARS_RETURN_PREV_DATA)
+			nfit_spa->ars_prev_data = 1;
 	}
 	queue_work(nfit_wq, &acpi_desc->work);
 	dev_dbg(dev, "%s: ars_scan triggered\n", __func__);
@@ -3015,8 +3021,10 @@ static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle)
 static void acpi_nfit_uc_error_notify(struct device *dev, acpi_handle handle)
 {
 	struct acpi_nfit_desc *acpi_desc = dev_get_drvdata(dev);
+	int flags = (acpi_desc->scrub_mode == HW_ERROR_SCRUB_ON) ?
+			0 : ND_ARS_RETURN_PREV_DATA;
 
-	acpi_nfit_ars_rescan(acpi_desc);
+	acpi_nfit_ars_rescan(acpi_desc, flags);
 }
 
 void __acpi_nfit_notify(struct device *dev, acpi_handle handle, u32 event)
diff --git a/drivers/acpi/nfit/mce.c b/drivers/acpi/nfit/mce.c
index fd86bec..feeb95d 100644
--- a/drivers/acpi/nfit/mce.c
+++ b/drivers/acpi/nfit/mce.c
@@ -79,7 +79,7 @@ static int nfit_handle_mce(struct notifier_block *nb, unsigned long val,
 			 * already in progress, just let that be the last
 			 * authoritative one
 			 */
-			acpi_nfit_ars_rescan(acpi_desc);
+			acpi_nfit_ars_rescan(acpi_desc, 0);
 		}
 		break;
 	}
diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h
index 6cf9d21..a6f8833 100644
--- a/drivers/acpi/nfit/nfit.h
+++ b/drivers/acpi/nfit/nfit.h
@@ -91,6 +91,7 @@ struct nfit_spa {
 	struct list_head list;
 	struct nd_region *nd_region;
 	unsigned int ars_required:1;
+	unsigned int ars_prev_data:1;
 	u32 clear_err_unit;
 	u32 max_ars;
 	struct acpi_nfit_system_address spa[0];
@@ -208,7 +209,7 @@ struct nfit_blk {
 
 extern struct list_head acpi_descs;
 extern struct mutex acpi_desc_lock;
-int acpi_nfit_ars_rescan(struct acpi_nfit_desc *acpi_desc);
+int acpi_nfit_ars_rescan(struct acpi_nfit_desc *acpi_desc, int flags);
 
 #ifdef CONFIG_X86_MCE
 void nfit_mce_register(void);
diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index 7ad3863..70a89f7 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -169,6 +169,7 @@ enum {
 enum {
 	ND_ARS_VOLATILE = 1,
 	ND_ARS_PERSISTENT = 2,
+	ND_ARS_RETURN_PREV_DATA = 1 << 1,
 	ND_CONFIG_LOCKED = 1,
 };
 

[toc] | [next] | [standalone]

#1667301

Thanks for this. I think it's going in the right direction, but one
comment below.

On Thu, Jun 8, 2017 at 11:36 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> ACPI 6.2 defines in section 9.20.7.2 that the OSPM may call a Start
> ARS with Flags Bit [1] set upon receiving the 0x81 notification.
>
>   Upon receiving the notification, the OSPM may decide to issue
>   a Start ARS with Flags Bit [1] set to prepare for the retrieval
>   of existing records and issue the Query ARS Status function to
>   retrieve the records.
>
> Add support to call a Start ARS from acpi_nfit_uc_error_notify()
> with ND_ARS_RETURN_PREV_DATA set when HW_ERROR_SCRUB_ON is not set.
>
> Link: http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Cc: Linda Knippers <linda.knippers@hpe.com>
> ---
>  drivers/acpi/nfit/core.c   |   14 +++++++++++---
>  drivers/acpi/nfit/mce.c    |    2 +-
>  drivers/acpi/nfit/nfit.h   |    3 ++-
>  include/uapi/linux/ndctl.h |    1 +
>  4 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index cc22778..d820d72 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -1031,7 +1031,7 @@ static ssize_t scrub_store(struct device *dev,
>         if (nd_desc) {
>                 struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
>
> -               rc = acpi_nfit_ars_rescan(acpi_desc);
> +               rc = acpi_nfit_ars_rescan(acpi_desc, 0);
>         }
>         device_unlock(dev);
>         if (rc)
> @@ -2057,6 +2057,10 @@ static int ars_start(struct acpi_nfit_desc *acpi_desc, struct nfit_spa *nfit_spa
>                 ars_start.type = ND_ARS_VOLATILE;
>         else
>                 return -ENOTTY;
> +       if (nfit_spa->ars_prev_data) {
> +               ars_start.flags |= ND_ARS_RETURN_PREV_DATA;
> +               nfit_spa->ars_prev_data = 0;
> +       }

I'd rather you plumb a new 'flags' parameter all the way through from
acpi_nfit_ars_rescan() to ars_start() rather than carrying this as a
property of nfit_spa.

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

#1668096 — RE: [PATCH v2 2/2] acpi/nfit: Issue Start ARS to retrieve existing records

> > --- a/drivers/acpi/nfit/core.c
> > +++ b/drivers/acpi/nfit/core.c
> > @@ -1031,7 +1031,7 @@ static ssize_t scrub_store(struct device *dev,
> >         if (nd_desc) {
> >                 struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
> >
> > -               rc = acpi_nfit_ars_rescan(acpi_desc);
> > +               rc = acpi_nfit_ars_rescan(acpi_desc, 0);
> >         }
> >         device_unlock(dev);
> >         if (rc)
> > @@ -2057,6 +2057,10 @@ static int ars_start(struct acpi_nfit_desc
> *acpi_desc, struct nfit_spa *nfit_spa
> >                 ars_start.type = ND_ARS_VOLATILE;
> >         else
> >                 return -ENOTTY;
> > +       if (nfit_spa->ars_prev_data) {
> > +               ars_start.flags |= ND_ARS_RETURN_PREV_DATA;
> > +               nfit_spa->ars_prev_data = 0;
> > +       }
> 
> I'd rather you plumb a new 'flags' parameter all the way through from
> acpi_nfit_ars_rescan() to ars_start() rather than carrying this as a
> property of nfit_spa.

Yes, I wanted to carry 'flags' all the way, but since acpi_nfit_ars_rescan()
calls acpi_nfit_scrub() via acpi_desc->work, all info needs to be marshalled
into struct acpi_nfit_desc.  Using nfit_spa allows a request to be carried as
per-spa basis...

Thanks,
-Toshi

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

#1668102

On Fri, Jun 16, 2017 at 5:01 PM, Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
>> > --- a/drivers/acpi/nfit/core.c
>> > +++ b/drivers/acpi/nfit/core.c
>> > @@ -1031,7 +1031,7 @@ static ssize_t scrub_store(struct device *dev,
>> >         if (nd_desc) {
>> >                 struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
>> >
>> > -               rc = acpi_nfit_ars_rescan(acpi_desc);
>> > +               rc = acpi_nfit_ars_rescan(acpi_desc, 0);
>> >         }
>> >         device_unlock(dev);
>> >         if (rc)
>> > @@ -2057,6 +2057,10 @@ static int ars_start(struct acpi_nfit_desc
>> *acpi_desc, struct nfit_spa *nfit_spa
>> >                 ars_start.type = ND_ARS_VOLATILE;
>> >         else
>> >                 return -ENOTTY;
>> > +       if (nfit_spa->ars_prev_data) {
>> > +               ars_start.flags |= ND_ARS_RETURN_PREV_DATA;
>> > +               nfit_spa->ars_prev_data = 0;
>> > +       }
>>
>> I'd rather you plumb a new 'flags' parameter all the way through from
>> acpi_nfit_ars_rescan() to ars_start() rather than carrying this as a
>> property of nfit_spa.
>
> Yes, I wanted to carry 'flags' all the way, but since acpi_nfit_ars_rescan()
> calls acpi_nfit_scrub() via acpi_desc->work, all info needs to be marshalled
> into struct acpi_nfit_desc.  Using nfit_spa allows a request to be carried as
> per-spa basis...

Ah ok, but I still think it does not belong to a spa. This is control
/ context information for the workqueue and that belongs with
acpi_nfit_desc. It think it's fine if all spas get re-scrubbed with
the "prev_data" flag in the the notification case.

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

#1668112 — RE: [PATCH v2 2/2] acpi/nfit: Issue Start ARS to retrieve existing records

> On Fri, Jun 16, 2017 at 5:01 PM, Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
> >> > --- a/drivers/acpi/nfit/core.c
> >> > +++ b/drivers/acpi/nfit/core.c
> >> > @@ -1031,7 +1031,7 @@ static ssize_t scrub_store(struct device *dev,
> >> >         if (nd_desc) {
> >> >                 struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
> >> >
> >> > -               rc = acpi_nfit_ars_rescan(acpi_desc);
> >> > +               rc = acpi_nfit_ars_rescan(acpi_desc, 0);
> >> >         }
> >> >         device_unlock(dev);
> >> >         if (rc)
> >> > @@ -2057,6 +2057,10 @@ static int ars_start(struct acpi_nfit_desc
> >> *acpi_desc, struct nfit_spa *nfit_spa
> >> >                 ars_start.type = ND_ARS_VOLATILE;
> >> >         else
> >> >                 return -ENOTTY;
> >> > +       if (nfit_spa->ars_prev_data) {
> >> > +               ars_start.flags |= ND_ARS_RETURN_PREV_DATA;
> >> > +               nfit_spa->ars_prev_data = 0;
> >> > +       }
> >>
> >> I'd rather you plumb a new 'flags' parameter all the way through from
> >> acpi_nfit_ars_rescan() to ars_start() rather than carrying this as a
> >> property of nfit_spa.
> >
> > Yes, I wanted to carry 'flags' all the way, but since acpi_nfit_ars_rescan()
> > calls acpi_nfit_scrub() via acpi_desc->work, all info needs to be marshalled
> > into struct acpi_nfit_desc.  Using nfit_spa allows a request to be carried as
> > per-spa basis...
> 
> Ah ok, but I still think it does not belong to a spa. This is control
> / context information for the workqueue and that belongs with
> acpi_nfit_desc. It think it's fine if all spas get re-scrubbed with
> the "prev_data" flag in the the notification case.

Sounds good.  I will add the flags info to acpi_nfit_desc and update
this patch.

Thanks!
-Toshi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web