[RFC AR6000 patch V2] Set hardware unavailable during wext_ioctl when the suspend or rfkill is send
Michael Trimarchi
trimarchi at gandalf.sssup.it
Tue Mar 31 08:07:40 CEST 2009
Werner Almesberger wrote:
> Michael Trimarchi wrote:
>
>> Subject: [PATCH] The ioctl wext etc, seems to be broken because they don't take any lock
>>
>
> I now had a closer look at your patch. Sorry for taking so long.
>
> You've solved a nasty problem and I think the logic of your patch is
> good in general. However, there are two things that could be improved:
>
> - you had to change a lot of returns to ret = ...; goto out;
> There is an easier way: if you have a resource that's being held
> throughout most of a function, you can just put it into a wrapper
> function and leave the returns intact. E.g.,
>
> static int do_foo(stuff)
> {
> ...
> if (bad_things_happen)
> return -EPICFAILURE;
> ...
> }
>
> static int foo(stuff)
> {
> int ret;
>
> if (!try_grab_whatever())
> return -ETHISSUCKS;
> ret = do_foo();
> release_whatever();
> return ret;
> }
>
Ok,
> - as a rule of thumb, if you need more than two variables to implement
> a single locking/synchronization mechanism, there's usually an easier
> way to do it. In this case, I think you only need one variable :-)
>
> First of all, you should be able to get rid of the reference count
> if you use a read-write lock: make the ioctls take a read lock and
> the avail/unavail take a write lock. This moves the reference count
> into the lock.
>
>
->ioctl_start ..... by a process, your lock is not acquire
<--- in the middle you acquire the hardware lock and don't made the hw_unvalaible
---> release the lock at the end
-> call ioctl_wext crash
Using the hw_unvalaible you are sure that the function return. The priv data
I think that are valid until the intertal eth reference count is valid.
Michael
More information about the openmoko-kernel
mailing list