lkml.org 
[lkml]   [2008]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC] x86: Switch apm to unlocked_kernel
Kevin Winchester wrote:
> On Thu, May 22, 2008 at 5:22 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> This pushes the lock a fair way down and the final kill looks like it
>> should be an easy project for someone who wants to have a shot at it.
>>
>
> I would like to have a shot at this if no one else if currently
> working on it. This would be my first attempt to graduate from
> testing (and a few simple patches) to an actual functional change. I
> would expect that the steps involved are:
>
> - See what data is affected under the BKL
> - See where else that data may be affected (including possible
> parallel calls to the same path)
> - Decide what locking is necessary
> - Implement it (the easy part)
>
> If this is about right (and I'm not missing anything major) - I will
> give it a try and see where I get.
>
> If, however, I have just proved that I do not have a full grasp of the
> problem, or if someone else is already working on this particular path
> - please let me know.
>

(Added Stephen to cc as he is the maintainer here)

After looking at this for a while to figure out the necessary locking, I
discovered drivers/char/apm_emulation.c, which has very similar
structures to arch/x86/kernel/apm_32.c, and has much more locking (e.g.
A state_lock mutex, a user_list_lock rwsem) and it uses a list_head for
the apm_user_list instead of an open coded doubly linked list.

This brought up the following question:

- Should I just copy the locking from apm_emulation to apm? Another
patch from Alan moved the BKL down into the apm_emulation ioctl method,
but it seems pretty well locked as it is - so that BKL can probably just
be removed there.

Also - as far as I can tell, the two files both do the exact same thing
from the point of view of the kapmd thread and /dev/apm_bios file, but
the real apm makes actual bios calls to perform the suspend/standby
operations, and apm_emulation just uses pm methods like pm_suspend().

- Thus, shouldn't the two files share code from a common third file?

If so, I could take on the task of that refactoring, if I were given an
idea of where I should put the new file (where does an x86 and
drivers/char common file go?).

Possible reasons this may not be worth it include:

- The APM code is possibly fragile, and not often used anymore. If this
is the case, then perhaps just adding the same locking from the
emulation file to the true apm file would be a good idea.
- There are subtle differences I am not seeing that requires the code to
be quite separate. If so, please let me know.


Thank you for any comments,

--
Kevin Winchester


\
 
 \ /
  Last update: 2008-05-24 21:01    [W:0.112 / U:0.492 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site