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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
