public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH] acpi: remove function tracing macros
@ 2006-01-16 18:14 Brown, Len
  2006-01-16 18:45 ` Pekka Enberg
  2006-01-19 13:36 ` Pavel Machek
  0 siblings, 2 replies; 6+ messages in thread
From: Brown, Len @ 2006-01-16 18:14 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: linux-acpi, linux-kernel


>From: Pekka Enberg <penberg@cs.helsinki.fi>
>
>This patch removes function tracing macro usage from drivers/acpi/. In
>particular, ACPI_FUNCTION_TRACE are ACPI_FUNCTION_NAME removed 
>completely and return_VALUE, return_PTR, and return_ACPI_STATUS
>are converted with proper use of return.
>
>I have not included the actual patch in this mail because it 
>is 600 KB in size. You can find the patch here:
>
>http://www.cs.helsinki.fi/u/penberg/linux/acpi-remove-function-tracing-macros.patch
>
>Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

I'm sorry, I can't apply this source clean-up patch.

We need tracing to debug interpreter failures on hardware
in the field.

And there is more to the long story, which I'll try to makes short here.

Linux shares the same dual-licensed ACPICA core interpreter with FreeBSD
Apple etc. -- indeed every ACPI-enabled OS other than Windows.
Linux gets a huge benefit from doing so.
In Linux, ACPICA refers to almost all the files under drivers/acpi/*.
(maybe I should re-arrange the source tree to make this more clear)

When we make GPL changes to those files, we diverge
from the rest of the universe and the overloaded
Linux/ACPI maintainer (me) starts to lose his sanity.
That said, if the author of the patch re-licenses it back
to Intel so it can be distributed under eitiher GPL or BSD,
then Intel can apply a change "up-stream" and divergence
can be avoided.  But per above, that isn't the primary
issue with ripping out tracing.

Note that tracing is built in only for CONFIG_ACPI_DEBUG.

Note that Patrick has an upcoming patch set that removes
tracing from the "pure GPL" drivers in drivers/acpi/*.c
where it isn't really needed.

Note that ACPICA 20060113 includes an additional patch
suggested by SuSE to split up CONFIG_ACPI_DEBUG
so that the tracing can be left out and the warnings
included.

thank you for your understanding.

-Len


^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: [PATCH] acpi: remove function tracing macros
@ 2006-01-16 20:01 Brown, Len
  2006-01-19 13:38 ` Pavel Machek
  0 siblings, 1 reply; 6+ messages in thread
From: Brown, Len @ 2006-01-16 20:01 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: linux-acpi, linux-kernel

 
>I appreciate that but per-function tracing is overkill.

I agree that 100% function coverage is overkill.
However, 100% is preferable to 0%.
The most desirable middle-ground will take some work
on the part of the folks that actually use the tracing.

>Especially since
>the macros used for it are very obfuscating (i.e. return_VALUE, et al)
>and we have things like kprobes.

Everybody agreees with you about the return_VALUE syntax.
The other irritating thing about this instrumentation is
that the function trace header looks like a call, but
is actually a declaration -- so sometimes DEBUG builds
break when executable code is put before it.  The fortunate
thing is that a relatively small group of people make
changes to the ACPI sub-system and this is one of the
things they learn about quickly:-)

If kprobes can give us the same functionality,
I'll be delighted if somebody can show me how.

>On Mon, 2006-01-16 at 13:14 -0500, Brown, Len wrote:
>> When we make GPL changes to those files, we diverge
>> from the rest of the universe and the overloaded
>> Linux/ACPI maintainer (me) starts to lose his sanity.
>> That said, if the author of the patch re-licenses it back
>> to Intel so it can be distributed under eitiher GPL or BSD,
>> then Intel can apply a change "up-stream" and divergence
>> can be avoided.  But per above, that isn't the primary
>> issue with ripping out tracing.
>> 
>> Note that tracing is built in only for CONFIG_ACPI_DEBUG.
>
>My main concern is that the ACPI subsystem uses obfuscating macros to
>implement function tracing in the kernel. Please note that we do not
>allow this in new code and there are janitor such as myself that are
>working to remove the existing ones.
>
>While I have no intention of making your life as Linux maintainer
>harder, I would appreciate if you would at least consider ripping out
>function tracing from upstream. I am certainly willing to relicense or
>even transfer copyrights of the patch if that's what you need.

I share your concern about source code style.
Note that as I mentioned, we've got some changes in the pipeline
to clean up drivers/acpi/*.c already.
Changing this in the upstream interpreter in drivers/acpi/*/
will be harder and will take longer.

thanks,
-Len

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] acpi: remove function tracing macros
@ 2006-01-11 16:01 Pekka Enberg
  0 siblings, 0 replies; 6+ messages in thread
From: Pekka Enberg @ 2006-01-11 16:01 UTC (permalink / raw)
  To: len.brown; +Cc: linux-acpi, linux-kernel

From: Pekka Enberg <penberg@cs.helsinki.fi>

This patch removes function tracing macro usage from drivers/acpi/. In
particular, ACPI_FUNCTION_TRACE are ACPI_FUNCTION_NAME removed completely
and return_VALUE, return_PTR, and return_ACPI_STATUS are converted with
proper use of return.

I have not included the actual patch in this mail because it is 600 KB in
size. You can find the patch here:

http://www.cs.helsinki.fi/u/penberg/linux/acpi-remove-function-tracing-macros.patch

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 drivers/acpi/ac.c                   |   60 ++------
 drivers/acpi/acpi_memhotplug.c      |  100 ++++---------
 drivers/acpi/battery.c              |  106 +++++---------
 drivers/acpi/bus.c                  |  118 ++++++----------
 drivers/acpi/button.c               |   64 +++-----
 drivers/acpi/container.c            |   40 +----
 drivers/acpi/debug.c                |   16 --
 drivers/acpi/dispatcher/dsfield.c   |   54 ++-----
 drivers/acpi/dispatcher/dsinit.c    |    6 
 drivers/acpi/dispatcher/dsmethod.c  |   52 ++-----
 drivers/acpi/dispatcher/dsmthdat.c  |   75 +++-------
 drivers/acpi/dispatcher/dsobject.c  |   42 ++---
 drivers/acpi/dispatcher/dsopcode.c  |   78 +++-------
 drivers/acpi/dispatcher/dsutils.c   |   58 ++-----
 drivers/acpi/dispatcher/dswexec.c   |   38 ++---
 drivers/acpi/dispatcher/dswload.c   |   34 +---
 drivers/acpi/dispatcher/dswscope.c  |   16 --
 drivers/acpi/dispatcher/dswstate.c  |   70 ++-------
 drivers/acpi/ec.c                   |  166 ++++++++--------------
 drivers/acpi/event.c                |   16 --
 drivers/acpi/events/evevent.c       |   22 ---
 drivers/acpi/events/evgpe.c         |   70 +++------
 drivers/acpi/events/evgpeblk.c      |   82 +++--------
 drivers/acpi/events/evmisc.c        |   30 +---
 drivers/acpi/events/evregion.c      |   68 +++------
 drivers/acpi/events/evrgnini.c      |   54 ++-----
 drivers/acpi/events/evsci.c         |   16 --
 drivers/acpi/events/evxface.c       |   62 +++-----
 drivers/acpi/events/evxfevnt.c      |   98 ++++---------
 drivers/acpi/events/evxfregn.c      |   16 --
 drivers/acpi/executer/exconfig.c    |   56 +++----
 drivers/acpi/executer/exconvrt.c    |   48 ++----
 drivers/acpi/executer/excreate.c    |   48 ++----
 drivers/acpi/executer/exdump.c      |   18 --
 drivers/acpi/executer/exfield.c     |   36 ++--
 drivers/acpi/executer/exfldio.c     |   62 +++-----
 drivers/acpi/executer/exmisc.c      |   34 +---
 drivers/acpi/executer/exmutex.c     |   32 +---
 drivers/acpi/executer/exnames.c     |   18 --
 drivers/acpi/executer/exoparg1.c    |   29 ---
 drivers/acpi/executer/exoparg2.c    |   22 ---
 drivers/acpi/executer/exoparg3.c    |   10 -
 drivers/acpi/executer/exoparg6.c    |    5 
 drivers/acpi/executer/exprep.c      |   36 +---
 drivers/acpi/executer/exregion.c    |   32 +---
 drivers/acpi/executer/exresnte.c    |   22 +--
 drivers/acpi/executer/exresolv.c    |   38 ++---
 drivers/acpi/executer/exresop.c     |   54 +++----
 drivers/acpi/executer/exstore.c     |   42 ++---
 drivers/acpi/executer/exstoren.c    |   14 -
 drivers/acpi/executer/exstorob.c    |   12 -
 drivers/acpi/executer/exsystem.c    |   38 +----
 drivers/acpi/executer/exutils.c     |   28 ---
 drivers/acpi/fan.c                  |   52 ++-----
 drivers/acpi/hardware/hwacpi.c      |   32 +---
 drivers/acpi/hardware/hwgpe.c       |   18 --
 drivers/acpi/hardware/hwregs.c      |   48 ++----
 drivers/acpi/hardware/hwsleep.c     |   70 +++------
 drivers/acpi/hardware/hwtimer.c     |   20 --
 drivers/acpi/hotkey.c               |  114 ++++-----------
 drivers/acpi/motherboard.c          |    6 
 drivers/acpi/namespace/nsaccess.c   |   22 +--
 drivers/acpi/namespace/nsalloc.c    |   36 +---
 drivers/acpi/namespace/nsdump.c     |   20 --
 drivers/acpi/namespace/nsdumpdv.c   |    4 
 drivers/acpi/namespace/nseval.c     |   42 ++---
 drivers/acpi/namespace/nsinit.c     |   24 +--
 drivers/acpi/namespace/nsload.c     |   42 ++---
 drivers/acpi/namespace/nsnames.c    |   18 --
 drivers/acpi/namespace/nsobject.c   |   32 +---
 drivers/acpi/namespace/nsparse.c    |   18 --
 drivers/acpi/namespace/nssearch.c   |   32 +---
 drivers/acpi/namespace/nsutils.c    |   67 ++-------
 drivers/acpi/namespace/nswalk.c     |   16 --
 drivers/acpi/namespace/nsxfeval.c   |   38 ++---
 drivers/acpi/namespace/nsxfname.c   |    2 
 drivers/acpi/numa.c                 |    2 
 drivers/acpi/osl.c                  |   50 ++----
 drivers/acpi/parser/psargs.c        |   44 ++----
 drivers/acpi/parser/psloop.c        |   33 ++--
 drivers/acpi/parser/psopcode.c      |    2 
 drivers/acpi/parser/psparse.c       |   20 --
 drivers/acpi/parser/psscope.c       |   22 ---
 drivers/acpi/parser/pstree.c        |    8 -
 drivers/acpi/parser/psutils.c       |    6 
 drivers/acpi/parser/pswalk.c        |    6 
 drivers/acpi/parser/psxface.c       |   16 --
 drivers/acpi/pci_bind.c             |   42 ++---
 drivers/acpi/pci_irq.c              |   79 ++++------
 drivers/acpi/pci_link.c             |  120 ++++++----------
 drivers/acpi/pci_root.c             |   28 +--
 drivers/acpi/power.c                |  137 +++++++-----------
 drivers/acpi/processor_core.c       |  120 +++++-----------
 drivers/acpi/processor_idle.c       |   82 ++++-------
 drivers/acpi/processor_perflib.c    |   98 +++++--------
 drivers/acpi/processor_thermal.c    |   42 ++---
 drivers/acpi/processor_throttling.c |   48 ++----
 drivers/acpi/resources/rsaddr.c     |   44 +-----
 drivers/acpi/resources/rscalc.c     |   16 --
 drivers/acpi/resources/rscreate.c   |   42 ++---
 drivers/acpi/resources/rsdump.c     |   30 ----
 drivers/acpi/resources/rsio.c       |   26 ---
 drivers/acpi/resources/rsirq.c      |   22 ---
 drivers/acpi/resources/rslist.c     |   16 --
 drivers/acpi/resources/rsmemory.c   |   24 ---
 drivers/acpi/resources/rsmisc.c     |   36 +---
 drivers/acpi/resources/rsutils.c    |   32 +---
 drivers/acpi/resources/rsxface.c    |   38 +----
 drivers/acpi/scan.c                 |  104 ++++----------
 drivers/acpi/sleep/proc.c           |   12 -
 drivers/acpi/sleep/wakeup.c         |    5 
 drivers/acpi/system.c               |   22 ---
 drivers/acpi/tables/tbconvrt.c      |   18 --
 drivers/acpi/tables/tbget.c         |   54 ++-----
 drivers/acpi/tables/tbgetall.c      |   38 ++---
 drivers/acpi/tables/tbinstal.c      |   44 ++----
 drivers/acpi/tables/tbrsdt.c        |   26 +--
 drivers/acpi/tables/tbutils.c       |   14 -
 drivers/acpi/tables/tbxface.c       |   52 ++-----
 drivers/acpi/tables/tbxfroot.c      |   58 +++----
 drivers/acpi/thermal.c              |  188 +++++++++----------------
 drivers/acpi/utilities/utalloc.c    |   44 +-----
 drivers/acpi/utilities/utcache.c    |   10 -
 drivers/acpi/utilities/utcopy.c     |   50 ++----
 drivers/acpi/utilities/utdelete.c   |   34 +---
 drivers/acpi/utilities/uteval.c     |   60 +++-----
 drivers/acpi/utilities/utglobal.c   |    6 
 drivers/acpi/utilities/utinit.c     |   10 -
 drivers/acpi/utilities/utmath.c     |   24 +--
 drivers/acpi/utilities/utmisc.c     |   52 ++-----
 drivers/acpi/utilities/utmutex.c    |   26 ---
 drivers/acpi/utilities/utobject.c   |   55 ++-----
 drivers/acpi/utilities/utstate.c    |   40 +----
 drivers/acpi/utilities/utxface.c    |   50 ++----
 drivers/acpi/utils.c                |   58 +++----
 drivers/acpi/video.c                |  262 ++++++++++++------------------------
 136 files changed, 2103 insertions(+), 3878 deletions(-)


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-01-19 14:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-01-16 18:14 [PATCH] acpi: remove function tracing macros Brown, Len
2006-01-16 18:45 ` Pekka Enberg
2006-01-19 13:36 ` Pavel Machek
  -- strict thread matches above, loose matches on Subject: below --
2006-01-16 20:01 Brown, Len
2006-01-19 13:38 ` Pavel Machek
2006-01-11 16:01 Pekka Enberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox