linux-kernel - Re: [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static

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: <201202131653.02013.rjw@sisk.pl>
Date:	Mon, 13 Feb 2012 16:53:01 +0100
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>
Cc:	Linux PM list <linux-pm@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Randy Dunlap <rdunlap@...otime.net>
Subject: Re: [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static

On Monday, February 13, 2012, Srivatsa S. Bhat wrote:
> On 02/13/2012 08:42 PM, Rafael J. Wysocki wrote:
> 
> > On Monday, February 13, 2012, Srivatsa S. Bhat wrote:
> >> On 02/13/2012 02:54 AM, Rafael J. Wysocki wrote:
> >>
> >>> On Sunday, February 12, 2012, Srivatsa S. Bhat wrote:
> >>>> On 02/12/2012 04:34 AM, Rafael J. Wysocki wrote:
> >>>>
> >>>>> From: Rafael J. Wysocki <rjw@...k.pl>
> >>>>>
> >>>>> The enter_state() function in kernel/power/suspend.c should be
> >>>>> static and state_store() in kernel/power/suspend.c should call
> >>>>> pm_suspend() instead of it, so make that happen (which also reduces
> >>>>> code duplication related to suspend statistics).
> >>>>>
> >>>>> Signed-off-by: Rafael J. Wysocki <rjw@...k.pl>
> >>>>> ---
> >>>>>  kernel/power/main.c    |    6 +-----
> >>>>>  kernel/power/power.h   |    2 --
> >>>>>  kernel/power/suspend.c |    2 +-
> >>>>>  3 files changed, 2 insertions(+), 8 deletions(-)
> >>>>>
> >>>>> Index: linux/kernel/power/main.c
> >>>>> ===================================================================
> >>>>> --- linux.orig/kernel/power/main.c
> >>>>> +++ linux/kernel/power/main.c
> >>>>> @@ -292,11 +292,7 @@ static ssize_t state_store(struct kobjec
> >>>>>  #ifdef CONFIG_SUSPEND
> >>>>>  	for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) {
> >>>>>  		if (*s && len == strlen(*s) && !strncmp(buf, *s, len))
> >>>>> -			break;
> >>>>> -	}
> >>>>> -	if (state < PM_SUSPEND_MAX && *s) {
> >>>>> -		error = enter_state(state);
> >>>>> -		suspend_stats_update(error);
> >>>>> +			error = pm_suspend(state);
> >>>>
> >>>>
> >>>> This code will not stop after calling pm_suspend(). It will try more iterations
> >>>> in the for loop right?
> >>>
> >>> Well, only one string in pm_states[] can be matched at a time, but I agree that
> >>> it doesn't make sense to continue the loop after we've found a match.
> >>>
> >>>> We can probably keep the 'for' loop as it is, and just replace the 'if' block
> >>>> following the 'for' loop by: error = pm_suspend(state);
> >>>
> >>> I think the patch below is correct.
> >>>
> >>> Thanks,
> >>> Rafael
> >>>
> >>> ---
> >>> From: Rafael J. Wysocki <rjw@...k.pl>
> >>> Subject: PM / Sleep: Make enter_state() in kernel/power/suspend.c static
> >>>
> >>> The enter_state() function in kernel/power/suspend.c should be
> >>> static and state_store() in kernel/power/suspend.c should call
> >>> pm_suspend() instead of it, so make that happen (which also reduces
> >>> code duplication related to suspend statistics).
> >>>
> >>> Signed-off-by: Rafael J. Wysocki <rjw@...k.pl>
> >>> ---
> >>
> >>
> >> Yeah, this version of the patch will work fine.
> >>
> >> Acked-by: Srivatsa S. Bhat <srivatsa.bhat@...ux.vnet.ibm.com>
> > 
> > Thanks!
> > 
> > I wonder if that should be Reviewed-by, tough?  You evidently have reviewed
> > the patch (actually, all three of them).
> > 
> 
> 
> Anything is fine :-) It is not very clear to me when to use Reviewed-by and
> when to use Acked-by.. so I randomly chose one of them.. :-)
> But please enlighten me as to when to use which one, so that in the future, I
> can use the right one :-)

"Acked-by" means, roughly, "I have no objection" or "looks good to me",
depending on who's saying that, but it doesn't imply that you've had more than
a cursory look at the patch in question.  "Reviewied-by", in contrast, means
something like "I have reviewed the patch in detail and haven't found anything
wrong in it" (which obviously means that you have no objection too, because
otherwise you'd have found _something_ wrong in the patch).

So, "Acked-by" from anyone other than the relevant maintainer is just an
"I'm for" declaration, while "Reviewied-by" from _anyone_ carries some actual
weight. 

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/