----- Original Message -----
> From: "Stanislav Kholmanskikh" <stanislav.kholmansk...@oracle.com>
> To: "Jan Stancek" <jstan...@redhat.com>, "Cyril Hrubis" <chru...@suse.cz>
> Cc: "Dave Kleikamp" <dave.kleik...@oracle.com>, ltp-list@lists.sourceforge.net
> Sent: Tuesday, 27 August, 2013 12:57:46 PM
> Subject: Re: [LTP] [PATCH] ltp: handle missing online file
> 
> 
> On 08/21/2013 04:19 PM, Dave Kleikamp wrote:
> > On 08/21/2013 01:50 AM, Jan Stancek wrote:
> >>
> >>
> >>
> >> ----- Original Message -----
> >>> From: "Dave Kleikamp" <dave.kleik...@oracle.com>
> >>> To: ltp-list@lists.sourceforge.net
> >>> Sent: Tuesday, 20 August, 2013 10:34:39 PM
> >>> Subject: [LTP] [PATCH] ltp: handle missing online file
> >>>
> >>> gather_node_cpus() fails if the
> >>> /sys/devices/system/node/nodeX/cpuX/online
> >>> file is missing for any cpu other than cpu0. The absence of the online
> >>> file
> >>> should not be treated as a failure, but as an indication that the cpu is
> >>> not hot-pluggable and cannot be taken offline.
> >>>
> >>> Signed-off-by: Dave Kleikamp <dave.kleik...@oracle.com>
> >> Looks good to me. One comment inline.
> >>
> >> Reviewed-by: Jan Stancek <jstan...@redhat.com>
> >>
> >>> diff --git a/testcases/kernel/mem/lib/mem.c
> >>> b/testcases/kernel/mem/lib/mem.c
> >>> index 9bc926e..99243a6 100644
> >>> --- a/testcases/kernel/mem/lib/mem.c
> >>> +++ b/testcases/kernel/mem/lib/mem.c
> >>> @@ -786,16 +786,14 @@ static void gather_node_cpus(char *cpus, long nd)
> >>>                   if (path_exist(path, nd, i)) {
> >>                                       ^^  ^^
> >>                                       |---|--> Are these parameters used
> >>                                       |for anything?
> >>                                                Line above already does
> >>                                                snprintf.
> > They aren't. That's why I left them off the call I added below.
> >
> >>>                           snprintf(path1, BUFSIZ, "%s/online", path);
> >>>                           /*
> >>> -                  * No cpu0/online knob, as it can't support to
> >>> -                  * on/offline cpu0, so if the 'nd' node contains
> >>> -                  * cpu0, it should skip to check cpu0/online's value.
> >>> +                  * if there is no online knob, then the cpu cannot
> >>> +                  * be taken offline
> >>>                            */
> >>> -                 if (i == 0)
> >>> -                         goto next;
> >>> -                 SAFE_FILE_SCANF(cleanup, path1, "%ld", &online);
> >>> -                 if (online == 0)
> >>> -                         continue;
> >>> -next:
> >>> +                 if (path_exist(path1)) {
> >>> +                         SAFE_FILE_SCANF(cleanup, path1, "%ld", &online);
> >>> +                         if (online == 0)
> >>> +                                 continue;
> >>> +                 }
> >>>                           sprintf(buf, "%d,", i);
> >>>                           strcat(cpus, buf);
> >>>                   }
> >>>
> >>>
> Hi.
> 
> I'm an interested party :)
> 
> What should be modified to include this patch in upstream?

I think it can go as it is.

> 
> Just substitute:
> 
>    if (path_exist(path, nd, i)) {
> 
> to:
> 
>    if (path_exists(path)) {
> 
> Right?

We can do this as follow-up since it's not directly tied to submitted patch,
just something I noticed in original code.

Regards,
Jan

------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to