* [PATCH 1/2] xen: correct error handling in xen-acpi-processor [not found] <20170331111948.17539-1-jgross@suse.com> @ 2017-03-31 11:19 ` Juergen Gross 2017-03-31 11:19 ` [PATCH 2/2] xen: make functions in xen-acpi-processor return void Juergen Gross ` (2 subsequent siblings) 3 siblings, 0 replies; 5+ messages in thread From: Juergen Gross @ 2017-03-31 11:19 UTC (permalink / raw) To: linux-kernel, xen-devel; +Cc: Juergen Gross, boris.ostrovsky, stable Error handling in upload_pm_data() is rather questionable: possible errno values from called functions are or'ed together resulting in strange return values: -EINVAL and -ENOMEM would e.g. result in returning -EXDEV. Fix this by bailing out early after the first error. Cc: stable@vger.kernel.org Signed-off-by: Juergen Gross <jgross@suse.com> --- drivers/xen/xen-acpi-processor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c index 23e391d..3659ed3 100644 --- a/drivers/xen/xen-acpi-processor.c +++ b/drivers/xen/xen-acpi-processor.c @@ -283,8 +283,8 @@ static int upload_pm_data(struct acpi_processor *_pr) if (_pr->flags.power) err = push_cxx_to_hypervisor(_pr); - if (_pr->performance && _pr->performance->states) - err |= push_pxx_to_hypervisor(_pr); + if (!err && _pr->performance && _pr->performance->states) + err = push_pxx_to_hypervisor(_pr); mutex_unlock(&acpi_ids_mutex); return err; -- 2.10.2 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] xen: make functions in xen-acpi-processor return void [not found] <20170331111948.17539-1-jgross@suse.com> 2017-03-31 11:19 ` [PATCH 1/2] xen: correct error handling in xen-acpi-processor Juergen Gross @ 2017-03-31 11:19 ` Juergen Gross [not found] ` <20170331111948.17539-2-jgross@suse.com> [not found] ` <20170331111948.17539-3-jgross@suse.com> 3 siblings, 0 replies; 5+ messages in thread From: Juergen Gross @ 2017-03-31 11:19 UTC (permalink / raw) To: linux-kernel, xen-devel; +Cc: Juergen Gross, boris.ostrovsky The return value of xen_copy_pct_data() is always 0 and never checked. xen_copy_psd_data() returns always 0, too, so checking the value is kind of pointless. Make the functions return void. Signed-off-by: Juergen Gross <jgross@suse.com> --- drivers/xen/xen-acpi-processor.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c index 3659ed3..8f01585 100644 --- a/drivers/xen/xen-acpi-processor.c +++ b/drivers/xen/xen-acpi-processor.c @@ -161,8 +161,8 @@ xen_copy_pss_data(struct acpi_processor *_pr, } return dst_states; } -static int xen_copy_psd_data(struct acpi_processor *_pr, - struct xen_processor_performance *dst) +static void xen_copy_psd_data(struct acpi_processor *_pr, + struct xen_processor_performance *dst) { struct acpi_psd_package *pdomain; @@ -189,10 +189,9 @@ static int xen_copy_psd_data(struct acpi_processor *_pr, } memcpy(&(dst->domain_info), pdomain, sizeof(struct acpi_psd_package)); - return 0; } -static int xen_copy_pct_data(struct acpi_pct_register *pct, - struct xen_pct_register *dst_pct) +static void xen_copy_pct_data(struct acpi_pct_register *pct, + struct xen_pct_register *dst_pct) { /* It would be nice if you could just do 'memcpy(pct, dst_pct') but * sadly the Xen structure did not have the proper padding so the @@ -205,7 +204,6 @@ static int xen_copy_pct_data(struct acpi_pct_register *pct, dst_pct->bit_offset = pct->bit_offset; dst_pct->reserved = pct->reserved; dst_pct->address = pct->address; - return 0; } static int push_pxx_to_hypervisor(struct acpi_processor *_pr) { @@ -233,8 +231,8 @@ static int push_pxx_to_hypervisor(struct acpi_processor *_pr) set_xen_guest_handle(dst_perf->states, dst_states); dst_perf->flags |= XEN_PX_PSS; } - if (!xen_copy_psd_data(_pr, dst_perf)) - dst_perf->flags |= XEN_PX_PSD; + xen_copy_psd_data(_pr, dst_perf); + dst_perf->flags |= XEN_PX_PSD; if (dst_perf->flags != (XEN_PX_PSD | XEN_PX_PSS | XEN_PX_PCT | XEN_PX_PPC)) { pr_warn("ACPI CPU%u missing some P-state data (%x), skipping\n", -- 2.10.2 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
[parent not found: <20170331111948.17539-2-jgross@suse.com>]
* Re: [PATCH 1/2] xen: correct error handling in xen-acpi-processor [not found] ` <20170331111948.17539-2-jgross@suse.com> @ 2017-03-31 14:10 ` Boris Ostrovsky [not found] ` <56430503-ab15-4ff1-9432-f8c23e8981eb@oracle.com> 1 sibling, 0 replies; 5+ messages in thread From: Boris Ostrovsky @ 2017-03-31 14:10 UTC (permalink / raw) To: Juergen Gross, linux-kernel, xen-devel; +Cc: stable On 03/31/2017 07:19 AM, Juergen Gross wrote: > Error handling in upload_pm_data() is rather questionable: possible > errno values from called functions are or'ed together resulting in > strange return values: -EINVAL and -ENOMEM would e.g. result in > returning -EXDEV. And it doesn't matter anyway since noone checks return value. (not that OR-ing errors makes much sense) > > Fix this by bailing out early after the first error. I am not sure about this: why should we not try to load P states if C states failed to load? -boris > > Cc: stable@vger.kernel.org > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > drivers/xen/xen-acpi-processor.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c > index 23e391d..3659ed3 100644 > --- a/drivers/xen/xen-acpi-processor.c > +++ b/drivers/xen/xen-acpi-processor.c > @@ -283,8 +283,8 @@ static int upload_pm_data(struct acpi_processor *_pr) > if (_pr->flags.power) > err = push_cxx_to_hypervisor(_pr); > > - if (_pr->performance && _pr->performance->states) > - err |= push_pxx_to_hypervisor(_pr); > + if (!err && _pr->performance && _pr->performance->states) > + err = push_pxx_to_hypervisor(_pr); > > mutex_unlock(&acpi_ids_mutex); > return err; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <56430503-ab15-4ff1-9432-f8c23e8981eb@oracle.com>]
* Re: [PATCH 1/2] xen: correct error handling in xen-acpi-processor [not found] ` <56430503-ab15-4ff1-9432-f8c23e8981eb@oracle.com> @ 2017-03-31 14:19 ` Juergen Gross 0 siblings, 0 replies; 5+ messages in thread From: Juergen Gross @ 2017-03-31 14:19 UTC (permalink / raw) To: Boris Ostrovsky, linux-kernel, xen-devel; +Cc: stable On 31/03/17 16:10, Boris Ostrovsky wrote: > On 03/31/2017 07:19 AM, Juergen Gross wrote: >> Error handling in upload_pm_data() is rather questionable: possible >> errno values from called functions are or'ed together resulting in >> strange return values: -EINVAL and -ENOMEM would e.g. result in >> returning -EXDEV. > > And it doesn't matter anyway since noone checks return value. (not that > OR-ing errors makes much sense) Ha, you are right! So I can just fold the patches into one and turn some more functions into retuning void. Juergen > >> >> Fix this by bailing out early after the first error. > > I am not sure about this: why should we not try to load P states if C > states failed to load? > > -boris > >> >> Cc: stable@vger.kernel.org >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> drivers/xen/xen-acpi-processor.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c >> index 23e391d..3659ed3 100644 >> --- a/drivers/xen/xen-acpi-processor.c >> +++ b/drivers/xen/xen-acpi-processor.c >> @@ -283,8 +283,8 @@ static int upload_pm_data(struct acpi_processor *_pr) >> if (_pr->flags.power) >> err = push_cxx_to_hypervisor(_pr); >> >> - if (_pr->performance && _pr->performance->states) >> - err |= push_pxx_to_hypervisor(_pr); >> + if (!err && _pr->performance && _pr->performance->states) >> + err = push_pxx_to_hypervisor(_pr); >> >> mutex_unlock(&acpi_ids_mutex); >> return err; > > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <20170331111948.17539-3-jgross@suse.com>]
* Re: [PATCH 2/2] xen: make functions in xen-acpi-processor return void [not found] ` <20170331111948.17539-3-jgross@suse.com> @ 2017-03-31 14:13 ` Boris Ostrovsky 0 siblings, 0 replies; 5+ messages in thread From: Boris Ostrovsky @ 2017-03-31 14:13 UTC (permalink / raw) To: Juergen Gross, linux-kernel, xen-devel On 03/31/2017 07:19 AM, Juergen Gross wrote: > The return value of xen_copy_pct_data() is always 0 and never checked. > xen_copy_psd_data() returns always 0, too, so checking the value is > kind of pointless. > > Make the functions return void. > > Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-31 14:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20170331111948.17539-1-jgross@suse.com>
2017-03-31 11:19 ` [PATCH 1/2] xen: correct error handling in xen-acpi-processor Juergen Gross
2017-03-31 11:19 ` [PATCH 2/2] xen: make functions in xen-acpi-processor return void Juergen Gross
[not found] ` <20170331111948.17539-2-jgross@suse.com>
2017-03-31 14:10 ` [PATCH 1/2] xen: correct error handling in xen-acpi-processor Boris Ostrovsky
[not found] ` <56430503-ab15-4ff1-9432-f8c23e8981eb@oracle.com>
2017-03-31 14:19 ` Juergen Gross
[not found] ` <20170331111948.17539-3-jgross@suse.com>
2017-03-31 14:13 ` [PATCH 2/2] xen: make functions in xen-acpi-processor return void Boris Ostrovsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).