All of lore.kernel.org
 help / color / mirror / Atom feed
From: Avi Kivity <avi@qumranet.com>
To: Jerone Young <jyoung5@us.ibm.com>
Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org
Subject: Re: [PATCH 1 of 3] Remove use of bit fields in kvm trace structure
Date: Mon, 23 Jun 2008 02:33:13 +0000	[thread overview]
Message-ID: <485F0B69.4010909@qumranet.com> (raw)
In-Reply-To: <c2777036ad4d61259024.1213935553@thinkpadL>

Jerone Young wrote:
>
> This patch fixes kvmtrace use on big endian systems. When using bit fields the compiler will lay data out in the wrong order expected when laid down into a file. This fixes it by using one variable instead of using bit fields.
>
> Signed-off-by: Jerone Young <jyoung5@us.ibm.com>
>
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -311,9 +311,13 @@ struct kvm_s390_interrupt {
>  
>  /* This structure represents a single trace buffer record. */
>  struct kvm_trace_rec {
> -	__u32 event:28;
> -	__u32 extra_u32:3;
> -	__u32 cycle_in:1;
> +	/* variable rec_val
> + 	 * is split into:
> + 	 * bits 0 - 27  -> event id
> + 	 * bits 28 -30  -> number of extra data args of size u32
> + 	 * bits 31      -> binary indicator for if tsc is in record
> + 	 */
> +	__u32 rec_val;
>   

Please use #defines for shifts and masks instead of open-coding.  These 
#defines would be part of the public interface.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


WARNING: multiple messages have this Message-ID (diff)
From: Avi Kivity <avi@qumranet.com>
To: Jerone Young <jyoung5@us.ibm.com>
Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org
Subject: Re: [PATCH 1 of 3] Remove use of bit fields in kvm trace structure
Date: Mon, 23 Jun 2008 05:33:13 +0300	[thread overview]
Message-ID: <485F0B69.4010909@qumranet.com> (raw)
In-Reply-To: <c2777036ad4d61259024.1213935553@thinkpadL>

Jerone Young wrote:
>
> This patch fixes kvmtrace use on big endian systems. When using bit fields the compiler will lay data out in the wrong order expected when laid down into a file. This fixes it by using one variable instead of using bit fields.
>
> Signed-off-by: Jerone Young <jyoung5@us.ibm.com>
>
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -311,9 +311,13 @@ struct kvm_s390_interrupt {
>  
>  /* This structure represents a single trace buffer record. */
>  struct kvm_trace_rec {
> -	__u32 event:28;
> -	__u32 extra_u32:3;
> -	__u32 cycle_in:1;
> +	/* variable rec_val
> + 	 * is split into:
> + 	 * bits 0 - 27  -> event id
> + 	 * bits 28 -30  -> number of extra data args of size u32
> + 	 * bits 31      -> binary indicator for if tsc is in record
> + 	 */
> +	__u32 rec_val;
>   

Please use #defines for shifts and masks instead of open-coding.  These 
#defines would be part of the public interface.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


  parent reply	other threads:[~2008-06-23  2:33 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-20  4:19 [PATCH 0 of 3] Add ability for KVM TRACE to work on other archs Jerone Young
2008-06-20  4:19 ` Jerone Young
2008-06-20  4:19 ` [PATCH 1 of 3] Remove use of bit fields in kvm trace structure Jerone Young
2008-06-20  4:19   ` Jerone Young
2008-06-20 16:59   ` Hollis Blanchard
2008-06-20 16:59     ` Hollis Blanchard
2008-06-23  2:33   ` Avi Kivity [this message]
2008-06-23  2:33     ` Avi Kivity
2008-06-20  4:19 ` [PATCH 2 of 3] Move KVM TRACE DEFINITIONS to common header Jerone Young
2008-06-20  4:19   ` Jerone Young
2008-06-29 11:50   ` Avi Kivity
2008-06-29 11:50     ` Avi Kivity
2008-07-01 19:59     ` Hollis Blanchard
2008-07-01 19:59       ` Hollis Blanchard
2008-07-01 21:23       ` [PATCH 2 of 3] [v2] " Hollis Blanchard
2008-07-01 21:23         ` Hollis Blanchard
2008-07-05  9:35         ` Avi Kivity
2008-07-05  9:35           ` Avi Kivity
2008-06-20  4:19 ` [PATCH 3 of 3] Add new KVM TRACE events Jerone Young
2008-06-20  4:19   ` Jerone Young

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=485F0B69.4010909@qumranet.com \
    --to=avi@qumranet.com \
    --cc=jyoung5@us.ibm.com \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=kvm@vger.kernel.org \
    /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.