Re: [PATCHv2 03/10] zram: use idr instead of `zram_devices' array

Sergey Senozhatsky
2015-04-23T04:30:10Z

On (04/23/15 11:23), Minchan Kim wrote:
[..]

> > +static int zram_exit_cb(int id, void *ptr, void *data)
>
> trivial: I prefer remove to exit.
>

ok.

> > +{
> > + zram_remove(ptr);
> > + return 0;
> > +}
> >
> > - kfree(zram_devices);
> > +static void destroy_devices(void)
> > +{
> > + idr_for_each(&zram_index_idr, &zram_exit_cb, NULL);
> > + idr_destroy(&zram_index_idr);
> > unregister_blkdev(zram_major, "zram");
> > - pr_info("Destroyed %u device(s)\n", nr);
> > + pr_info("Destroyed device(s)\n");
> > }
> >
> > static int __init zram_init(void)
> > @@ -1283,16 +1302,9 @@ static int __init zram_init(void)
> > return -EBUSY;
> > }
> >
> > - /* Allocate the device array and initialize each one */
> > - zram_devices = kzalloc(num_devices * sizeof(struct zram), GFP_KERNEL);
> > - if (!zram_devices) {
> > - unregister_blkdev(zram_major, "zram");
> > - return -ENOMEM;
> > - }
> > -
> > for (dev_id = 0; dev_id < num_devices; dev_id++) {
> > - ret = create_device(&zram_devices[dev_id], dev_id);
> > - if (ret)
> > + ret = zram_add(dev_id);
> > + if (ret != 0)
>
> It's better to check ret < 0 rather than ret != 0.
>

yes, it's

for (dev_id = 0; dev_id < num_devices; dev_id++) {
mutex_lock(&zram_index_mutex);
ret = zram_add(dev_id);
mutex_unlock(&zram_index_mutex);
if (ret < 0)
goto out_error;
}

at the end of the patch set.

will clean up everything and resubmit after the merge window, next week.

thanks!

-ss

> Otherwise,
> Acked-by: Minchan Kim <minchan@kernel.org>
>
> --
> Kind regards,
> Minchan Kim
>