On Fri, 30 Jul 2010 17:47:04 +0200
Thomas Renninger <[email protected]> wrote:

> -static int acpi_ec_open_io(struct inode *i, struct file *f)
> +static int acpi_ec_open(struct inode *i, struct file *f)
>  {
>       f->private_data = i->i_private;
> +     if (mutex_trylock(&ec_fs_lock))
> +             return 0;
> +     else
> +             return -EBUSY;
> +}

Well that sucks - a userspace interface which is _designed_ to randomly
and rarely fail?

An application tries to open the thing and gets -EBUSY, what's it
supposed to do?  Sleep and try again?  Crash and dump core?

Also the code's uncommented, so the reader has no clue why a trylock
was used.  There are very few circumstances where any trylock can
acceptably be left uncommented.

afacit we can use mutex_lock() here and the implementation would be
heaps better.
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" 
in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to