sedf/build: Fix build when using -fno-inline

sedf/build: Fix build when using -fno-inline

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 264 bytes --]

I found this issue while trying to debug on a separate issue.  It
certainly affects unstable thru 4.1, and probably earlier, so should be
take for backport.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix-no-inline-build.patch --]
[-- Type: text/x-patch; name="fix-no-inline-build.patch", Size: 1165 bytes --]

# HG changeset patch
# Parent 5fbdbf585f5f2ee9a3e3c75a8a9f9f2cc6eda65c
build: Fix build when using -fno-inline

struct task_slice.migrated is not initialised by this function, and
subsequently returned by value, leading to the error:

sched_sedf.c: In function ‘sedf_do_extra_schedule’:
sched_sedf.c:711: error: ‘ret.migrated’ may be used uninitialised in
this function

for both gcc 4.1.2 and 4.4.3 (which are the two I have easily to hand)
when combined with the -fno-inline compile option.

Signed-off-by Andrew Cooper <andrew.cooper3@citrix.com>

--
This is compile tested only, but given that the sole caller of
sedf_do_extra_schedule() unconditionally sets migrated to 0, I am fairly
confident of the correctness of the fix.

diff -r 5fbdbf585f5f xen/common/sched_sedf.c
--- a/xen/common/sched_sedf.c
+++ b/xen/common/sched_sedf.c
@@ -667,7 +667,7 @@ static void desched_extra_dom(s_time_t n
 static struct task_slice sedf_do_extra_schedule(
     s_time_t now, s_time_t end_xt, struct list_head *extraq[], int cpu)
 {
-    struct task_slice   ret;
+    struct task_slice   ret = { 0 };
     struct sedf_vcpu_info *runinf;
     ASSERT(end_xt > now);
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel V2] sedf/build: Fix build when using -fno-inline

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 386 bytes --]

On 03/10/12 13:35, Andrew Cooper wrote:
> I found this issue while trying to debug on a separate issue.  It
> certainly affects unstable thru 4.1, and probably earlier, so should be
> take for backport.
>

Apologies - try this patch which has less Unicode in the commit message.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com


[-- Attachment #2: fix-no-inline-build.patch --]
[-- Type: text/x-patch, Size: 1215 bytes --]

# HG changeset patch
# Parent 5fbdbf585f5f2ee9a3e3c75a8a9f9f2cc6eda65c
sedf/build: Fix build when using -fno-inline

struct task_slice.migrated is not initialised by this function, and
subsequently returned by value, leading to the error:

sched_sedf.c: In function 'sedf_do_extra_schedule':
sched_sedf.c:711: error: 'ret.migrated' may be used uninitialised in
this function

for both gcc 4.1.2 and 4.4.3 (which are the two I have easily to hand)
when combined with the -fno-inline compile option.

Signed-off-by Andrew Cooper <andrew.cooper3@citrix.com>

--
This is compile tested only, but given that the sole caller of
sedf_do_extra_schedule() unconditionally sets migrated to 0, I am fairly
confident of the correctness of the fix.

Changes since v1:
  - Fix unicode issue in comment.

diff -r 5fbdbf585f5f xen/common/sched_sedf.c
--- a/xen/common/sched_sedf.c
+++ b/xen/common/sched_sedf.c
@@ -667,7 +667,7 @@ static void desched_extra_dom(s_time_t n
 static struct task_slice sedf_do_extra_schedule(
     s_time_t now, s_time_t end_xt, struct list_head *extraq[], int cpu)
 {
-    struct task_slice   ret;
+    struct task_slice   ret = { 0 };
     struct sedf_vcpu_info *runinf;
     ASSERT(end_xt > now);
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel V2] sedf/build: Fix build when using -fno-inline

[Jan Beulich]

>>> Andrew Cooper <andrew.cooper3@citrix.com> 10/03/12 2:38 PM >>>
>On 03/10/12 13:35, Andrew Cooper wrote:
>> I found this issue while trying to debug on a separate issue.  It
>> certainly affects unstable thru 4.1, and probably earlier, so should be
>> take for backport.
>>
>
>Apologies - try this patch which has less Unicode in the commit message.

I certainly agree to the change for -unstable, but what's the rationale for
the backport request (given that there's no -fno-inline anywhere in the tree)?
Is this actively causing problems in any non-debugging environment?

Jan

Re: [Xen-devel V2] sedf/build: Fix build when using -fno-inline

[Andrew Cooper]

On 03/10/12 14:22, Jan Beulich wrote:
>>>> Andrew Cooper <andrew.cooper3@citrix.com> 10/03/12 2:38 PM >>>
>> On 03/10/12 13:35, Andrew Cooper wrote:
>>> I found this issue while trying to debug on a separate issue.  It
>>> certainly affects unstable thru 4.1, and probably earlier, so should be
>>> take for backport.
>>>
>> Apologies - try this patch which has less Unicode in the commit message.
> I certainly agree to the change for -unstable, but what's the rationale for
> the backport request (given that there's no -fno-inline anywhere in the tree)?
> Is this actively causing problems in any non-debugging environment?
>
> Jan
>

Not as far as I am aware.  Backporting it will make no difference to the
older trees in general, but will prevent people who are debugging older
trees from needing to fix the build every time they actually need to
invoke -fno-inline.  (I have been working on this issue for 4 straight
days now, debugging on 4.1 and was quite taken aback when the build broke).

I would argue that the benefits (for developers) do outweigh the
bascially-0 cost, even if it isn't strictly a functional bugfix.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com

Re: [Xen-devel V2] sedf/build: Fix build when using -fno-inline

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1317 bytes --]

On Wed, 2012-10-03 at 13:38 +0100, Andrew Cooper wrote:
> # HG changeset patch
> # Parent 5fbdbf585f5f2ee9a3e3c75a8a9f9f2cc6eda65c
> sedf/build: Fix build when using -fno-inline
> 
> struct task_slice.migrated is not initialised by this function, and
> subsequently returned by value, leading to the error:
> 
> sched_sedf.c: In function 'sedf_do_extra_schedule':
> sched_sedf.c:711: error: 'ret.migrated' may be used uninitialised in
> this function
> 
> for both gcc 4.1.2 and 4.4.3 (which are the two I have easily to hand)
> when combined with the -fno-inline compile option.
> 
> Signed-off-by Andrew Cooper <andrew.cooper3@citrix.com>
> 
Acked-by: Dario Faggioli <dario.faggioli@citrix.com>

> --
> This is compile tested only, but given that the sole caller of
> sedf_do_extra_schedule() unconditionally sets migrated to 0, I am
> fairly
> confident of the correctness of the fix.
> 
Agreed, sedf does not include any task migration mechanism, so 0 is all
you can have there. :-)

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel