All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ACPICA: Remove unnecessary call to debugger
@ 2017-06-08 21:31 Adam Borowski
  2017-06-08 21:43   ` [Devel] " Rafael J. Wysocki
  2017-06-09  1:18   ` [Devel] " Zheng, Lv
  0 siblings, 2 replies; 10+ messages in thread
From: Adam Borowski @ 2017-06-08 21:31 UTC (permalink / raw)
  To: Robert Moore, Lv Zheng, Rafael J. Wysocki, Len Brown, linux-acpi, devel
  Cc: Adam Borowski

This is a port of https://github.com/acpica/acpica/commit/eaa455ac by
Robert Moore to the kernel:

> This call was simply wrong, and resulted in a -1 index into the operand
> stack.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=120351
Signed-off-by: Adam Borowski <kilobyte@angband.pl>
---
You guys fixed this in your tool months ago, but it's still unfixed in the
kernel.  Let's apply that patch, then.  The coding styles differ so much the
patch is hardly recognizable, but it's a direct port.

 drivers/acpi/acpica/dsutils.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/acpi/acpica/dsutils.c b/drivers/acpi/acpica/dsutils.c
index 406edec20de7..0dabd9b95684 100644
--- a/drivers/acpi/acpica/dsutils.c
+++ b/drivers/acpi/acpica/dsutils.c
@@ -633,15 +633,6 @@ acpi_ds_create_operand(struct acpi_walk_state *walk_state,
 
 		if ((op_info->flags & AML_HAS_RETVAL) ||
 		    (arg->common.flags & ACPI_PARSEOP_IN_STACK)) {
-			ACPI_DEBUG_PRINT((ACPI_DB_DISPATCH,
-					  "Argument previously created, already stacked\n"));
-
-			acpi_db_display_argument_object(walk_state->
-							operands[walk_state->
-								 num_operands -
-								 1],
-							walk_state);
-
 			/*
 			 * Use value that was already previously returned
 			 * by the evaluation of this argument
-- 
2.11.0


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

* Re: [PATCH] ACPICA: Remove unnecessary call to debugger
@ 2017-06-08 21:43   ` Rafael J. Wysocki
  0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2017-06-08 21:43 UTC (permalink / raw)
  To: Adam Borowski, Lv Zheng
  Cc: Robert Moore, Rafael J. Wysocki, Len Brown,
	ACPI Devel Maling List, devel

On Thu, Jun 8, 2017 at 11:31 PM, Adam Borowski <kilobyte@angband.pl> wrote:
> This is a port of https://github.com/acpica/acpica/commit/eaa455ac by
> Robert Moore to the kernel:
>
>> This call was simply wrong, and resulted in a -1 index into the operand
>> stack.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=120351
> Signed-off-by: Adam Borowski <kilobyte@angband.pl>
> ---
> You guys fixed this in your tool months ago, but it's still unfixed in the
> kernel.  Let's apply that patch, then.  The coding styles differ so much the
> patch is hardly recognizable, but it's a direct port.

Lv, any comments?

>  drivers/acpi/acpica/dsutils.c | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/drivers/acpi/acpica/dsutils.c b/drivers/acpi/acpica/dsutils.c
> index 406edec20de7..0dabd9b95684 100644
> --- a/drivers/acpi/acpica/dsutils.c
> +++ b/drivers/acpi/acpica/dsutils.c
> @@ -633,15 +633,6 @@ acpi_ds_create_operand(struct acpi_walk_state *walk_state,
>
>                 if ((op_info->flags & AML_HAS_RETVAL) ||
>                     (arg->common.flags & ACPI_PARSEOP_IN_STACK)) {
> -                       ACPI_DEBUG_PRINT((ACPI_DB_DISPATCH,
> -                                         "Argument previously created, already stacked\n"));
> -
> -                       acpi_db_display_argument_object(walk_state->
> -                                                       operands[walk_state->
> -                                                                num_operands -
> -                                                                1],
> -                                                       walk_state);
> -
>                         /*
>                          * Use value that was already previously returned
>                          * by the evaluation of this argument
> --
> 2.11.0

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

* Re: [Devel] [PATCH] ACPICA: Remove unnecessary call to debugger
@ 2017-06-08 21:43   ` Rafael J. Wysocki
  0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2017-06-08 21:43 UTC (permalink / raw)
  To: devel

[-- Attachment #1: Type: text/plain, Size: 1880 bytes --]

On Thu, Jun 8, 2017 at 11:31 PM, Adam Borowski <kilobyte(a)angband.pl> wrote:
> This is a port of https://github.com/acpica/acpica/commit/eaa455ac by
> Robert Moore to the kernel:
>
>> This call was simply wrong, and resulted in a -1 index into the operand
>> stack.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=120351
> Signed-off-by: Adam Borowski <kilobyte(a)angband.pl>
> ---
> You guys fixed this in your tool months ago, but it's still unfixed in the
> kernel.  Let's apply that patch, then.  The coding styles differ so much the
> patch is hardly recognizable, but it's a direct port.

Lv, any comments?

>  drivers/acpi/acpica/dsutils.c | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/drivers/acpi/acpica/dsutils.c b/drivers/acpi/acpica/dsutils.c
> index 406edec20de7..0dabd9b95684 100644
> --- a/drivers/acpi/acpica/dsutils.c
> +++ b/drivers/acpi/acpica/dsutils.c
> @@ -633,15 +633,6 @@ acpi_ds_create_operand(struct acpi_walk_state *walk_state,
>
>                 if ((op_info->flags & AML_HAS_RETVAL) ||
>                     (arg->common.flags & ACPI_PARSEOP_IN_STACK)) {
> -                       ACPI_DEBUG_PRINT((ACPI_DB_DISPATCH,
> -                                         "Argument previously created, already stacked\n"));
> -
> -                       acpi_db_display_argument_object(walk_state->
> -                                                       operands[walk_state->
> -                                                                num_operands -
> -                                                                1],
> -                                                       walk_state);
> -
>                         /*
>                          * Use value that was already previously returned
>                          * by the evaluation of this argument
> --
> 2.11.0

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

* RE: [PATCH] ACPICA: Remove unnecessary call to debugger
@ 2017-06-09  0:35     ` Zheng, Lv
  0 siblings, 0 replies; 10+ messages in thread
From: Zheng, Lv @ 2017-06-09  0:35 UTC (permalink / raw)
  To: Rafael J. Wysocki, Adam Borowski
  Cc: Moore, Robert, Wysocki, Rafael J, Len Brown,
	ACPI Devel Maling List, devel

Hi,

> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of Rafael J. Wysocki
> Subject: Re: [PATCH] ACPICA: Remove unnecessary call to debugger
> 
> On Thu, Jun 8, 2017 at 11:31 PM, Adam Borowski <kilobyte@angband.pl> wrote:
> > This is a port of https://github.com/acpica/acpica/commit/eaa455ac by
> > Robert Moore to the kernel:
> >
> >> This call was simply wrong, and resulted in a -1 index into the operand
> >> stack.
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=120351
> > Signed-off-by: Adam Borowski <kilobyte@angband.pl>
> > ---
> > You guys fixed this in your tool months ago, but it's still unfixed in the
> > kernel.  Let's apply that patch, then.  The coding styles differ so much the
> > patch is hardly recognizable, but it's a direct port.
> 
> Lv, any comments?

The commit was originally sent by me with this commit:
https://github.com/zetalog/linux/commit/2cd640e5fd50
You can see full bug triage story here:
https://bugzilla.kernel.org/show_bug.cgi?id=194845
And upstream ACPICA discussion here:
https://github.com/acpica/acpica/pull/245
After the merge, bob helped to close another bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=120351

I already sent linuxized commit to you in the ACPICA release with restored patch description:
https://patchwork.kernel.org/patch/9765837/
Such modification is what we need to do in release cycle according to the community work.
You don't need to take this one.

Thanks and best regards
Lv

> >  drivers/acpi/acpica/dsutils.c | 9 ---------
> >  1 file changed, 9 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/dsutils.c b/drivers/acpi/acpica/dsutils.c
> > index 406edec20de7..0dabd9b95684 100644
> > --- a/drivers/acpi/acpica/dsutils.c
> > +++ b/drivers/acpi/acpica/dsutils.c
> > @@ -633,15 +633,6 @@ acpi_ds_create_operand(struct acpi_walk_state *walk_state,
> >
> >                 if ((op_info->flags & AML_HAS_RETVAL) ||
> >                     (arg->common.flags & ACPI_PARSEOP_IN_STACK)) {
> > -                       ACPI_DEBUG_PRINT((ACPI_DB_DISPATCH,
> > -                                         "Argument previously created, already stacked\n"));
> > -
> > -                       acpi_db_display_argument_object(walk_state->
> > -                                                       operands[walk_state->
> > -                                                                num_operands -
> > -                                                                1],
> > -                                                       walk_state);
> > -
> >                         /*
> >                          * Use value that was already previously returned
> >                          * by the evaluation of this argument
> > --
> > 2.11.0

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

* Re: [Devel] [PATCH] ACPICA: Remove unnecessary call to debugger
@ 2017-06-09  0:35     ` Zheng, Lv
  0 siblings, 0 replies; 10+ messages in thread
From: Zheng, Lv @ 2017-06-09  0:35 UTC (permalink / raw)
  To: devel

[-- Attachment #1: Type: text/plain, Size: 2826 bytes --]

Hi,

> From: rjwysocki(a)gmail.com [mailto:rjwysocki(a)gmail.com] On Behalf Of Rafael J. Wysocki
> Subject: Re: [PATCH] ACPICA: Remove unnecessary call to debugger
> 
> On Thu, Jun 8, 2017 at 11:31 PM, Adam Borowski <kilobyte(a)angband.pl> wrote:
> > This is a port of https://github.com/acpica/acpica/commit/eaa455ac by
> > Robert Moore to the kernel:
> >
> >> This call was simply wrong, and resulted in a -1 index into the operand
> >> stack.
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=120351
> > Signed-off-by: Adam Borowski <kilobyte(a)angband.pl>
> > ---
> > You guys fixed this in your tool months ago, but it's still unfixed in the
> > kernel.  Let's apply that patch, then.  The coding styles differ so much the
> > patch is hardly recognizable, but it's a direct port.
> 
> Lv, any comments?

The commit was originally sent by me with this commit:
https://github.com/zetalog/linux/commit/2cd640e5fd50
You can see full bug triage story here:
https://bugzilla.kernel.org/show_bug.cgi?id=194845
And upstream ACPICA discussion here:
https://github.com/acpica/acpica/pull/245
After the merge, bob helped to close another bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=120351

I already sent linuxized commit to you in the ACPICA release with restored patch description:
https://patchwork.kernel.org/patch/9765837/
Such modification is what we need to do in release cycle according to the community work.
You don't need to take this one.

Thanks and best regards
Lv

> >  drivers/acpi/acpica/dsutils.c | 9 ---------
> >  1 file changed, 9 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/dsutils.c b/drivers/acpi/acpica/dsutils.c
> > index 406edec20de7..0dabd9b95684 100644
> > --- a/drivers/acpi/acpica/dsutils.c
> > +++ b/drivers/acpi/acpica/dsutils.c
> > @@ -633,15 +633,6 @@ acpi_ds_create_operand(struct acpi_walk_state *walk_state,
> >
> >                 if ((op_info->flags & AML_HAS_RETVAL) ||
> >                     (arg->common.flags & ACPI_PARSEOP_IN_STACK)) {
> > -                       ACPI_DEBUG_PRINT((ACPI_DB_DISPATCH,
> > -                                         "Argument previously created, already stacked\n"));
> > -
> > -                       acpi_db_display_argument_object(walk_state->
> > -                                                       operands[walk_state->
> > -                                                                num_operands -
> > -                                                                1],
> > -                                                       walk_state);
> > -
> >                         /*
> >                          * Use value that was already previously returned
> >                          * by the evaluation of this argument
> > --
> > 2.11.0

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

* RE: [PATCH] ACPICA: Remove unnecessary call to debugger
@ 2017-06-09  1:18   ` Zheng, Lv
  0 siblings, 0 replies; 10+ messages in thread
From: Zheng, Lv @ 2017-06-09  1:18 UTC (permalink / raw)
  To: Adam Borowski, Moore, Robert, Wysocki, Rafael J, Len Brown,
	linux-acpi, devel

Hi, Adam

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Adam
> Borowski
> Sent: Friday, June 9, 2017 5:31 AM
> To: Moore, Robert <robert.moore@intel.com>; Zheng, Lv <lv.zheng@intel.com>; Wysocki, Rafael J
> <rafael.j.wysocki@intel.com>; Len Brown <lenb@kernel.org>; linux-acpi@vger.kernel.org;
> devel@acpica.org
> Cc: Adam Borowski <kilobyte@angband.pl>
> Subject: [PATCH] ACPICA: Remove unnecessary call to debugger
> 
> This is a port of https://github.com/acpica/acpica/commit/eaa455ac by
> Robert Moore to the kernel:
> 
> > This call was simply wrong, and resulted in a -1 index into the operand
> > stack.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=120351
> Signed-off-by: Adam Borowski <kilobyte@angband.pl>

I think my first fix also worked and your test on bug 120351 was wrong.
I just added comment a moment ago.

However I just pushed the removal of the wrong debugging statement fix to the upstream.
It's really redundant, you need to enable the log and check them.

The patch has already been sent here:
https://patchwork.kernel.org/patch/9765837/
So you don't have to send it again.

Thanks
Lv

> You guys fixed this in your tool months ago, but it's still unfixed in the
> kernel.  Let's apply that patch, then.  The coding styles differ so much the
> patch is hardly recognizable, but it's a direct port.
> 
>  drivers/acpi/acpica/dsutils.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/dsutils.c b/drivers/acpi/acpica/dsutils.c
> index 406edec20de7..0dabd9b95684 100644
> --- a/drivers/acpi/acpica/dsutils.c
> +++ b/drivers/acpi/acpica/dsutils.c
> @@ -633,15 +633,6 @@ acpi_ds_create_operand(struct acpi_walk_state *walk_state,
> 
>  		if ((op_info->flags & AML_HAS_RETVAL) ||
>  		    (arg->common.flags & ACPI_PARSEOP_IN_STACK)) {
> -			ACPI_DEBUG_PRINT((ACPI_DB_DISPATCH,
> -					  "Argument previously created, already stacked\n"));
> -
> -			acpi_db_display_argument_object(walk_state->
> -							operands[walk_state->
> -								 num_operands -
> -								 1],
> -							walk_state);
> -
>  			/*
>  			 * Use value that was already previously returned
>  			 * by the evaluation of this argument
> --
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [Devel] [PATCH] ACPICA: Remove unnecessary call to debugger
@ 2017-06-09  1:18   ` Zheng, Lv
  0 siblings, 0 replies; 10+ messages in thread
From: Zheng, Lv @ 2017-06-09  1:18 UTC (permalink / raw)
  To: devel

[-- Attachment #1: Type: text/plain, Size: 2535 bytes --]

Hi, Adam

> From: linux-acpi-owner(a)vger.kernel.org [mailto:linux-acpi-owner(a)vger.kernel.org] On Behalf Of Adam
> Borowski
> Sent: Friday, June 9, 2017 5:31 AM
> To: Moore, Robert <robert.moore(a)intel.com>; Zheng, Lv <lv.zheng(a)intel.com>; Wysocki, Rafael J
> <rafael.j.wysocki(a)intel.com>; Len Brown <lenb(a)kernel.org>; linux-acpi(a)vger.kernel.org;
> devel(a)acpica.org
> Cc: Adam Borowski <kilobyte(a)angband.pl>
> Subject: [PATCH] ACPICA: Remove unnecessary call to debugger
> 
> This is a port of https://github.com/acpica/acpica/commit/eaa455ac by
> Robert Moore to the kernel:
> 
> > This call was simply wrong, and resulted in a -1 index into the operand
> > stack.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=120351
> Signed-off-by: Adam Borowski <kilobyte(a)angband.pl>

I think my first fix also worked and your test on bug 120351 was wrong.
I just added comment a moment ago.

However I just pushed the removal of the wrong debugging statement fix to the upstream.
It's really redundant, you need to enable the log and check them.

The patch has already been sent here:
https://patchwork.kernel.org/patch/9765837/
So you don't have to send it again.

Thanks
Lv

> You guys fixed this in your tool months ago, but it's still unfixed in the
> kernel.  Let's apply that patch, then.  The coding styles differ so much the
> patch is hardly recognizable, but it's a direct port.
> 
>  drivers/acpi/acpica/dsutils.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/dsutils.c b/drivers/acpi/acpica/dsutils.c
> index 406edec20de7..0dabd9b95684 100644
> --- a/drivers/acpi/acpica/dsutils.c
> +++ b/drivers/acpi/acpica/dsutils.c
> @@ -633,15 +633,6 @@ acpi_ds_create_operand(struct acpi_walk_state *walk_state,
> 
>  		if ((op_info->flags & AML_HAS_RETVAL) ||
>  		    (arg->common.flags & ACPI_PARSEOP_IN_STACK)) {
> -			ACPI_DEBUG_PRINT((ACPI_DB_DISPATCH,
> -					  "Argument previously created, already stacked\n"));
> -
> -			acpi_db_display_argument_object(walk_state->
> -							operands[walk_state->
> -								 num_operands -
> -								 1],
> -							walk_state);
> -
>  			/*
>  			 * Use value that was already previously returned
>  			 * by the evaluation of this argument
> --
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo(a)vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH] ACPICA: Remove unnecessary call to debugger
  2017-06-09  1:18   ` [Devel] " Zheng, Lv
  (?)
@ 2017-06-09  1:33   ` Adam Borowski
  2017-06-09  2:56       ` [Devel] " Zheng, Lv
  -1 siblings, 1 reply; 10+ messages in thread
From: Adam Borowski @ 2017-06-09  1:33 UTC (permalink / raw)
  To: Zheng, Lv; +Cc: Moore, Robert, Wysocki, Rafael J, Len Brown, linux-acpi, devel

On Fri, Jun 09, 2017 at 01:18:25AM +0000, Zheng, Lv wrote:
> > Subject: [PATCH] ACPICA: Remove unnecessary call to debugger
> > 
> > This is a port of https://github.com/acpica/acpica/commit/eaa455ac by
> > Robert Moore to the kernel:
> > 
> > > This call was simply wrong, and resulted in a -1 index into the operand
> > > stack.
> > 
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=120351
> > Signed-off-by: Adam Borowski <kilobyte@angband.pl>
> 
> I think my first fix also worked and your test on bug 120351 was wrong.
> I just added comment a moment ago.
> 
> However I just pushed the removal of the wrong debugging statement fix to the upstream.
> It's really redundant, you need to enable the log and check them.
> 
> The patch has already been sent here:
> https://patchwork.kernel.org/patch/9765837/
> So you don't have to send it again.

Date: June 5, and it's not even in next-20170608 yet, so I obviously didn't
see you applying the fix (it seemed stalled since Apr 4).  If it's already
on the way, that's good.

However, it is not redundant: without this removal, I see the UBSAN error
(at least in current mainline).

Whether an alternate fix would be enough is another question.  You guys know
more about the issue, all I know is whether I see warnings spewed into my
dmesg :)


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ A tit a day keeps the vet away.
⣾⠁⢰⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ (Rejoice as my small-animal-murder-machine got unbroken after
⠈⠳⣄⠀⠀⠀⠀ nearly two years of no catch!)

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

* RE: [PATCH] ACPICA: Remove unnecessary call to debugger
@ 2017-06-09  2:56       ` Zheng, Lv
  0 siblings, 0 replies; 10+ messages in thread
From: Zheng, Lv @ 2017-06-09  2:56 UTC (permalink / raw)
  To: Adam Borowski
  Cc: Moore, Robert, Wysocki, Rafael J, Len Brown, linux-acpi, devel

Hi,

> From: Adam Borowski [mailto:kilobyte@angband.pl]
> Subject: Re: [PATCH] ACPICA: Remove unnecessary call to debugger
> 
> On Fri, Jun 09, 2017 at 01:18:25AM +0000, Zheng, Lv wrote:
> > > Subject: [PATCH] ACPICA: Remove unnecessary call to debugger
> > >
> > > This is a port of https://github.com/acpica/acpica/commit/eaa455ac by
> > > Robert Moore to the kernel:
> > >
> > > > This call was simply wrong, and resulted in a -1 index into the operand
> > > > stack.
> > >
> > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=120351
> > > Signed-off-by: Adam Borowski <kilobyte@angband.pl>
> >
> > I think my first fix also worked and your test on bug 120351 was wrong.
> > I just added comment a moment ago.
> >
> > However I just pushed the removal of the wrong debugging statement fix to the upstream.
> > It's really redundant, you need to enable the log and check them.
> >
> > The patch has already been sent here:
> > https://patchwork.kernel.org/patch/9765837/
> > So you don't have to send it again.
> 
> Date: June 5, and it's not even in next-20170608 yet, so I obviously didn't
> see you applying the fix (it seemed stalled since Apr 4).  If it's already
> on the way, that's good.

Yes, ACPICA release was frozen, waiting for spec 6.2 release.

> 
> However, it is not redundant: without this removal, I see the UBSAN error
> (at least in current mainline).

We have different meaning of redundant. :)

I mean:
I managed to enable the debugging log, and it always dumps things in this way:
 ArgObj: 00ADA610 Integer 0000000000000000
 ArgObj: 00ADA4F0 Integer 00000000FFFF00FF
 ArgObj: 00ADA4F0 Integer 00000000FFFF00FF <= redundant log generated by this line
 ArgObj: 00ADA580 Integer 0000000000000000
 ResultObj: 00ADA710 Integer 0000000000000000
In theory, AcpiDbDisplayArgumentObject() should be invoked for each AcpiDsObjStackPush().
There are only 2 AcpiDsObjStackPush() invocations.
Both of them have already been paired with AcpiDbDisplayArgumentObject() in AcpiDsCreateOperand().

This isolated AcpiDbDisplayArgumentObject() tries to find back missing stacked object debugging information.
			ACPI_DEBUG_PRINT((ACPI_DB_DISPATCH,
 					  "Argument previously created, already stacked\n"));
However there won't be such a missing object as all operands are created via AcpiDsCreateOperand().
Finally this invocation only generates above redundant debugging logs.

Cheers,
Lv

> 
> Whether an alternate fix would be enough is another question.  You guys know
> more about the issue, all I know is whether I see warnings spewed into my
> dmesg :)
> 
> 
> Meow!
> --
> ⢀⣴⠾⠻⢶⣦⠀ A tit a day keeps the vet away.
> ⣾⠁⢰⠒⠀⣿⡁
> ⢿⡄⠘⠷⠚⠋⠀ (Rejoice as my small-animal-murder-machine got unbroken after
> ⠈⠳⣄⠀⠀⠀⠀ nearly two years of no catch!)

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

* Re: [Devel] [PATCH] ACPICA: Remove unnecessary call to debugger
@ 2017-06-09  2:56       ` Zheng, Lv
  0 siblings, 0 replies; 10+ messages in thread
From: Zheng, Lv @ 2017-06-09  2:56 UTC (permalink / raw)
  To: devel

[-- Attachment #1: Type: text/plain, Size: 2889 bytes --]

Hi,

> From: Adam Borowski [mailto:kilobyte(a)angband.pl]
> Subject: Re: [PATCH] ACPICA: Remove unnecessary call to debugger
> 
> On Fri, Jun 09, 2017 at 01:18:25AM +0000, Zheng, Lv wrote:
> > > Subject: [PATCH] ACPICA: Remove unnecessary call to debugger
> > >
> > > This is a port of https://github.com/acpica/acpica/commit/eaa455ac by
> > > Robert Moore to the kernel:
> > >
> > > > This call was simply wrong, and resulted in a -1 index into the operand
> > > > stack.
> > >
> > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=120351
> > > Signed-off-by: Adam Borowski <kilobyte(a)angband.pl>
> >
> > I think my first fix also worked and your test on bug 120351 was wrong.
> > I just added comment a moment ago.
> >
> > However I just pushed the removal of the wrong debugging statement fix to the upstream.
> > It's really redundant, you need to enable the log and check them.
> >
> > The patch has already been sent here:
> > https://patchwork.kernel.org/patch/9765837/
> > So you don't have to send it again.
> 
> Date: June 5, and it's not even in next-20170608 yet, so I obviously didn't
> see you applying the fix (it seemed stalled since Apr 4).  If it's already
> on the way, that's good.

Yes, ACPICA release was frozen, waiting for spec 6.2 release.

> 
> However, it is not redundant: without this removal, I see the UBSAN error
> (at least in current mainline).

We have different meaning of redundant. :)

I mean:
I managed to enable the debugging log, and it always dumps things in this way:
 ArgObj: 00ADA610 Integer 0000000000000000
 ArgObj: 00ADA4F0 Integer 00000000FFFF00FF
 ArgObj: 00ADA4F0 Integer 00000000FFFF00FF <= redundant log generated by this line
 ArgObj: 00ADA580 Integer 0000000000000000
 ResultObj: 00ADA710 Integer 0000000000000000
In theory, AcpiDbDisplayArgumentObject() should be invoked for each AcpiDsObjStackPush().
There are only 2 AcpiDsObjStackPush() invocations.
Both of them have already been paired with AcpiDbDisplayArgumentObject() in AcpiDsCreateOperand().

This isolated AcpiDbDisplayArgumentObject() tries to find back missing stacked object debugging information.
			ACPI_DEBUG_PRINT((ACPI_DB_DISPATCH,
 					  "Argument previously created, already stacked\n"));
However there won't be such a missing object as all operands are created via AcpiDsCreateOperand().
Finally this invocation only generates above redundant debugging logs.

Cheers,
Lv

> 
> Whether an alternate fix would be enough is another question.  You guys know
> more about the issue, all I know is whether I see warnings spewed into my
> dmesg :)
> 
> 
> Meow!
> --
> ⢀⣴⠾⠻⢶⣦⠀ A tit a day keeps the vet away.
> ⣾⠁⢰⠒⠀⣿⡁
> ⢿⡄⠘⠷⠚⠋⠀ (Rejoice as my small-animal-murder-machine got unbroken after
> ⠈⠳⣄⠀⠀⠀⠀ nearly two years of no catch!)

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

end of thread, other threads:[~2017-06-09  2:57 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-08 21:31 [PATCH] ACPICA: Remove unnecessary call to debugger Adam Borowski
2017-06-08 21:43 ` Rafael J. Wysocki
2017-06-08 21:43   ` [Devel] " Rafael J. Wysocki
2017-06-09  0:35   ` Zheng, Lv
2017-06-09  0:35     ` [Devel] " Zheng, Lv
2017-06-09  1:18 ` Zheng, Lv
2017-06-09  1:18   ` [Devel] " Zheng, Lv
2017-06-09  1:33   ` Adam Borowski
2017-06-09  2:56     ` Zheng, Lv
2017-06-09  2:56       ` [Devel] " Zheng, Lv

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.