linux-kernel - Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.

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: <20120925111243.GR9137@arwen.pp.htv.fi>
Date:	Tue, 25 Sep 2012 14:12:43 +0300
From:	Felipe Balbi <balbi@...com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	Felipe Balbi <balbi@...com>,
	"Poddar, Sourav" <sourav.poddar@...com>,
	gregkh@...uxfoundation.org, khilman@...com, paul@...an.com,
	tony@...mide.com, linux-kernel@...r.kernel.org,
	santosh.shilimkar@...com, linux-serial@...r.kernel.org,
	linux-omap@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	alan@...ux.intel.com
Subject: Re: [RFT/PATCH] serial: omap: prevent resume if device is not
 suspended.

Hi,

On Tue, Sep 25, 2012 at 12:07:03PM +0100, Russell King - ARM Linux wrote:
> On Tue, Sep 25, 2012 at 01:37:03PM +0300, Felipe Balbi wrote:
> > On Tue, Sep 25, 2012 at 11:29:58AM +0100, Russell King - ARM Linux wrote:
> > > Because you are accusing me of potentially breaking your beagleboard
> > > for merely suggesting further investigation and a better commit message.
> > 
> > Where did I accuse you of anyting ? I just mentioned we experienced a
> > regression with beagleboard XM when using pm_runtime_set_active().
> 
> I quote:
> :> But should we cause a regression to beagleboard XM because of that ?
>    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

maybe that wasn't the best way to put it, but I was trying to point out
that either there was a bug on pm core or omap_device/hwmod, not that we
shouldn't investigate.

> I say again: I was _only_ suggesting further investigation, yet you
> were mouthing off about causing a regression to beagleboard "because
> of that", effectively saying that no, we should not do any further
> investigation and this is the only fix.

not what I meant, but fair enough... The "because of that" was supposed
to mean "because of pm_runtime_set_active()". I find the documentation
for that particular call to be rather poor and a bit confusing,
specially when further down, the very same document uses a
"is_suspended" flag which, in fact, shouldn't be needed when we have
pm_runtime_is_suspended() and the like.

> > We pinged Paul and asked if he had seen that before, he had no
> > pointers... Because Documentation/power/runtime_pm.txt was using a
> > mystruct->is_suspended flag, we just decided to follow the same
> > "design" since no-one was able to suggest why pm_runtime_set_active()
> > was breaking beagleXM nor how it was supposed to actually work.
> > 
> > Reading the code: pm_runtime_set_active() would tell pm_runtime core
> > the device is actually active by setting runtime_status to RPM_ACTIVE,
> > thus the following pm_runtime_get_sync() wouldn't actually call
> > runtime_resume() callback, but it would increment usage_counter.
> > 
> > I can't see why this would fail on beagleXM, but it does and we'd like
> > to hear in which situations this could fail...
> 
> Well, I've just spent five minutes analysing the code paths - which I
> hadn't looked at before - and I've pointed out what's probably causing
> the problem for Beagle.  I think you owe me an appology over your
> aggressive attitude towards my suggestions that there needed to be
> some further investigation.

I don't see any aggressive attitude towards what you suggested,
actually. Mailing list archives are available to check, but the one
cursing around was always yourself and THAT deserves an apology.

If you still think I've been at all aggressive, then sure, I apologize,
it wasn't what I meant though.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)