Subject: Re: [RFCv2][PATCH] flexible array implementation
From: Dave Hansen <dave@linux.vnet.ibm.com>
To: Matt Helsley <matthltc@us.ibm.com>
Cc: vda.linux@googlemail.com, bblum@google.com, linux-kernel@vger.kernel.org,
       containers@lists.linux-foundation.org, menage@google.com,
       akpm@linux-foundation.org
In-Reply-To: <20090722183018.GE5878@count0.beaverton.ibm.com>
References: <20090721220017.60A219D3@kernel>
	 <20090722183018.GE5878@count0.beaverton.ibm.com>
Content-Type: text/plain
Date: Wed, 22 Jul 2009 15:03:26 -0700
Message-Id: <1248300206.24021.347.camel@nimitz>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1373
Lines: 35

On Wed, 2009-07-22 at 11:30 -0700, Matt Helsley wrote:
> > +static int __check_part_and_nr(struct flex_array *fa,
> > +                            int part_nr, int element_nr)
> > +{
> > +     if (part_nr >= __nr_part_ptrs() ||
> > +         element_nr > fa->nr_elements) {
> > +             WARN(1, "bad flexible array element number: %d > %d\n",
> > +                     element_nr, fa->nr_elements);
> > +             return -EINVAL;
> > +     }
> > +     return 0;
> > +}
> 
> Should the above be inline? Does it make sense to optimize the "common"
> case and penalize inappropriate access by putting an unlikely() in
> there? Or is it too early for this stuff?

I think I'll leave it to the compiler for now.  Since we also don't have
a single user, I don't think we have an idea how hot of a path this
might get used in.

> I wonder how the *, /, and % ops will affect things that otherwise
> would have been reduced to shifts and masks -- especially on the
> "smaller" embedded archs.

I'm generally fine with rounding all these sizes to powers-of-two.  But,
I do think it's a wee bit premature at this point.

-- Dave

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/