From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752802AbbAaLHN (ORCPT ); Sat, 31 Jan 2015 06:07:13 -0500 Received: from mail-pa0-f54.google.com ([209.85.220.54]:63285 "EHLO mail-pa0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750957AbbAaLHK (ORCPT ); Sat, 31 Jan 2015 06:07:10 -0500 Date: Sat, 31 Jan 2015 20:07:43 +0900 From: Sergey Senozhatsky To: Ganesh Mahendran Cc: Sergey Senozhatsky , Sergey Senozhatsky , Minchan Kim , Andrew Morton , linux-kernel , Linux-MM , Nitin Gupta , Jerome Marchand Subject: Re: [PATCH v1 2/2] zram: remove init_lock in zram_make_request Message-ID: <20150131110743.GA2299@swordfish> References: <1422432945-6764-1-git-send-email-minchan@kernel.org> <1422432945-6764-2-git-send-email-minchan@kernel.org> <20150129151227.GA936@swordfish> <20150130080808.GA782@swordfish> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? 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 ... 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() > 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