lkml.org 
[lkml]   [2015]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v1 2/2] zram: remove init_lock in zram_make_request
From
Hello, Sergey

2015-01-31 19:07 GMT+08:00 Sergey Senozhatsky <sergey.senozhatsky@gmail.com>:
> 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
>


\
 
 \ /
  Last update: 2015-01-31 14:21    [W:0.205 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site