[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