From mboxrd@z Thu Jan  1 00:00:00 1970
From: Takuya Yoshikawa <takuya.yoshikawa@gmail.com>
Subject: Re: [PATCH -v3] KVM: x86: lapic: Clean up find_highest_vector() and
 count_vectors()
Date: Fri, 31 Aug 2012 01:09:56 +0900
Message-ID: <20120831010956.000856ead3c6f96dbe2bec46@gmail.com>
References: <20120828185756.c754f4b4.yoshikawa.takuya@oss.ntt.co.jp>
	<20120829191057.GA10834@amt.cnet>
	<20120829225120.GA9146@redhat.com>
	<20120830100906.5b4bcffd.yoshikawa.takuya@oss.ntt.co.jp>
	<20120830063702.GB9286@redhat.com>
	<20120830185052.086cbb0f.yoshikawa.takuya@oss.ntt.co.jp>
	<20120830101033.GA19622@redhat.com>
	<20120830192439.3e088595.yoshikawa.takuya@oss.ntt.co.jp>
	<20120830104458.GA19997@redhat.com>
	<20120830213019.82a822da.yoshikawa.takuya@oss.ntt.co.jp>
	<20120830132131.GB21132@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>,
	Marcelo Tosatti <mtosatti@redhat.com>, avi@redhat.com,
	kvm@vger.kernel.org
To: "Michael S. Tsirkin" <mst@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-pz0-f46.google.com ([209.85.210.46]:63501 "EHLO
	mail-pz0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753657Ab2H3QKB (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 30 Aug 2012 12:10:01 -0400
Received: by dady13 with SMTP id y13so1313156dad.19
        for <kvm@vger.kernel.org>; Thu, 30 Aug 2012 09:10:01 -0700 (PDT)
In-Reply-To: <20120830132131.GB21132@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Thu, 30 Aug 2012 16:21:31 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:
 
> > +static u32 apic_read_reg(int reg_off, void *bitmap)
> > +{
> > +	return *((u32 *)(bitmap + reg_off));
> > +}
> > +
> 
> Contrast with apic_set_reg which gets apic,
> add fact that all callers invoke REG_POS and you will
> see this is a bad API.
> 
> I played with some APIs but in the end it's
> probably better to just open-code this.

I don't mind open-coding this.

> As a bonus, open-coding will avoid the need
> for cast above, which is good: casts make code more
> fragile.

But I still don't understand why we can eliminate casting:

  u32 reg_val;

  reg_val = *((u32 *)(bitmap + REG_POS(vec)));
  if (reg_val)
      return __fls(reg_val) + vec;

(I'm not sure compilers are allowed to push out the value and
do multiple references for this code as explained in
https://lwn.net/Articles/508991/ )


If you mean

  u32 *reg;

  reg = bitmap + REG_POS(vec);
  if (*reg)
      return __fls(*reg) + vec;

I'm still not confident if this is a good style.
I rarely see code doing

  if (*p)
      __fls(*p);

This looks like explicite multiple references: I'm not saying
this will actually be compiled to do multiple references.

Thanks,
	Takuya