On Tue, Jun 8, 2010 at 10:46 AM, Anton Vorontsov <cbouatmai...@gmail.com> wrote:
> On Tue, Jun 08, 2010 at 10:02:49AM -0600, Grant Likely wrote:
>> On Tue, Jun 8, 2010 at 9:57 AM, Anton Vorontsov <cbouatmai...@gmail.com>
>> wrote:
>> > On Tue, Jun 08, 2010 at 08:26:43AM -0600, Grant Likely wrote:
>> > [...]
>> >> + dev = kzalloc(sizeof(*dev) + (sizeof(struct resource) * i),
>> >> GFP_KERNEL);
>> >> if (!dev)
>> >> return NULL;
>> >> -
>> >> dev->dev.of_node = of_node_get(np);
>> >> dev->dev.dma_mask = &dev->archdata.dma_mask;
>> >> dev->dev.parent = parent;
>> >> dev->dev.release = of_release_dev;
>> >>
>> >> + /* Populate the resource table */
>> >> + if (num_irq || num_reg) {
>> >> + dev->resource = (void*)&dev[1];
>> >
>> > This is ugly. Why not allocate the memory specifically for
>> > dev->resource? Is this because you plan to get rid of
>> > of_release_dev(), and the generic release_dev() won't
>> > know if it should free the dev->resource? There must
>> > be a better way to handle this.
>>
>> Allocating in one big block means less potential memory fragmentation,
>> and the kernel can free it all at once.
>
> Are there any numbers of saved amount of memory so that we
> could compare?
>
> The "less memory fragmentation" is indeed potential, but
> introduction of obscure code is going on now at this precise
> moment.
It's not obscure. It's smaller and simpler code with fewer error paths.
>> This is a common pattern.
>
> This can't be true because it produces ugly casts and fragile
> code all over the place -- which is exactly what everybody
> tries to avoid in the kernel.
Fragile? How? &var[1] *always* gives you a pointer to the first
address after a structure. If the structure changes, then so does the
offset. Heck, if the type of 'var' changes, then the offset changes
in kind too. If anything, I should have also used sizeof(*res) in the
kzalloc call so that the allocated size is protected against a type
change to 'res' too.
If you prefer, I can move the dev->resource assignment to immediately
after the kzalloc to keep everything contained within 4 lines of each
other.
> Such a pattern is common when a driver allocates e.g. tx and rx
> buffers (of the same type) together, and then split the buffer
> into two pointers.
tx & rx buffers are different because they must be DMAable. That
imposes alignment requirements with kzalloc() guarantees.
> But I heard of no such pattern for 'struct device + struct
> resources' allocation without even some kind of _priv struct,
> which is surely something new, and ugly.
git grep '\*).*&[a-z1-9_]*\[1\]'
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
