linux-kernel - Re: [patch 2.6.29-rc7 regulator-next] regulator: refcount fixes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200903141429.24534.david-b@pacbell.net>
Date:	Sat, 14 Mar 2009 14:29:24 -0700
From:	David Brownell <david-b@...bell.net>
To:	Mark Brown <broonie@...ena.org.uk>
Cc:	Liam Girdwood <lrg@...mlogic.co.uk>,
	lkml <linux-kernel@...r.kernel.org>,
	OMAP <linux-omap@...r.kernel.org>
Subject: Re: [patch 2.6.29-rc7 regulator-next] regulator: refcount fixes

On Thursday 12 March 2009, Mark Brown wrote:
> On Thu, Mar 12, 2009 at 03:02:55PM -0800, David Brownell wrote:
>
> > One could as easily have "handle" and "regulator" be the
> > same ... so the get/put idioms could work like they do
> > elsewhere in the kernel.
> 
> I really don't see that there is any meaningful difference here; from
> the point of view of the consumer the fact that the thing it gets back
> is a handle to a structure the core uses to keep track of the consumer
> rather than the underlying hardware object is an implementation detail
> that shouldn't make any difference to them.  In terms of the programming
> model it seems like a layering violation to know the difference between
> one opaque structure and another.

You're not stepping back from the current interface, which is
a prerequisite to understanding the points I was making:

 * Almost everywhere else in the kernel, there's only one
   handle (no per-client facet idiom), for which get/put
   works.

   Having the handle alloc/free methods be called get/put
   is a kind of problem.  We want models and idioms to
   converge, not diverge, in almost all cases ... using
   the same names to mean different things isn't good.

 * The thing that *is* per-client is basically a constraint
   set ... but it's called a "regulator", which again is
   confusing.

In the regulator-next tree you've now moved regulator_dev
into the public interface ... that's the handle to the
real hardware.  Sort of a hint that it can't really be
hidden in the way you originally thought.

> > See above.  Currently constraints are hidden for "consumers",
> > behind functional accessors like regulator_set_voltage().
> > There are no explicit constraint objects, as there are for
> > the machines.
> 
> The current interface has been driven by the needs of the users: the
> majority of consumers want to do one operation on a regular basis -
> normally that's enable/disable, most devices are just powering
> themselves up and down, though for some things voltage changes are much
> more common (DVFS being the prime example).  Overall it's been fairly
> similar to the clock API in terms of usage pattern.

Except that the clock interface uses put/get in the normal way;
they are not alloc/free calls, just lookup/refcount calls.

> In terms of looking at redesigning the API

You were the one suggesting the need for a new call, formalizing
a model that didn't previously exist ... not me!  :)

Which is why I suggested taht if you were going to add calls,
it'd be worth thinking a bit more about some existing glitches.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/