Hello, Sergey 2015-01-31 19:07 GMT+08:00 Sergey Senozhatsky <sergey.senozhatsky@xxxxxxxxx>: > On (01/31/15 16:50), Ganesh Mahendran wrote: >> >> > after umount we still have init device. so, *theoretically*, we >> >> > can see something like >> >> > >> >> > CPU0 CPU1 >> >> > umount >> >> > reset_store >> >> > bdev->bd_holders == 0 mount >> >> > ... zram_make_request() >> >> > zram_reset_device() > [..] > > >> >> Maybe I did not explain clearly. I send a patch about this issue: >> >> https://patchwork.kernel.org/patch/5754041/ > > > excuse me? explain to me clearly what? my finding and my analysis? Sorry, I missed this mail https://lkml.org/lkml/2015/1/27/1029 That's why I ask questions in this https://lkml.org/lkml/2015/1/29/580 after Minchan's description. > > > this is the second time in a week that you hijack someone's work > and you don't even bother to give any credit to people. > > > Minchan moved zram_meta_free(meta) out of init_lock here > https://lkml.org/lkml/2015/1/21/29 > > I proposed to also move zs_free() of meta->handles here > https://lkml.org/lkml/2015/1/21/384 I thought you wanted move the code block after up_write(&zram->init_lock); And I found the code block can be even encapsulated in zram_meta_free(). That's why I sent: https://lkml.org/lkml/2015/1/24/50 > > > ... so what happened then -- you jumped in and sent a patch. > https://lkml.org/lkml/2015/1/24/50 > > > Minchan sent you a hint https://lkml.org/lkml/2015/1/26/471 > >> but it seems the patch is based on my recent work "zram: free meta out of init_lock". > > > > "the patch is based on my work"! > > > > now, for the last few days we were discussing init_lock and I first > expressed my concerns and spoke about 'free' vs. 'use' problem > here (but still didn't have enough spare to submit, besides we are in > the middle of reset/init/write rework) > > https://lkml.org/lkml/2015/1/27/1029 > >> >>bdev->bd_holders protects from resetting device which has read/write >>operation ongoing on the onther CPU. >> >>I need to refresh on how ->bd_holders actually incremented/decremented. >>can the following race condition take a place? >> >> CPU0 CPU1 >>reset_store() >>bdev->bd_holders == false >> zram_make_request >> -rm- down_read(&zram->init_lock); >> init_done(zram) == true >>zram_reset_device() valid_io_request() >> __zram_make_request >>down_write(&zram->init_lock); zram_bvec_rw >>[..] >>set_capacity(zram->disk, 0); >>zram->init_done = false; >>kick_all_cpus_sync(); zram_bvec_write or zram_bvec_read() >>zram_meta_free(zram->meta); >>zcomp_destroy(zram->comp); zcomp_compress() or zcomp_decompress() Sorry, I did not check this mail. >> > > > and later here https://lkml.org/lkml/2015/1/29/645 > >> >>after umount we still have init device. so, *theoretically*, we >>can see something like >> >> >> CPU0 CPU1 >>umount >>reset_store >>bdev->bd_holders == 0 mount >>... zram_make_request() >>zram_reset_device() >> > > > > so what happened next? your patch happened next. > with quite familiar problem description > >> >> CPU0 CPU1 >> t1: bdput >> t2: mount /dev/zram0 /mnt >> t3: zram_reset_device >> > > and now you say that I don't understant something in "your analysis"? > > > > stop doing this. this is not how it works. > > > -ss > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>