Re: [PATCH] bitmap: fix bitmap_find_free_region()

[Johannes Weiner]

On Sat, Dec 20, 2008 at 12:19:46AM +0100, Guennadi Liakhovetski wrote:
> Well, I don't quite understand. Doesn't bitmap_find_free_region() _have_ 
> to return an error if the request cannot be satisfied? Isn't it what it 
> does if after scanning the bitmap no suitable free region is found?

There is a difference here.  In the case it already checks, the
request was sane but couldn't be satisfied due to prior requests.
This is an expected situation on the bitmap allocator level.

The check you want to add is for fixing up the caller.

This API is especially clear on that separation because the caller
even has to pass in the size of the bitmap, so itself should ensure
that the combination of bitmap size and request size is sane.

> What's the difference? Why don't we want to fix _that_ function? It
> also has other users - currently I see 3 of them in the kernel:
> 
> mm/vmalloc.c

This has its own sanity checks.  It's a bug for the user of vb_alloc()
to pass a size that is larger than VMAP_MAX_ALLOC pages and the bitmap
has a minimum of VMAP_MAX_ALLOC * 2 bits.

> arch/powerpc/sysdev/msi_bitmap.c (5 occurrences)

The users allocating irqs from there seem all sane and doing only low
orders.  Not yet quite through, though.

> arch/sh/kernel/cpu/sh4/sq.c

Not yet looked into it.

> Yes, I understand that the caller _can_ verify whether the requested 
> region at all fits into the bitmap, but from the PoV of offering a 
> consistent API this seems strange.

Not really.  The user that is supposed to use the bitmap also sets it
up.  If the requests are bogus wrt the original parameters it's the
users fault, not that of the API.

> Thanks
> Guennadi

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