Re: [PATCH 2/2] staging: zram: remove init_done from zram struct (v2)

[Dan Carpenter <dan.carpenter@oracle.com>]

On Fri, Sep 06, 2013 at 06:21:20PM +0300, Sergey Senozhatsky wrote:
> @@ -558,14 +563,12 @@ static void zram_reset_device(struct zram *zram, bool reset_capacity)
>  	flush_work(&zram->free_work);
>  
>  	down_write(&zram->init_lock);
> -	if (!zram->init_done) {
> +	if (!init_done(zram)) {
>  		up_write(&zram->init_lock);
>  		return;
>  	}
>  
>  	meta = zram->meta;
> -	zram->init_done = 0;
> -
>  	/* Free all pages that are still in this zram device */
>  	for (index = 0; index < zram->disksize >> PAGE_SHIFT; index++) {
>  		unsigned long handle = meta->table[index].handle;
> @@ -604,9 +607,7 @@ static void zram_init_device(struct zram *zram, struct zram_meta *meta)
>  
>  	/* zram devices sort of resembles non-rotational disks */
>  	queue_flag_set_unlocked(QUEUE_FLAG_NONROT, zram->disk->queue);
> -
>  	zram->meta = meta;
> -	zram->init_done = 1;
>  
>  	pr_debug("Initialization done!\n");
>  }

I am uncomfortable with the locking in zram_reset_device().  There
should be a check for init_done() in zram_slot_free_notify() otherwise
we could add more work at the same time we are calling flush_work().

It should be that as soon as we start to reset then we say init is not
done, we stop loading more work, we any existing work and then clean up.
(There are details involved that I haven't looked at, but the original
code looks racy to me).

regards,
dan carpenter