From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 92F73C169C4 for ; Tue, 12 Feb 2019 00:04:12 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5ECEA2184A for ; Tue, 12 Feb 2019 00:04:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="NXjiArMH" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5ECEA2184A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=fromorbit.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/uWJ9W1TA0WNLpCx69OinDoDiUAGN7cXTa9YnspXhWw=; b=NXjiArMHzkZakw edsTdYJLxFWjBsnubSf2pE3YL88CcEBm6ZaK4ZdDVmhCW5CkoWjvjOn902gT7TBuIQz0nqyejaHZ2 ymltiXc94T3VCq6dIW1mXi7tNYUrgNkXLvfcXaFMv154nZjG0OLsHlom4ycsOF40+33d99caFdvAA t0k5y975Hbmx6bCq17G7583HzD2kKW7u65sTL5sNCdtzQSj5RrJ+ziKKWHydiInvbRqpyllS075Q3 J/s1eRxZxR6kqDpQYT/JwSdZyV02gQEp4ZrCrbGfQhoEqu1UgKiatWgcCMGfils4SmIVQpig/ouOr MHtuDPzzUDuMwooltTIQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gtLYX-0007Ae-60; Tue, 12 Feb 2019 00:04:09 +0000 Received: from ipmail01.adl6.internode.on.net ([150.101.137.136]) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gtLYT-0007AJ-M1 for linux-mtd@lists.infradead.org; Tue, 12 Feb 2019 00:04:07 +0000 Received: from ppp59-167-129-252.static.internode.on.net (HELO dastard) ([59.167.129.252]) by ipmail01.adl6.internode.on.net with ESMTP; 12 Feb 2019 10:33:57 +1030 Received: from dave by dastard with local (Exim 4.80) (envelope-from ) id 1gtLYL-0002P2-4h; Tue, 12 Feb 2019 11:03:57 +1100 Date: Tue, 12 Feb 2019 11:03:57 +1100 From: Dave Chinner To: Eric Biggers Subject: Re: [RFC PATCH v2 11/20] fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl Message-ID: <20190212000357.GI20493@dastard> References: <20190211172738.4633-1-ebiggers@kernel.org> <20190211172738.4633-12-ebiggers@kernel.org> <20190211221249.GH20493@dastard> <20190211233128.GB226227@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190211233128.GB226227@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190211_160406_212367_92B750EE X-CRM114-Status: GOOD ( 23.60 ) X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Satya Tangirala , linux-api@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-fscrypt@vger.kernel.org, keyrings@vger.kernel.org, linux-mtd@lists.infradead.org, linux-crypto@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, Paul Crowley Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-mtd" Errors-To: linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org On Mon, Feb 11, 2019 at 03:31:29PM -0800, Eric Biggers wrote: > Hi Dave, > > On Tue, Feb 12, 2019 at 09:12:49AM +1100, Dave Chinner wrote: > > On Mon, Feb 11, 2019 at 09:27:29AM -0800, Eric Biggers wrote: > > > > Indeed, this is exactly what ->drop_inode() is for. > > > > Take this function: > > > > > +static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk) > > > +{ > > > + struct fscrypt_info *ci; > > > + struct inode *inode; > > > + struct inode *toput_inode = NULL; > > > + > > > + spin_lock(&mk->mk_decrypted_inodes_lock); > > > + > > > + list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) { > > > + inode = ci->ci_inode; > > > + spin_lock(&inode->i_lock); > > > + if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) { > > > + spin_unlock(&inode->i_lock); > > > + continue; > > > + } > > > + __iget(inode); > > > + spin_unlock(&inode->i_lock); > > > + spin_unlock(&mk->mk_decrypted_inodes_lock); > > > + > > > + shrink_dcache_inode(inode); > > > + iput(toput_inode); > > > + toput_inode = inode; > > > + > > > + spin_lock(&mk->mk_decrypted_inodes_lock); > > > + } > > > + > > > + spin_unlock(&mk->mk_decrypted_inodes_lock); > > > + iput(toput_inode); > > > +} > > > > It takes a new reference to each decrypted inode, and then drops it > > again after all the dentry cache references have been killed and > > we've got a reference to the next inode in the list. Killing the > > dentry references to the inode means it should only have in-use > > references and the reference this function holds on it. > > > > If the inode is not in use then there will be only one, and so it > > will fall into iput_final() and the ->drop_inode() function > > determines if the inode should be evicted from the cache and > > destroyed immediately. IOWs, implement fscrypt_drop_inode() to do > > the right thing when the key has been destroyed, and you can get rid > > of all this crazy inode cache walk-and-invalidate hackery. > > > > Thanks for the feedback! If I understand correctly, your suggestion is: > > - Keep evict_dentries_for_decrypted_inodes() as-is, i.e. fscrypt would still > evict the dentries for all inodes in ->mk_decrypted_inodes. > (I don't see how it could work otherwise.) > > - However, evict_decrypted_inodes() would be removed and fscrypt would not > directly evict the list of inodes. Instead, the filesystem's ->drop_inode() > would be made to return 1 if the inode's master key has been removed. Thus > each inode, if no longer in use, would end up getting evicted during the > iput() in evict_dentries_for_decrypted_inodes(). *nod* > I hadn't thought of this, and I think it would work; I'll try implementing it. > It would also have the advantage that if a key is removed while an inode is > still in-use, that inode will be evicted as soon as it's no longer in use rather > than waiting around until another FS_IOC_REMOVE_ENCRYPTION_KEY. *nod* > The ioctl will need a different way to determine whether any inodes couldn't be > evicted, but simply checking whether ->mk_decrypted_inodes ended up empty or not > should work. *nod* > FWIW, originally I also considered leaving the inodes in the inode cache and > instead only freeing ->i_crypt_info and truncating the pagecache. But I don't > see a way to do it even with this new idea; for one, ->drop_inode() is called > under ->i_lock. So it seems that eviction is still the way to go. Yeah, eviction is by far the easiest way to deal with this. If it's being frequently referenced/written, the backing buffer should be in memory anyway and the next access simply has to re-instantiate the inode cache from the buffer and won't need to do IO. Cheers, Dave. -- Dave Chinner david@fromorbit.com ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/