From: Hemant Kumar <hemant@linux.vnet.ibm.com>
To: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>,
linux-kernel@vger.kernel.org, srikar@linux.vnet.ibm.com,
acme@kernel.org, borntraeger@de.ibm.com, mingo@redhat.com,
paulus@samba.org, scottwood@freescale.com,
naveen.n.rao@linux.vnet.ibm.com, sukadev@linux.vnet.ibm.com,
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v9 1/4] perf, kvm/{x86, s390}: Remove dependency on uapi/kvm_perf.h
Date: Thu, 29 Oct 2015 12:37:18 +0530 [thread overview]
Message-ID: <5631C5A6.4040101@linux.vnet.ibm.com> (raw)
In-Reply-To: <8737wuk9g6.fsf@linux.vnet.ibm.com>
On 10/29/2015 02:17 AM, Alexander Yarygin wrote:
> Hemant Kumar writes:
>
>> Hi David,
>>
>>
>> On 10/07/2015 09:41 PM, David Ahern wrote:
>>> On 10/6/15 8:25 PM, Hemant Kumar wrote:
>>>> @@ -358,7 +357,12 @@ static bool handle_end_event(struct
>>>> perf_kvm_stat *kvm,
>>>> time_diff = sample->time - time_begin;
>>>>
>>>> if (kvm->duration && time_diff > kvm->duration) {
>>>> - char decode[DECODE_STR_LEN];
>>>> + char *decode = zalloc(decode_str_len);
>>> decode can still be a stack variable even with variable length.
>>>
>> Yeah, we can do that. But, I am not sure whether its a standard way.
>>
> Well, I also vote for making them variable length arrays. I guess that
> wouldn't be a problem because the "variable" here is actually a constant
> compile time value, even if it's extern.
>
> But if people are strongly against it, as an alternative I can suggest
> to move the 'char *decode' variable to the perf_kvm_stat structure,
> allocate it once e.g. in kvm_events_report() and just write to it via
> decode_key(). If I'm not mistaken, we always write \0 trimmed strings,
> so garbage after \0 shouldn't be a problem.
I agree. We can do that. But, since this is a small change (making
the variable a constant compile time value rather than the array being
a run time value), we can do that subsequently. For now, we can go
with the current patchset.
> It's not a real problem anyway :)
Yeah.
> For s390 parts:
> Acked-by: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Thanks a lot for testing and acking it. :)
>>> -----8<-----
>>>
>>>> @@ -575,7 +581,7 @@ static void show_timeofday(void)
>>>>
>>>> static void print_result(struct perf_kvm_stat *kvm)
>>>> {
>>>> - char decode[DECODE_STR_LEN];
>>>> + char *decode;
>>> and a stack variable here too.
>>>
>> Same here.
>>
>>> David
>>> _______________________________________________
>>> Linuxppc-dev mailing list
>>> Linuxppc-dev@lists.ozlabs.org
>>> https://lists.ozlabs.org/listinfo/linuxppc-dev
--
Thanks,
Hemant Kumar
WARNING: multiple messages have this Message-ID (diff)
From: Hemant Kumar <hemant@linux.vnet.ibm.com>
To: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>,
linux-kernel@vger.kernel.org, srikar@linux.vnet.ibm.com,
acme@kernel.org, borntraeger@de.ibm.com, mingo@redhat.com,
paulus@samba.org, scottwood@freescale.com,
naveen.n.rao@linux.vnet.ibm.com, sukadev@linux.vnet.ibm.com,
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v9 1/4] perf,kvm/{x86,s390}: Remove dependency on uapi/kvm_perf.h
Date: Thu, 29 Oct 2015 12:37:18 +0530 [thread overview]
Message-ID: <5631C5A6.4040101@linux.vnet.ibm.com> (raw)
In-Reply-To: <8737wuk9g6.fsf@linux.vnet.ibm.com>
On 10/29/2015 02:17 AM, Alexander Yarygin wrote:
> Hemant Kumar writes:
>
>> Hi David,
>>
>>
>> On 10/07/2015 09:41 PM, David Ahern wrote:
>>> On 10/6/15 8:25 PM, Hemant Kumar wrote:
>>>> @@ -358,7 +357,12 @@ static bool handle_end_event(struct
>>>> perf_kvm_stat *kvm,
>>>> time_diff = sample->time - time_begin;
>>>>
>>>> if (kvm->duration && time_diff > kvm->duration) {
>>>> - char decode[DECODE_STR_LEN];
>>>> + char *decode = zalloc(decode_str_len);
>>> decode can still be a stack variable even with variable length.
>>>
>> Yeah, we can do that. But, I am not sure whether its a standard way.
>>
> Well, I also vote for making them variable length arrays. I guess that
> wouldn't be a problem because the "variable" here is actually a constant
> compile time value, even if it's extern.
>
> But if people are strongly against it, as an alternative I can suggest
> to move the 'char *decode' variable to the perf_kvm_stat structure,
> allocate it once e.g. in kvm_events_report() and just write to it via
> decode_key(). If I'm not mistaken, we always write \0 trimmed strings,
> so garbage after \0 shouldn't be a problem.
I agree. We can do that. But, since this is a small change (making
the variable a constant compile time value rather than the array being
a run time value), we can do that subsequently. For now, we can go
with the current patchset.
> It's not a real problem anyway :)
Yeah.
> For s390 parts:
> Acked-by: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Thanks a lot for testing and acking it. :)
>>> -----8<-----
>>>
>>>> @@ -575,7 +581,7 @@ static void show_timeofday(void)
>>>>
>>>> static void print_result(struct perf_kvm_stat *kvm)
>>>> {
>>>> - char decode[DECODE_STR_LEN];
>>>> + char *decode;
>>> and a stack variable here too.
>>>
>> Same here.
>>
>>> David
>>> _______________________________________________
>>> Linuxppc-dev mailing list
>>> Linuxppc-dev@lists.ozlabs.org
>>> https://lists.ozlabs.org/listinfo/linuxppc-dev
--
Thanks,
Hemant Kumar
next prev parent reply other threads:[~2015-10-29 7:07 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-07 2:25 [PATCH v9 1/4] perf, kvm/{x86, s390}: Remove dependency on uapi/kvm_perf.h Hemant Kumar
2015-10-07 2:25 ` [PATCH v9 1/4] perf,kvm/{x86,s390}: " Hemant Kumar
2015-10-07 2:25 ` [PATCH v9 2/4] perf,kvm/{x86,s390}: Remove const from kvm_events_tp Hemant Kumar
2015-10-07 2:25 ` [PATCH v9 3/4] perf,kvm/powerpc: Port perf kvm stat to powerpc Hemant Kumar
2015-10-07 2:25 ` [PATCH v9 4/4] perf,kvm/powerpc: Add support for HCALL reasons Hemant Kumar
2015-10-07 16:11 ` [PATCH v9 1/4] perf,kvm/{x86,s390}: Remove dependency on uapi/kvm_perf.h David Ahern
2015-10-08 9:39 ` [PATCH v9 1/4] perf, kvm/{x86, s390}: " Hemant Kumar
2015-10-08 9:39 ` [PATCH v9 1/4] perf,kvm/{x86,s390}: " Hemant Kumar
2015-10-28 20:47 ` [PATCH v9 1/4] perf, kvm/{x86, s390}: " Alexander Yarygin
2015-10-28 20:47 ` [PATCH v9 1/4] perf,kvm/{x86,s390}: " Alexander Yarygin
2015-10-29 7:07 ` Hemant Kumar [this message]
2015-10-29 7:07 ` Hemant Kumar
2015-10-26 8:26 ` [PATCH v9 1/4] perf, kvm/{x86, s390}: " Hemant Kumar
2015-10-26 8:26 ` [PATCH v9 1/4] perf,kvm/{x86,s390}: " Hemant Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5631C5A6.4040101@linux.vnet.ibm.com \
--to=hemant@linux.vnet.ibm.com \
--cc=acme@kernel.org \
--cc=borntraeger@de.ibm.com \
--cc=dsahern@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mingo@redhat.com \
--cc=naveen.n.rao@linux.vnet.ibm.com \
--cc=paulus@samba.org \
--cc=scottwood@freescale.com \
--cc=srikar@linux.vnet.ibm.com \
--cc=sukadev@linux.vnet.ibm.com \
--cc=yarygin@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.