Prev: [PATCH 02/25] lmb: No reason to include asm/lmb.h late
Next: perf: core, remove hw_perf_event_init
From: Anton Vorontsov on 11 May 2010 14:40
On Tue, May 11, 2010 at 07:58:12PM +0200, Daniel Mack wrote:
[...]
> > Plus, that way you don't need is_writable().
>
> Ugh, really? I would _much_ prefer to actually _see_ which property is
> writeable, just from looking at the file attributes in sysfs.

Well, yeah... that would be also good for userland apps (GUI
apps, in particular). Though, I'm not yet sure how to implement
it without modifying a global array...

To avoid the race that I described in previous email we could
insert some mutex, but I'd like to avoid the global stuff.

--
Anton Vorontsov
email: cbouatmailru(a)gmail.com
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Daniel Mack on 11 May 2010 18:30
On Tue, May 11, 2010 at 10:23:47PM +0400, Anton Vorontsov wrote:
> On Tue, May 11, 2010 at 07:58:12PM +0200, Daniel Mack wrote:
> [...]
> > Hmm, no. The code defaults to (S_IRUSR | S_IRGRP | S_IROTH) which is
> > 0444, just like it was before. So there shouldn't be any regression. The
> > mode is only changed if the psy defines a property_is_writeable()
> > callback which returns 1. Or do I miss your point?
>
> Yes, power_supply_attrs is a global array, and you shouldn't change
> it between power_supply_register() calls.
>
> If you don't see why it's a bad idea in general, think about it
> other way, a race:
>
> ...someone registers psy0 with attr X marked as read-only...
> ...code flow stops before device_create_file(psy0, global->mode)..
> [preempt]
> ...someone registers psy1 with attr X marked as writable...
> ...you set up global->mode to 0644...
> [preempt again]
> ...we end up calling device_create_file(psy0, 0644)...

Ah, I see. But the struct passed in is just used as template, right? So
for the particular case you outlined, a simple lock should do the trick,
right? I would prefer this over always-writeable file entries.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 18 May 2010 16:10
On Tue, May 18, 2010 at 09:49:52PM +0200, Daniel Mack wrote:
> This patch adds support for writeable power supply properties and
> exposes them as writeable to sysfs.

Please properly document this in Documentation/ABI as you are changing
the userspace ABI.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  | 
Pages: 1 2
Prev: [PATCH 02/25] lmb: No reason to include asm/lmb.h late
Next: perf: core, remove hw_perf_event_init