Re: Is this code right in zram?

From: Jerome Marchand
Date: Mon Jun 04 2012 - 08:08:35 EST


On 05/31/2012 11:43 PM, John Moser wrote:
> before I go stomping all over other peoples' work and sending idiotic
> patches, I think I'll ask. Since I have no clue what I'm doing.
>
> in drivers/staging/zram.c out of 3.4 (I just grabbed the source hours
> ago), I see this on lines 810-822:

I assume that's drivers/staging/zram_drv.c.

>
> /* Allocate the device array and initialize each one */
> pr_info("Creating %u devices ...\n", num_devices);
> zram_devices = kzalloc(num_devices * sizeof(struct zram),
> GFP_KERNEL);
> if (!zram_devices) {
> ret = -ENOMEM;
> goto unregister;
> }
>
> for (dev_id = 0; dev_id < num_devices; dev_id++) {
> ret = create_device(&zram_devices[dev_id], dev_id);
> if (ret)
> goto free_devices;
> }
>
> Curiosity got me to here:
>
> http://lwn.net/Articles/147014/
>
> So assuming this, what I see here is:
>
> - kmalloc(num_devices * sizeof(struct zram), GFP_KERNEL);
> - memset() that to 0
> - immediately fill in this RAM without reading it
>
> I'm wondering what the immediate need is to fill the area with zeros?

It's to avoid to have undefined values between the time the device is
created and initialized. In that case, it may be superfluous since the
locking mechanism should avoid to access an uninitialized device.

> Also curious as to whether the kzalloc() thing should better be
> kcalloc(num_devices, sizeof(struct zram), GFP_KERNEL) as a matter of
> convention.

It probably doesn't matter much. Apparently kcalloc() never got a lot
of success and kzalloc(sizeof(foo) * num_foos) type of allocation are
more popular.

Jerome

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

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