Received: by 2002:a05:6a10:9848:0:0:0:0 with SMTP id x8csp192478pxf; Wed, 24 Mar 2021 02:46:13 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwv7KVyMDPx8iyUdcAWIpuZXKqw0xgCe9QRmGyUav/kNOg3/0h9YeZo2SiEwAiRbckOHR5i X-Received: by 2002:aa7:db53:: with SMTP id n19mr2533399edt.330.1616579173308; Wed, 24 Mar 2021 02:46:13 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1616579173; cv=none; d=google.com; s=arc-20160816; b=pm2xlJdQ2i5ZuRm+sQBSS+5083KMCvxVj3iwYsMJUVaI/SVNDonZcBUlyEzPE/TOg2 HMNiagWMKDIUDSvWAEvVHmE5THXPcTKqSA5NCfhhFmCaQc+2mrLQNAb8Q09Dt5upCL85 Qpk9/o1XixJiCtpJedpbCyL/buTFipgoYNkApfHi8i805e/khR0tTsoPWESG4ZFNTq/g aH5fmvGFTLgugl+C/zUJ+Bh+oII+u50KB5XRc/DYv8+ci7Aam/H+J8FyOIf7T5PdQij8 aAcnHvMu3Gbtt5wgTNoLnZlTvBZ9WrOgx/DzcOpbb2kqUeIIBpFeF+29sB8lDS4L/3zw BLzQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=41t+0XxoZcRTvp5msaJ+40jeQNA2lKYfXZeN4M9XPqw=; b=vK17St1IzwGnNnfjiC10QcYfD8KdGPg+Tz1JlUlTDSzwcqKTzrD+Wat7hvyHoKWtka RC0EDlJ839F8cByU9oUnJX0ZBysw71hA3Y/KKRPDqrsKXrWQTqCBWuheLaV3K7So1qwJ sh2OZG5fOV4lcRwJvKm3yvRkGQgP7UZVx3VmAMtFBRaBrixPiFoZJVYbJ59u5Ka2fOSS i2LnzPsM1XvOOqukZbBsB4d0oTw6X+V+mqLhprfS7l5v8CphsUU0IY4yGBVDgNBODBPX m5W4VzEEdfz28R55zuzXj5QwNajXh3akDcDsXiZjdLPhG1eq2ZOTXJ3IzX/Eqx/nPn+L 7LDA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b="u/J8/9w3"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id b17si1395111eds.443.2021.03.24.02.45.50; Wed, 24 Mar 2021 02:46:13 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b="u/J8/9w3"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232563AbhCXEWE (ORCPT + 99 others); Wed, 24 Mar 2021 00:22:04 -0400 Received: from mail.kernel.org ([198.145.29.99]:39858 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231185AbhCXEWD (ORCPT ); Wed, 24 Mar 2021 00:22:03 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id D22F6619E0; Wed, 24 Mar 2021 04:22:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1616559723; bh=JUMOPNlFXenu8b+jfOqqANM0mC2Z8hnowz+qX2OCQdI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=u/J8/9w3HYkY1/vsmGH+9h6jcA1zy3y25aSDyz4I4wUoL5y/zey99NLVk1yYkJkQI of247FptTlN+R34Mu9VPMAZKEq8/mfAcuLWgN8WL0agEMIXPG+T9xVNmQ6HMFKsfuc pg40odZ7IjHy1+QTBIZEb0zqTZ89tGsVNql0c2vLqwlV0N2Q1sjHhWG1Ht+HaEk7Xm dxqbm5XgjoqBfHZtnIP1cl+LjnObf8dyyL1VNYwaZcdDbdhgfyC8eyaWBTw2BqJjpR 9YIBf/StAB1L9Wxi1RTGi2iOHuiWkUpnuJ9w9QmZJslLnA2UbW3jxc1cyL5FsB64fW Sj6qTqC+r6ftg== Date: Tue, 23 Mar 2021 21:22:01 -0700 From: Jaegeuk Kim To: Chao Yu Cc: linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, chao@kernel.org Subject: Re: [PATCH] Revert "f2fs: give a warning only for readonly partition" Message-ID: References: <20210323064155.12582-1-yuchao0@huawei.com> <107e671d-68ea-1a74-521e-ab2b6fe36416@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <107e671d-68ea-1a74-521e-ab2b6fe36416@huawei.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/24, Chao Yu wrote: > On 2021/3/24 2:39, Jaegeuk Kim wrote: > > On 03/23, Chao Yu wrote: > > > This reverts commit 938a184265d75ea474f1c6fe1da96a5196163789. > > > > > > Because that commit fails generic/050 testcase which expect failure > > > during mount a recoverable readonly partition. > > > > I think we need to change generic/050, since f2fs can recover this partition, > > Well, not sure we can change that testcase, since it restricts all generic > filesystems behavior. At least, ext4's behavior makes sense to me: > > journal_dev_ro = bdev_read_only(journal->j_dev); > really_read_only = bdev_read_only(sb->s_bdev) | journal_dev_ro; > > if (journal_dev_ro && !sb_rdonly(sb)) { > ext4_msg(sb, KERN_ERR, > "journal device read-only, try mounting with '-o ro'"); > err = -EROFS; > goto err_out; > } > > if (ext4_has_feature_journal_needs_recovery(sb)) { > if (sb_rdonly(sb)) { > ext4_msg(sb, KERN_INFO, "INFO: recovery " > "required on readonly filesystem"); > if (really_read_only) { > ext4_msg(sb, KERN_ERR, "write access " > "unavailable, cannot proceed " > "(try mounting with noload)"); > err = -EROFS; > goto err_out; > } > ext4_msg(sb, KERN_INFO, "write access will " > "be enabled during recovery"); > } > } > > > even though using it as readonly. And, valid checkpoint can allow for user to > > read all the data without problem. > > >> if (f2fs_hw_is_readonly(sbi)) { > > Since device is readonly now, all write to the device will fail, checkpoint can > not persist recovered data, after page cache is expired, user can see stale data. My point is, after mount with ro, there'll be no data write which preserves the current status. So, in the next time, we can recover fsync'ed data later, if user succeeds to mount as rw. Another point is, with the current checkpoint, we should not have any corrupted metadata. So, why not giving a chance to show what data remained to user? I think this can be doable only with CoW filesystems. > > Am I missing something? > > Thanks, > > > > > > > > > Fixes: 938a184265d7 ("f2fs: give a warning only for readonly partition") > > > Signed-off-by: Chao Yu > > > --- > > > fs/f2fs/super.c | 8 +++++--- > > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > > > index b48281642e98..2b78ee11f093 100644 > > > --- a/fs/f2fs/super.c > > > +++ b/fs/f2fs/super.c > > > @@ -3952,10 +3952,12 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) > > > * previous checkpoint was not done by clean system shutdown. > > > */ > > > if (f2fs_hw_is_readonly(sbi)) { > > > - if (!is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) > > > + if (!is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) { > > > + err = -EROFS; > > > f2fs_err(sbi, "Need to recover fsync data, but write access unavailable"); > > > - else > > > - f2fs_info(sbi, "write access unavailable, skipping recovery"); > > > + goto free_meta; > > > + } > > > + f2fs_info(sbi, "write access unavailable, skipping recovery"); > > > goto reset_checkpoint; > > > } > > > -- > > > 2.29.2 > > . > >