Date:	Sun, 1 Mar 2009 10:20:51 +0100
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Arve Hjønnevåg <arve@...roid.com>
Cc:	Pavel Machek <pavel@....cz>,
	Alan Stern <stern@...land.harvard.edu>,
	"Woodruff, Richard" <r-woodruff2@...com>,
	Arjan van de Ven <arjan@...radead.org>,
	Kyle Moffett <kyle@...fetthome.net>,
	Oliver Neukum <oliver@...kum.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	pm list <linux-pm@...ts.linux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Nigel Cunningham <nigel@...el.suspend2.net>,
	Matthew Garrett <mjg59@...f.ucam.org>,
	mark gross <mgross@...ux.intel.com>,
	Uli Luckas <u.luckas@...d.de>,
	Igor Stoppa <igor.stoppa@...ia.com>,
	Brian Swetland <swetland@...gle.com>,
	Len Brown <lenb@...nel.org>
Subject: Re: [RFD] Automatic suspend

On Sunday 01 March 2009, Arve Hjønnevåg wrote:
> On Sat, Feb 28, 2009 at 2:53 PM, Rafael J. Wysocki <rjw@...k.pl> wrote:
> >> >> What you are talking about here is mostly an optimization of the
> >> >> wakelock api. You have removed timeout support and made each wakelock
> >> >> reference counted.
> >> >
> >> > I also removed the arbitrary number of wakelocks (I really _hate_ the name,
> >> > can we please stop using it from now on?).
> >>
> >> What do you mean by this? You removed the struct wake_lock?
> >
> > Basically, yes.
> 
> Why is this better? If you move the stats into devices and tasks, this
> may take up more space than adding an object to the structures or
> tasks that you are protecting.

OK, this is a valid point as long as we're going to use the stats in the
original form (which I'm not sure we want).

Still, as far as the number of "locks" one process can take is limited, it's
fine.

> >> >> I do provide an option to turn off the wakelock stats, which makes
> >> >> wake_lock/unlock significantly faster, but we never run with wakelock
> >> >> stats off. Also, it pushes timeout handling to the drivers. I know may
> >> >> of you don't like timeout support, but ignoring the problem is not a
> >> >> solution. If each driver that needs timeouts uses its own timer, then
> >> >> you will often wakeup from idle just to unlock a wakelock that will
> >> >> not trigger suspend. This wakeup is a thousand times as costly on the
> >> >> msm platform as a wakelock/unlock pair (with wakelock stats enabled).
> >> >
> >> > Well, at least a couple of people told you that the timeouts are hardly
> >> > acceptable and they told you why.  Please stop repeating the same arguments you
> >> > have given already for a couple of times.  They're not convincing.
> >>
> >> And you keep ignoring them.
> >
> > Not ignoring, but considering them as insufficient.  And since they've already
> > been considered as insufficient, there's no point repeating them over and over
> > again.  That doesn't make them any better.
> 
> The problem is that what you consider insufficient is what allows us
> to ship a product.

This doesn't matter a whit, because the mainline kernel is not just your
product.

By the same rule you could say that every working vendor driver is worth
merging into the mainline kernel, which clearly is not the case.

> >> > I do realize that you consider your current solution as the best thing since
> >> > the sliced bread, but please accept the fact that the other people think
> >> > differently.
> >>
> >> I certainly do not think my current solution is the best, it is very
> >> invasive. I do however think your proposed solution is worse. The only
> >> proposed alternative that we could actually ship a product on today is
> >> to not use suspend at all.
> >
> > Well, I'm sure your code is useful for the Android platform, but the question
> > is whether we want this code in the mainline kernel.  For now, the answer is
> > "no, we don't".  Moreover, since you're the one who wants the code to be
> > merged, it's your burden to make it acceptable for us.  However you're going
> > to do it is up to you, but certainly trying to force your current code on us
> > is not going to work.
> 
> I don't think I am the only one who want this code in the mainline
> kernel. Many people want to use the android platform, and support in
> the mainline kernel would be beneficial to some of them. I made many
> requested changes to my code that provides no benefit to us, but I
> have not made any changes that breaks our own use.

OK, please resubmit the patches, then.

> > BTW, I think you handled the thing wrong.  If I were you, I wouldn't even try
> > to push the code as you did.  I would instead make it as simple as reasonably
> > possible so that the basic idea was clear and understandable to everyone.
> > Then, if there were any doubts with respect to the basic idea, I'd try to
> > clarify them and I'd consider modifying the code to address objections.
> > I'd only try to add more features after the basic idea had been accepted.
> 
> The basic concept was developed long before android was a public project.

Please refer to the Ben's message for a good answer to this.

Thanks,
Rafael
--
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/

Powered by blists - more mailing lists