All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
To: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>,
	linux-kernel@vger.kernel.org, srikar@linux.vnet.ibm.com,
	yarygin@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: Wed, 28 Oct 2015 23:47:19 +0300	[thread overview]
Message-ID: <8737wuk9g6.fsf@linux.vnet.ibm.com> (raw)
In-Reply-To: <561639B8.8040407@linux.vnet.ibm.com>


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.

It's not a real problem anyway :)

For s390 parts:
Acked-by: Alexander Yarygin <yarygin@linux.vnet.ibm.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
To: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>,
	linux-kernel@vger.kernel.org, srikar@linux.vnet.ibm.com,
	yarygin@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: Wed, 28 Oct 2015 23:47:19 +0300	[thread overview]
Message-ID: <8737wuk9g6.fsf@linux.vnet.ibm.com> (raw)
In-Reply-To: <561639B8.8040407@linux.vnet.ibm.com>


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.

It's not a real problem anyway :)

For s390 parts:
Acked-by: Alexander Yarygin <yarygin@linux.vnet.ibm.com>

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


  reply	other threads:[~2015-10-28 20:47 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     ` Alexander Yarygin [this message]
2015-10-28 20:47       ` Alexander Yarygin
2015-10-29  7:07       ` [PATCH v9 1/4] perf, kvm/{x86, s390}: " Hemant Kumar
2015-10-29  7:07         ` [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
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=8737wuk9g6.fsf@linux.vnet.ibm.com \
    --to=yarygin@linux.vnet.ibm.com \
    --cc=acme@kernel.org \
    --cc=borntraeger@de.ibm.com \
    --cc=dsahern@gmail.com \
    --cc=hemant@linux.vnet.ibm.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 \
    /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.