From: Andrew Morton <akpm@linux-foundation.org>
To: Thomas Renninger <trenn@suse.de>
Cc: mjg59@srcf.ucam.org, platform-driver-x86@vger.kernel.org,
linux-acpi@vger.kernel.org, astarikovskiy@suse.de
Subject: Re: [PATCH] acpi ec_sys: Export fields of all regions from the EC to debugfs readable
Date: Fri, 30 Jul 2010 16:30:53 -0700 [thread overview]
Message-ID: <20100730163053.bff43f26.akpm@linux-foundation.org> (raw)
In-Reply-To: <1280504824-8134-1-git-send-email-trenn@suse.de>
On Fri, 30 Jul 2010 17:47:04 +0200
Thomas Renninger <trenn@suse.de> 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.
next prev parent reply other threads:[~2010-07-30 23:30 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-30 15:47 [PATCH] acpi ec_sys: Export fields of all regions from the EC to debugfs readable Thomas Renninger
2010-07-30 23:30 ` Andrew Morton [this message]
2010-08-01 0:36 ` Thomas Renninger
2010-08-04 13:53 ` [PATCH] acpi ec_sys: Export fields of all regions from the EC to debugfs readable (v2) Thomas Renninger
2010-08-04 14:19 ` [PATCH] acpi ec_sys: Export fields of all regions from the EC to debugfs readable (v3) Thomas Renninger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100730163053.bff43f26.akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=astarikovskiy@suse.de \
--cc=linux-acpi@vger.kernel.org \
--cc=mjg59@srcf.ucam.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=trenn@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox