[RFC AR6000 patch V2] Set hardware unavailable during wext_ioctl when the suspend or rfkill is send
Werner Almesberger
werner at openmoko.org
Tue Mar 31 02:49:38 CEST 2009
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;
}
- 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.
But you can do even better: also hw_available isn't necessary if you
use the lock itself as an indicator: when the hardware becomes
available, obtain the write lock. When the hardware becomes available,
release the lock. In the ioctls, just down_read_trylock. If you can't
get the lock, you know the hardware is unavailable.
Can you please make a revised patch with these simplifications ?
Thanks,
- Werner
More information about the openmoko-kernel
mailing list