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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B3372C433EF for ; Fri, 1 Oct 2021 07:05:52 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 392C161A71 for ; Fri, 1 Oct 2021 07:05:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 392C161A71 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.dev Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id A2C8F9400F5; Fri, 1 Oct 2021 03:05:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9DCDE9400E4; Fri, 1 Oct 2021 03:05:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8CB669400F5; Fri, 1 Oct 2021 03:05:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0121.hostedemail.com [216.40.44.121]) by kanga.kvack.org (Postfix) with ESMTP id 7FC259400E4 for ; Fri, 1 Oct 2021 03:05:51 -0400 (EDT) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 4441832073 for ; Fri, 1 Oct 2021 07:05:51 +0000 (UTC) X-FDA: 78646983702.21.0F44E17 Received: from out2.migadu.com (out2.migadu.com [188.165.223.204]) by imf16.hostedemail.com (Postfix) with ESMTP id A7515F000AED for ; Fri, 1 Oct 2021 07:05:50 +0000 (UTC) Date: Fri, 1 Oct 2021 16:05:39 +0900 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1633071948; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=GvGmNJe2m6c7do6Iwp74+Xpwugj6O+M8yPQxd3iIJ60=; b=pMQryGTJW4ett9e1r6f81QtKAR+z8Lz88hg1yr+o5gM67pKsLpUqSYDJ7x2N2M4lTj9fzP yn7DOVLePs4+oWKlJk4DnoPMc1qqCk8/dDLI+mePZPoI5OOCU9xbi7AhtH8cspOO2cOb+z +z1HA4+UjMWnWl9A2k/euhFFAzvbu5A= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Naoya Horiguchi To: Yang Shi Cc: naoya.horiguchi@nec.com, hughd@google.com, kirill.shutemov@linux.intel.com, willy@infradead.org, peterx@redhat.com, osalvador@suse.de, akpm@linux-foundation.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [v3 PATCH 4/5] mm: shmem: don't truncate page if memory failure happens Message-ID: <20211001070539.GA1364952@u2004> References: <20210930215311.240774-1-shy828301@gmail.com> <20210930215311.240774-5-shy828301@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210930215311.240774-5-shy828301@gmail.com> X-Migadu-Flow: FLOW_OUT X-Migadu-Auth-User: naoya.horiguchi@linux.dev X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: A7515F000AED X-Stat-Signature: 1f3qap84o8x65muekkj4o1qxktfehucj Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=pMQryGTJ; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf16.hostedemail.com: domain of naoya.horiguchi@linux.dev designates 188.165.223.204 as permitted sender) smtp.mailfrom=naoya.horiguchi@linux.dev X-HE-Tag: 1633071950-193783 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Sep 30, 2021 at 02:53:10PM -0700, Yang Shi wrote: > The current behavior of memory failure is to truncate the page cache > regardless of dirty or clean. If the page is dirty the later access > will get the obsolete data from disk without any notification to the > users. This may cause silent data loss. It is even worse for shmem > since shmem is in-memory filesystem, truncating page cache means > discarding data blocks. The later read would return all zero. > > The right approach is to keep the corrupted page in page cache, any > later access would return error for syscalls or SIGBUS for page fault, > until the file is truncated, hole punched or removed. The regular > storage backed filesystems would be more complicated so this patch > is focused on shmem. This also unblock the support for soft > offlining shmem THP. > > Signed-off-by: Yang Shi > --- ... > @@ -894,6 +896,12 @@ static int me_pagecache_clean(struct page_state *ps, struct page *p) > goto out; > } > > + /* > + * The shmem page is kept in page cache instead of truncating > + * so need decrement the refcount from page cache. > + */ This comment seems to me confusing because no refcount is decremented here. What the variable dec tries to do is to give the expected value of the refcount of the error page after successfull erorr handling, which differs according to the page state before error handling, so dec adjusts it. How about the below? + /* + * The shmem page is kept in page cache instead of truncating + * so is expected to have an extra refcount after error-handling. + */ > + dec = shmem_mapping(mapping); > + > /* > * Truncation is a bit tricky. Enable it per file system for now. > * ... > @@ -2466,7 +2467,17 @@ shmem_write_begin(struct file *file, struct address_space *mapping, > return -EPERM; > } > > - return shmem_getpage(inode, index, pagep, SGP_WRITE); > + ret = shmem_getpage(inode, index, pagep, SGP_WRITE); > + > + if (*pagep) { > + if (PageHWPoison(*pagep)) { Unless you plan to add some code in the near future, how about merging these two if sentences? if (*pagep && PageHWPoison(*pagep)) { Thanks, Naoya Horiguchi > + unlock_page(*pagep); > + put_page(*pagep); > + ret = -EIO; > + } > + } > + > + return ret; > } > > static int