Xen devel - Oct 2012 - sedf/build: Fix build when using -fno-inline

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


--------------070906060006040201060901
Content-Type: text/x-patch; name="fix-no-inline-build.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="fix-no-inline-build.patch"

# 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);
 

--------------070906060006040201060901
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

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

--------------070906060006040201060901--

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



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

>>> 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

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

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)




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