linux-kernel - Re: [PATCH 03/15] ACPICA: move common private headers under kernel/acpi/acpica/

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 2 Jan 2009 20:14:51 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Len Brown <lenb@...nel.org>
Cc:	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	Len Brown <len.brown@...el.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 03/15] ACPICA: move common private headers under
	kernel/acpi/acpica/


* Len Brown <lenb@...nel.org> wrote:

> From: Len Brown <len.brown@...el.com>
> 
> Signed-off-by: Len Brown <len.brown@...el.com>
> ---

>  kernel/acpi/acpica/dsfield.c                    |    2 +-
>  kernel/acpi/acpica/dsinit.c                     |    2 +-
>  kernel/acpi/acpica/dsmethod.c                   |    2 +-
>  kernel/acpi/acpica/dsmthdat.c                   |    2 +-
>  kernel/acpi/acpica/dsobject.c                   |    2 +-
>  kernel/acpi/acpica/dsopcode.c                   |    2 +-
>  kernel/acpi/acpica/dsutils.c                    |    2 +-
>  kernel/acpi/acpica/dswexec.c                    |    2 +-
>  kernel/acpi/acpica/dswload.c                    |    2 +-
>  kernel/acpi/acpica/dswscope.c                   |    2 +-
>  kernel/acpi/acpica/dswstate.c                   |    2 +-
>  kernel/acpi/acpica/evevent.c                    |    2 +-
>  kernel/acpi/acpica/evgpe.c                      |    2 +-
>  kernel/acpi/acpica/evgpeblk.c                   |    2 +-
>  kernel/acpi/acpica/evmisc.c                     |    2 +-
>  kernel/acpi/acpica/evregion.c                   |    2 +-
>  kernel/acpi/acpica/evrgnini.c                   |    2 +-
>  kernel/acpi/acpica/evsci.c                      |    2 +-
>  kernel/acpi/acpica/evxface.c                    |    2 +-
>  kernel/acpi/acpica/evxfevnt.c                   |    2 +-
>  kernel/acpi/acpica/evxfregn.c                   |    2 +-
>  kernel/acpi/acpica/exconfig.c                   |    2 +-
>  kernel/acpi/acpica/exconvrt.c                   |    2 +-
>  kernel/acpi/acpica/excreate.c                   |    2 +-
>  kernel/acpi/acpica/exdump.c                     |    2 +-
>  kernel/acpi/acpica/exfield.c                    |    2 +-
>
>  [... etc ...]

hm, dunno. Do we really want to introduce 'driver/platform' space items 
like this in the core kernel/* ?

If it goes there then IMHO the ACPI code needs to be cleaned up 
_significantly_ to not wrap native Linux calls like spinlocks, allocators, 
etc.

Random example - i dont think stuff like this is readable [in to-be 
kernel/acpi/utilities/utcache.c]:

        if (cache->current_depth >= cache->max_depth) {
                ACPI_FREE(object);
                ACPI_MEM_TRACKING(cache->total_freed++);
        }

        /* Otherwise put this object back into the cache */

        else {
                status = acpi_ut_acquire_mutex(ACPI_MTX_CACHES);
                if (ACPI_FAILURE(status)) {
                        return (status);
                }

acpi_ut_acquire_mutex() under [to-be] kernel/acpi/utilities/utmutex.c 
looks absolutely horrible, redirected after some debugging layer to its 
final destination:

./include/acpi/acpiosxf.h:#define acpi_os_acquire_mutex(handle,time) 
acpi_os_wait_semaphore (handle, 1, time), which does [kernel/acpi/osl.c]:

/*
 * TODO: Support for units > 1?
 */
acpi_status acpi_os_wait_semaphore(acpi_handle handle, u32 units, u16 timeout)
{
	acpi_status status = AE_OK;
	struct semaphore *sem = (struct semaphore *)handle;
	long jiffies;
	int ret = 0;

	if (!sem || (units < 1))
		return AE_BAD_PARAMETER;

	if (units > 1)
		return AE_SUPPORT;

	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Waiting for semaphore[%p|%d|%d]\n",
			  handle, units, timeout));

	if (timeout == ACPI_WAIT_FOREVER)
		jiffies = MAX_SCHEDULE_TIMEOUT;
	else
		jiffies = msecs_to_jiffies(timeout);
	
	ret = down_timeout(sem, jiffies);
	if (ret)
		status = AE_TIME;

	if (ACPI_FAILURE(status)) {
		ACPI_DEBUG_PRINT((ACPI_DB_MUTEX,
				  "Failed to acquire semaphore[%p|%d|%d], %s",
				  handle, units, timeout,
				  acpi_format_exception(status)));
	} else {
		ACPI_DEBUG_PRINT((ACPI_DB_MUTEX,
				  "Acquired semaphore[%p|%d|%d]", handle,
				  units, timeout));
	}

	return status;
}

so it's a glorified down_timeout(). While we have mutex_timeout(). What's 
the plan here?

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/