Re: [patch 15/15] Generic Mutex Subsystem, arch-semaphores.patch

[Christoph Hellwig <hch@infradead.org>]

On Mon, Dec 19, 2005 at 09:34:06AM -0500, Steven Rostedt wrote:
> >  acpi_status
> >  acpi_os_create_semaphore(u32 max_units, u32 initial_units,
> > acpi_handle * handle)
> >  {
> > -       struct semaphore *sem = NULL;
> > +       struct arch_semaphore *sem = NULL;
> >  
> >         ACPI_FUNCTION_TRACE("os_create_semaphore");
> >  
> > -       sem = acpi_os_allocate(sizeof(struct semaphore));
> > +       sem = acpi_os_allocate(sizeof(struct arch_semaphore));
> 
> [OT]
> This is why I prefer sizeof(*sem) over sizeof(struct type_of_sem) but I
> regress.  And I don't buy that argument of the mistaken sizeof(sem)
> since, I've never had to deal with that bug!  Oh well, each to their
> own.

What's more important is that acpi is doing something fundamentally stupid
here.  Putting a lock into a separate allocation (and the allocator wrapped
again..) is just freaking stupid, period.

Someone needs to go through ACPI and rewrite this freaking junk into proper
linux code.  Maybe it'd even get less buggy if a single person finally had
a chance to actually understand all of the code after only half of it is
left ;-) 

> 
> -- Steve
> 
> >         if (!sem)
> >                 return_ACPI_STATUS(AE_NO_MEMORY);
> > -       memset(sem, 0, sizeof(struct semaphore));
> > +       memset(sem, 0, sizeof(struct arch_semaphore));
> >         sema_init(sem, initial_units);

and a memset to it, WTF..  seems like the acpi people just need to be shot.