Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1193506

Re: [RFC v2] genalloc:add an gen_pool_first_fit_align algo to genalloc

From Scott Wood <scottwood@freescale.com>
Newsgroups linux.kernel
Subject Re: [RFC v2] genalloc:add an gen_pool_first_fit_align algo to genalloc
Date 2015-07-27 23:30 +0200
Message-ID <pQY8b-5DK-23@gated-at.bofh.it> (permalink)
References <pQNFL-729-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, 2015-07-27 at 17:57 +0800, Zhao Qiang wrote:
> diff --git a/lib/genalloc.c b/lib/genalloc.c
> index d214866..e6608cd 100644
> --- a/lib/genalloc.c
> +++ b/lib/genalloc.c
> @@ -509,6 +509,31 @@ unsigned long gen_pool_first_fit(unsigned long *map, 
> unsigned long size,
>  EXPORT_SYMBOL(gen_pool_first_fit);
>  
>  /**
> + * gen_pool_first_fit_align - find the first available region
> + * of memory matching the size requirement (no alignment constraint)
> + * @map: The address to base the search on
> + * @size: The bitmap size in bits
> + * @start: The bitnumber to start searching at
> + * @nr: The number of zeroed bits we're looking for
> + * @data: additional data - unused
> + */
> +unsigned long gen_pool_first_fit_align(unsigned long *map, unsigned long 
> size,
> +             unsigned long start, unsigned int nr, void *data)
> +{
> +     unsigned long align_mask;
> +     int order;
> +
> +     if (data && data->pool) {

There is no way that this compiles.  You can't dereference a void pointer.

Please test your code before submitting, even for an RFC.

> +             order = data->pool->min_alloc_order;

I don't think pool belongs in data.  It's fundamental enough that, if a 
pointer to pool is needed, it should be an argument to the algorithm.

> +             align_mask = ((data->align + (1UL << order) - 1) >> order) - 1;
> +     } else {
> +             pr_err("no data or data->pool\n");
> +     }

This is way too vague and unobtrusive of an error message, and also not rate-
limited, etc.  I wouldn't bother checking at all.  Just let it crash on the 
developer's machine if they use this without passing in data.

Where's the part that adds the ability to pass in data to each allocation 
call, as per the previous discussion?

-Scott

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

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[RFC v2] genalloc:add an gen_pool_first_fit_align algo to genalloc Zhao Qiang <qiang.zhao@freescale.com> - 2015-07-27 12:20 +0200
  Re: [RFC v2] genalloc:add an gen_pool_first_fit_align algo to  genalloc Scott Wood <scottwood@freescale.com> - 2015-07-27 23:30 +0200
    RE: [RFC v2] genalloc:add an gen_pool_first_fit_align algo to  genalloc Zhao Qiang <qiang.zhao@freescale.com> - 2015-07-28 07:40 +0200
      RE: [RFC v2] genalloc:add an gen_pool_first_fit_align algo to  genalloc Zhao Qiang <qiang.zhao@freescale.com> - 2015-07-30 04:10 +0200

csiph-web