Received: by 2002:ad5:474a:0:0:0:0:0 with SMTP id i10csp3978997imu; Mon, 12 Nov 2018 03:54:39 -0800 (PST) X-Google-Smtp-Source: AJdET5ec03OA/8HRxsMHaEB62NVx8qUVncoWuEPx+E44mbDx6bbs8DTURVr0YjUgAklbeczPPFVd X-Received: by 2002:a62:cdc1:: with SMTP id o184-v6mr612427pfg.181.1542023679856; Mon, 12 Nov 2018 03:54:39 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1542023679; cv=none; d=google.com; s=arc-20160816; b=echnM3ehPGh/47QSfl6LTgB3A23aJhfPURKoANVc2XlN7OsrkNM+aSsRLPYsR9nv2k gQjcxXL1IQrZ+tuKPUC+48NPwSoCImrieayUpGFlpHC8Csg69GIN7f32OOaRFuKN3RJI 4Dm+YJeRGkw2Wov59peyNxKVdPnd6e2zaZHfwE6wECls8FaGq1dSZRlak06L0tLbExjq VRaWZaYX3GHrpEShGOSAnxM93maVOcqWnX+GyrNhIEbTEv7H9Y3lKJGQlAR9wAfcgwYL 4TiND+cvv4EIzDbtzURgF1AboqfRI9Kx28Jgn9CUYK88CGMlLrvX/+lK0gxnQ24ZcUtL awag== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date; bh=eCzA6Lga491KhlYPzQnVlM8PP5Tj9zoRsfVyKs/1byo=; b=cCL7UjqKOdtN5VXxkVmZPzkbaSfRbt7s/04gn/mZjS4uYGrrzcRRSjc1oUev8luHvE lPmJOFURyzn/S1OcrplIGCtcj1dSoruM0DLdp/8rjr+dIIWhs7vJXML9AOTfMukpuc2L MZepawoyultwPs5zt3JlPV1oq8Fszj6OJTU8F9svJCbygFtuzY9fvJlE5b9DMEQ/jels 877LFvOcoFejylXjrBW8m2BTr+gGDNVQXtGn1llG+qex0eVDcna5CyuX3CqaiDo2c9jR FIfUjdbjzbkCy6kza/+r0cN5TnqVHON1s89403YnUdb4UgFN0Ay6GgZ721RUYLlIQgYa Odtw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id i2si16189042pgl.153.2018.11.12.03.54.24; Mon, 12 Nov 2018 03:54:39 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729331AbeKLVq6 (ORCPT + 99 others); Mon, 12 Nov 2018 16:46:58 -0500 Received: from mx2.suse.de ([195.135.220.15]:33438 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728302AbeKLVq6 (ORCPT ); Mon, 12 Nov 2018 16:46:58 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 4FEE2B036; Mon, 12 Nov 2018 11:54:01 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 9F7231E09A5; Mon, 12 Nov 2018 12:54:00 +0100 (CET) Date: Mon, 12 Nov 2018 12:54:00 +0100 From: Jan Kara To: Andrey Ryabinin Cc: Jan Kara , Pavel Tikhomirov , Andrew Morton , Vasily Averin , Konstantin Khorenko , Johannes Weiner , Mel Gorman , Matthew Wilcox , Andi Kleen , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm: cleancache: fix corruption on missed inode invalidation Message-ID: <20181112115400.GE7175@quack2.suse.cz> References: <20181112095734.17979-1-ptikhomirov@virtuozzo.com> <20181112113153.GC7175@quack2.suse.cz> <2abdb97e-0fed-0fb5-6941-e7afcc9e0209@virtuozzo.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2abdb97e-0fed-0fb5-6941-e7afcc9e0209@virtuozzo.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 12-11-18 14:40:06, Andrey Ryabinin wrote: > > > On 11/12/18 2:31 PM, Jan Kara wrote: > > On Mon 12-11-18 12:57:34, Pavel Tikhomirov wrote: > >> If all pages are deleted from the mapping by memory reclaim and also > >> moved to the cleancache: > >> > >> __delete_from_page_cache > >> (no shadow case) > >> unaccount_page_cache_page > >> cleancache_put_page > >> page_cache_delete > >> mapping->nrpages -= nr > >> (nrpages becomes 0) > >> > >> We don't clean the cleancache for an inode after final file truncation > >> (removal). > >> > >> truncate_inode_pages_final > >> check (nrpages || nrexceptional) is false > >> no truncate_inode_pages > >> no cleancache_invalidate_inode(mapping) > >> > >> These way when reading the new file created with same inode we may get > >> these trash leftover pages from cleancache and see wrong data instead of > >> the contents of the new file. > >> > >> Fix it by always doing truncate_inode_pages which is already ready for > >> nrpages == 0 && nrexceptional == 0 case and just invalidates inode. > >> > >> Fixes: commit 91b0abe36a7b ("mm + fs: store shadow entries in page cache") > >> To: Andrew Morton > >> Cc: Johannes Weiner > >> Cc: Mel Gorman > >> Cc: Jan Kara > >> Cc: Matthew Wilcox > >> Cc: Andi Kleen > >> Cc: linux-mm@kvack.org > >> Cc: linux-kernel@vger.kernel.org > >> Reviewed-by: Vasily Averin > >> Reviewed-by: Andrey Ryabinin > >> Signed-off-by: Pavel Tikhomirov > >> --- > >> mm/truncate.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > > > > The patch looks good but can you add a short comment before the > > truncate_inode_pages() call explaining why it needs to be called always? > > Something like: > > > > /* > > * Cleancache needs notification even if there are no pages or > > * shadow entries... > > */ > > Or we can just call cleancache_invalidate_inode(mapping) on else branch, > so the code would be more self-explanatory, and also avoid > function call in no-cleancache setups, which should the most of setups. That is workable for me as well although I'd be somewhat worried that if we have calls to inform cleancache about final inode teardown in two different places, they can get out of sync easily. So I somewhat prefer the current solution + comment. Honza -- Jan Kara SUSE Labs, CR