Message ID | 20230402112825.42486-1-chao@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp1769744vqo; Sun, 2 Apr 2023 07:32:28 -0700 (PDT) X-Google-Smtp-Source: AKy350bJGP7WtAVloMZDg8ZG2xKVnchN4uA+unWQ4l/1y2L9FA/Hn1Nb7IZXsj4ZvO2pHMKTIrDo X-Received: by 2002:a17:903:2341:b0:19c:e666:a678 with SMTP id c1-20020a170903234100b0019ce666a678mr35997174plh.50.1680445947993; Sun, 02 Apr 2023 07:32:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1680445947; cv=none; d=google.com; s=arc-20160816; b=XEQqAEi5Y3o/9MJCiBHxpAsEDPIEY0sKrfRUQX9TOhlNj21l+zidvyAXF12j2h8OJu VOVxi4y58UNjZlIfxxePEo83QWaL2jriBF5SQz64s07YGWl94p2PR766jv9Djw2YN8Kv GUE14lDIpRa7Dym0Uf/WpEYzduZTCugKl+N8mGSj13dxjyFn3BkY5DG9jpwusm1/70AV TfMLgNe7iG3b70Tlp+q5dvZlW1Bbce2DGOHv8AA+pmsVEnupPzQgrMMSUeqoj9ZYD0pC ab3+b2tkxZVywE86rFnXb68Lvj/8oE0JG06GMYyE13+E3HatcOj2c4cRpjfGR+Hi7RN2 oyUg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=oAJ6wnKs7pHk1kU21AHBCRd0mLOp7pIytOQooGGNy1Y=; b=Y6miYNLblm55jBAHlkwB6gUBJso5NmB0rVAm41b8vSfWlqiPKHdpxxQ1uDvMPzTF3X GzDdyzWgiFVZ1CbQdj/lDTV4Inr9fqhdtq+elgbuEDet0XBhH2tspTZi+84Gyx20iYAN z3NH08PoNL0bR93dBxjJQtFwpiuD3RflI1VXEc4OFzerUfQ2tVaxRdcBF2qcoYQ5o6t4 pQMpPGh6oJBDrTeFrnPzy7OnIPgZ2DF6+mmnvvTeTatSELYkCC+W+VwL9ZHHnlWFWtLw oFQJ1wonITjGLYbASCLBE3a0jT5Wjm6SxgHS7Tz5tuz+LZQFUEcRUkwjWMpupFc2C8Hr QJhg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=X2wM5cOl; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id li12-20020a170903294c00b0019b0b007994si6169074plb.163.2023.04.02.07.32.15; Sun, 02 Apr 2023 07:32:27 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=X2wM5cOl; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 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 S230420AbjDBOQz (ORCPT <rfc822;dexuan.linux@gmail.com> + 99 others); Sun, 2 Apr 2023 10:16:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60438 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230383AbjDBOQy (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 2 Apr 2023 10:16:54 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 46C1424AF4 for <linux-kernel@vger.kernel.org>; Sun, 2 Apr 2023 07:16:53 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id DFD9F60B0A for <linux-kernel@vger.kernel.org>; Sun, 2 Apr 2023 14:16:52 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8C27BC433EF; Sun, 2 Apr 2023 14:16:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1680445012; bh=Qtn8hWe71iYL9XhCOCUkzSe1paWQKj/BQ5emAN7XJ0g=; h=From:To:Cc:Subject:Date:From; b=X2wM5cOl9weVcBsxJh1ZyFcw1pOWFy7Vj7wh4ACVlrt58Rb+NhkgDz+psWsoDquDc qw2cF+Tvl6L2i961U+hvI4wgMn/o5FJtryRoKykoF9xM/5WpB+8LtjFzZdctb2ULcc g4rOblIcczYYrylu8i6k5knfjFrFUvCASA7Zbsm58XacdF/NhgjPHfg6udISvibXvr JKte1GOUM9Sjnqe+NOOK5wP8Rx4fw1xTVyQF+J9K4bQ3tKI5fGEUszds66jbyceqpq Tot0BqFLANebBYBeM7SJMa+GvawQI0BpKO/T/lpmXU527ib/u6C2bQHesIGskjWtbK AGbkGbz12RT5Q== From: Chao Yu <chao@kernel.org> To: jaegeuk@kernel.org Cc: linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, Chao Yu <chao@kernel.org> Subject: [PATCH] f2fs: fix to check readonly condition correctly Date: Sun, 2 Apr 2023 19:28:25 +0800 Message-Id: <20230402112825.42486-1-chao@kernel.org> X-Mailer: git-send-email 2.36.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-5.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,SPF_HELO_NONE, SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1762075290340529225?= X-GMAIL-MSGID: =?utf-8?q?1762075290340529225?= |
Series |
f2fs: fix to check readonly condition correctly
|
|
Commit Message
Chao Yu
April 2, 2023, 11:28 a.m. UTC
This patch does below changes:
- If f2fs enables readonly feature or device is readonly, allow to
mount readonly mode only
- Introduce f2fs_dev_is_readonly() to indicate whether image or device
is readonly
- remove unnecessary f2fs_hw_is_readonly() in f2fs_write_checkpoint()
and f2fs_convert_inline_inode()
- enable FLUSH_MERGE only if f2fs is mounted as rw and image/device
is writable
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/checkpoint.c | 2 +-
fs/f2fs/f2fs.h | 10 +++++-----
fs/f2fs/inline.c | 3 +--
fs/f2fs/super.c | 13 +++++++------
4 files changed, 14 insertions(+), 14 deletions(-)
Comments
On 04/02, Chao Yu wrote: > This patch does below changes: > > - If f2fs enables readonly feature or device is readonly, allow to > mount readonly mode only > - Introduce f2fs_dev_is_readonly() to indicate whether image or device > is readonly > - remove unnecessary f2fs_hw_is_readonly() in f2fs_write_checkpoint() > and f2fs_convert_inline_inode() > - enable FLUSH_MERGE only if f2fs is mounted as rw and image/device > is writable What is the problem to solve here? > > Signed-off-by: Chao Yu <chao@kernel.org> > --- > fs/f2fs/checkpoint.c | 2 +- > fs/f2fs/f2fs.h | 10 +++++----- > fs/f2fs/inline.c | 3 +-- > fs/f2fs/super.c | 13 +++++++------ > 4 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > index 8e1db5752fff..1eef597ed393 100644 > --- a/fs/f2fs/checkpoint.c > +++ b/fs/f2fs/checkpoint.c > @@ -1604,7 +1604,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) > unsigned long long ckpt_ver; > int err = 0; > > - if (f2fs_readonly(sbi->sb) || f2fs_hw_is_readonly(sbi)) > + if (f2fs_readonly(sbi->sb)) > return -EROFS; > > if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) { > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 2d4a7ef62537..7de95133478a 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -4446,6 +4446,11 @@ static inline bool f2fs_hw_is_readonly(struct f2fs_sb_info *sbi) > return false; > } > > +static inline bool f2fs_dev_is_readonly(struct f2fs_sb_info *sbi) > +{ > + return f2fs_sb_has_readonly(sbi) || f2fs_hw_is_readonly(sbi); > +} > + > static inline bool f2fs_lfs_mode(struct f2fs_sb_info *sbi) > { > return F2FS_OPTION(sbi).fs_mode == FS_MODE_LFS; > @@ -4546,11 +4551,6 @@ static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs, > } > } > > -static inline bool f2fs_is_readonly(struct f2fs_sb_info *sbi) > -{ > - return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb); > -} > - > #define EFSBADCRC EBADMSG /* Bad CRC detected */ > #define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */ > > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c > index 72269e7efd26..2c36f2dc2317 100644 > --- a/fs/f2fs/inline.c > +++ b/fs/f2fs/inline.c > @@ -203,8 +203,7 @@ int f2fs_convert_inline_inode(struct inode *inode) > struct page *ipage, *page; > int err = 0; > > - if (!f2fs_has_inline_data(inode) || > - f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb)) > + if (!f2fs_has_inline_data(inode) || f2fs_readonly(sbi->sb)) > return 0; > > err = f2fs_dquot_initialize(inode); > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index d016f398fcad..db7649010c12 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -1382,15 +1382,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) > return -EINVAL; > } > > - if (f2fs_is_readonly(sbi) && test_opt(sbi, FLUSH_MERGE)) { > + if (f2fs_dev_is_readonly(sbi) && !f2fs_readonly(sbi->sb)) { > + f2fs_err(sbi, "Allow to mount readonly mode only"); > + return -EROFS; > + } > + > + if (f2fs_readonly(sbi->sb) && test_opt(sbi, FLUSH_MERGE)) { > f2fs_err(sbi, "FLUSH_MERGE not compatible with readonly mode"); > return -EINVAL; > } > > - if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) { > - f2fs_err(sbi, "Allow to mount readonly mode only"); > - return -EROFS; > - } > return 0; > } > > @@ -2122,7 +2123,7 @@ static void default_options(struct f2fs_sb_info *sbi) > set_opt(sbi, MERGE_CHECKPOINT); > F2FS_OPTION(sbi).unusable_cap = 0; > sbi->sb->s_flags |= SB_LAZYTIME; > - if (!f2fs_is_readonly(sbi)) > + if (!f2fs_readonly(sbi->sb) && !f2fs_dev_is_readonly(sbi)) > set_opt(sbi, FLUSH_MERGE); > if (f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi)) > set_opt(sbi, DISCARD); > -- > 2.36.1
On 2023/4/4 1:59, Jaegeuk Kim wrote: > On 04/02, Chao Yu wrote: >> This patch does below changes: >> >> - If f2fs enables readonly feature or device is readonly, allow to >> mount readonly mode only >> - Introduce f2fs_dev_is_readonly() to indicate whether image or device >> is readonly >> - remove unnecessary f2fs_hw_is_readonly() in f2fs_write_checkpoint() >> and f2fs_convert_inline_inode() >> - enable FLUSH_MERGE only if f2fs is mounted as rw and image/device >> is writable > > What is the problem to solve here? With below case, it can mount multi-device image w/ rw option, however one of secondary device is set as ro, later update will cause panic, so I introduced f2fs_dev_is_readonly(), and check multi-devices rw status in parse_options() w/ it in order to avoid such inconsistent mount status. mkfs.f2fs -c /dev/zram1 /dev/zram0 -f blockdev --setro /dev/zram1 mount -t f2fs /dev/zram0 /mnt/f2fs/ mount: /mnt/f2fs: WARNING: source write-protected, mounted read-only. mount -t f2fs -o remount,rw /mnt/f2fs/ dd if=/dev/zero of=/mnt/f2fs/file bs=1M count=8192 kernel BUG at fs/f2fs/inline.c:258! RIP: 0010:f2fs_write_inline_data+0x23e/0x2d0 [f2fs] Call Trace: f2fs_write_single_data_page+0x26b/0x9f0 [f2fs] f2fs_write_cache_pages+0x389/0xa60 [f2fs] __f2fs_write_data_pages+0x26b/0x2d0 [f2fs] f2fs_write_data_pages+0x2e/0x40 [f2fs] do_writepages+0xd3/0x1b0 __writeback_single_inode+0x5b/0x420 writeback_sb_inodes+0x236/0x5a0 __writeback_inodes_wb+0x56/0xf0 wb_writeback+0x2a3/0x490 wb_do_writeback+0x2b2/0x330 wb_workfn+0x6a/0x260 process_one_work+0x270/0x5e0 worker_thread+0x52/0x3e0 kthread+0xf4/0x120 ret_from_fork+0x29/0x50 Thanks, > >> >> Signed-off-by: Chao Yu <chao@kernel.org> >> --- >> fs/f2fs/checkpoint.c | 2 +- >> fs/f2fs/f2fs.h | 10 +++++----- >> fs/f2fs/inline.c | 3 +-- >> fs/f2fs/super.c | 13 +++++++------ >> 4 files changed, 14 insertions(+), 14 deletions(-) >> >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >> index 8e1db5752fff..1eef597ed393 100644 >> --- a/fs/f2fs/checkpoint.c >> +++ b/fs/f2fs/checkpoint.c >> @@ -1604,7 +1604,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) >> unsigned long long ckpt_ver; >> int err = 0; >> >> - if (f2fs_readonly(sbi->sb) || f2fs_hw_is_readonly(sbi)) >> + if (f2fs_readonly(sbi->sb)) >> return -EROFS; >> >> if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) { >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index 2d4a7ef62537..7de95133478a 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -4446,6 +4446,11 @@ static inline bool f2fs_hw_is_readonly(struct f2fs_sb_info *sbi) >> return false; >> } >> >> +static inline bool f2fs_dev_is_readonly(struct f2fs_sb_info *sbi) >> +{ >> + return f2fs_sb_has_readonly(sbi) || f2fs_hw_is_readonly(sbi); >> +} >> + >> static inline bool f2fs_lfs_mode(struct f2fs_sb_info *sbi) >> { >> return F2FS_OPTION(sbi).fs_mode == FS_MODE_LFS; >> @@ -4546,11 +4551,6 @@ static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs, >> } >> } >> >> -static inline bool f2fs_is_readonly(struct f2fs_sb_info *sbi) >> -{ >> - return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb); >> -} >> - >> #define EFSBADCRC EBADMSG /* Bad CRC detected */ >> #define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */ >> >> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c >> index 72269e7efd26..2c36f2dc2317 100644 >> --- a/fs/f2fs/inline.c >> +++ b/fs/f2fs/inline.c >> @@ -203,8 +203,7 @@ int f2fs_convert_inline_inode(struct inode *inode) >> struct page *ipage, *page; >> int err = 0; >> >> - if (!f2fs_has_inline_data(inode) || >> - f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb)) >> + if (!f2fs_has_inline_data(inode) || f2fs_readonly(sbi->sb)) >> return 0; >> >> err = f2fs_dquot_initialize(inode); >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >> index d016f398fcad..db7649010c12 100644 >> --- a/fs/f2fs/super.c >> +++ b/fs/f2fs/super.c >> @@ -1382,15 +1382,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) >> return -EINVAL; >> } >> >> - if (f2fs_is_readonly(sbi) && test_opt(sbi, FLUSH_MERGE)) { >> + if (f2fs_dev_is_readonly(sbi) && !f2fs_readonly(sbi->sb)) { >> + f2fs_err(sbi, "Allow to mount readonly mode only"); >> + return -EROFS; >> + } >> + >> + if (f2fs_readonly(sbi->sb) && test_opt(sbi, FLUSH_MERGE)) { >> f2fs_err(sbi, "FLUSH_MERGE not compatible with readonly mode"); >> return -EINVAL; >> } >> >> - if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) { >> - f2fs_err(sbi, "Allow to mount readonly mode only"); >> - return -EROFS; >> - } >> return 0; >> } >> >> @@ -2122,7 +2123,7 @@ static void default_options(struct f2fs_sb_info *sbi) >> set_opt(sbi, MERGE_CHECKPOINT); >> F2FS_OPTION(sbi).unusable_cap = 0; >> sbi->sb->s_flags |= SB_LAZYTIME; >> - if (!f2fs_is_readonly(sbi)) >> + if (!f2fs_readonly(sbi->sb) && !f2fs_dev_is_readonly(sbi)) >> set_opt(sbi, FLUSH_MERGE); >> if (f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi)) >> set_opt(sbi, DISCARD); >> -- >> 2.36.1
On 2023/4/4 1:59, Jaegeuk Kim wrote: > On 04/02, Chao Yu wrote: >> This patch does below changes: >> >> - If f2fs enables readonly feature or device is readonly, allow to >> mount readonly mode only >> - Introduce f2fs_dev_is_readonly() to indicate whether image or device >> is readonly >> - remove unnecessary f2fs_hw_is_readonly() in f2fs_write_checkpoint() >> and f2fs_convert_inline_inode() >> - enable FLUSH_MERGE only if f2fs is mounted as rw and image/device >> is writable Jaegeuk, Please drop this patch, and help to check below one, thanks. :) f2fs: use f2fs_hw_is_readonly() instead of bdev_read_only() Thanks, > > What is the problem to solve here? > >> >> Signed-off-by: Chao Yu <chao@kernel.org> >> --- >> fs/f2fs/checkpoint.c | 2 +- >> fs/f2fs/f2fs.h | 10 +++++----- >> fs/f2fs/inline.c | 3 +-- >> fs/f2fs/super.c | 13 +++++++------ >> 4 files changed, 14 insertions(+), 14 deletions(-) >> >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >> index 8e1db5752fff..1eef597ed393 100644 >> --- a/fs/f2fs/checkpoint.c >> +++ b/fs/f2fs/checkpoint.c >> @@ -1604,7 +1604,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) >> unsigned long long ckpt_ver; >> int err = 0; >> >> - if (f2fs_readonly(sbi->sb) || f2fs_hw_is_readonly(sbi)) >> + if (f2fs_readonly(sbi->sb)) >> return -EROFS; >> >> if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) { >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index 2d4a7ef62537..7de95133478a 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -4446,6 +4446,11 @@ static inline bool f2fs_hw_is_readonly(struct f2fs_sb_info *sbi) >> return false; >> } >> >> +static inline bool f2fs_dev_is_readonly(struct f2fs_sb_info *sbi) >> +{ >> + return f2fs_sb_has_readonly(sbi) || f2fs_hw_is_readonly(sbi); >> +} >> + >> static inline bool f2fs_lfs_mode(struct f2fs_sb_info *sbi) >> { >> return F2FS_OPTION(sbi).fs_mode == FS_MODE_LFS; >> @@ -4546,11 +4551,6 @@ static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs, >> } >> } >> >> -static inline bool f2fs_is_readonly(struct f2fs_sb_info *sbi) >> -{ >> - return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb); >> -} >> - >> #define EFSBADCRC EBADMSG /* Bad CRC detected */ >> #define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */ >> >> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c >> index 72269e7efd26..2c36f2dc2317 100644 >> --- a/fs/f2fs/inline.c >> +++ b/fs/f2fs/inline.c >> @@ -203,8 +203,7 @@ int f2fs_convert_inline_inode(struct inode *inode) >> struct page *ipage, *page; >> int err = 0; >> >> - if (!f2fs_has_inline_data(inode) || >> - f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb)) >> + if (!f2fs_has_inline_data(inode) || f2fs_readonly(sbi->sb)) >> return 0; >> >> err = f2fs_dquot_initialize(inode); >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >> index d016f398fcad..db7649010c12 100644 >> --- a/fs/f2fs/super.c >> +++ b/fs/f2fs/super.c >> @@ -1382,15 +1382,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) >> return -EINVAL; >> } >> >> - if (f2fs_is_readonly(sbi) && test_opt(sbi, FLUSH_MERGE)) { >> + if (f2fs_dev_is_readonly(sbi) && !f2fs_readonly(sbi->sb)) { >> + f2fs_err(sbi, "Allow to mount readonly mode only"); >> + return -EROFS; >> + } >> + >> + if (f2fs_readonly(sbi->sb) && test_opt(sbi, FLUSH_MERGE)) { >> f2fs_err(sbi, "FLUSH_MERGE not compatible with readonly mode"); >> return -EINVAL; >> } >> >> - if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) { >> - f2fs_err(sbi, "Allow to mount readonly mode only"); >> - return -EROFS; >> - } >> return 0; >> } >> >> @@ -2122,7 +2123,7 @@ static void default_options(struct f2fs_sb_info *sbi) >> set_opt(sbi, MERGE_CHECKPOINT); >> F2FS_OPTION(sbi).unusable_cap = 0; >> sbi->sb->s_flags |= SB_LAZYTIME; >> - if (!f2fs_is_readonly(sbi)) >> + if (!f2fs_readonly(sbi->sb) && !f2fs_dev_is_readonly(sbi)) >> set_opt(sbi, FLUSH_MERGE); >> if (f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi)) >> set_opt(sbi, DISCARD); >> -- >> 2.36.1
On 2023/4/10 10:19, Chao Yu wrote: > On 2023/4/4 1:59, Jaegeuk Kim wrote: >> On 04/02, Chao Yu wrote: >>> This patch does below changes: >>> >>> - If f2fs enables readonly feature or device is readonly, allow to >>> mount readonly mode only >>> - Introduce f2fs_dev_is_readonly() to indicate whether image or device >>> is readonly >>> - remove unnecessary f2fs_hw_is_readonly() in f2fs_write_checkpoint() >>> and f2fs_convert_inline_inode() >>> - enable FLUSH_MERGE only if f2fs is mounted as rw and image/device >>> is writable > > Jaegeuk, > > Please drop this patch, and help to check below one, thanks. :) > > f2fs: use f2fs_hw_is_readonly() instead of bdev_read_only() Oh, I didn't make it clear. I found you applied both v1 and v2 of this patch. So I mean how about keeping v2: [PATCH v2] f2fs: fix to check readonly condition correctly https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev-test&id=2bce08d26c0fe3a4ce1cca03ee0ea0c59d98c0b2 and apply [PATCH] f2fs: use f2fs_hw_is_readonly() instead of bdev_read_only() https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev-test&id=7ec21ff2e8f521842938415af741fc72f39c0135 to instead [PATCH] f2fs: fix to check readonly condition correctly https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev-test&id=66eb18a8b892afe085097747521fb280a1577ea4 Thanks, > > Thanks, > >> >> What is the problem to solve here? >> >>> >>> Signed-off-by: Chao Yu <chao@kernel.org> >>> --- >>> fs/f2fs/checkpoint.c | 2 +- >>> fs/f2fs/f2fs.h | 10 +++++----- >>> fs/f2fs/inline.c | 3 +-- >>> fs/f2fs/super.c | 13 +++++++------ >>> 4 files changed, 14 insertions(+), 14 deletions(-) >>> >>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >>> index 8e1db5752fff..1eef597ed393 100644 >>> --- a/fs/f2fs/checkpoint.c >>> +++ b/fs/f2fs/checkpoint.c >>> @@ -1604,7 +1604,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) >>> unsigned long long ckpt_ver; >>> int err = 0; >>> >>> - if (f2fs_readonly(sbi->sb) || f2fs_hw_is_readonly(sbi)) >>> + if (f2fs_readonly(sbi->sb)) >>> return -EROFS; >>> >>> if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) { >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>> index 2d4a7ef62537..7de95133478a 100644 >>> --- a/fs/f2fs/f2fs.h >>> +++ b/fs/f2fs/f2fs.h >>> @@ -4446,6 +4446,11 @@ static inline bool f2fs_hw_is_readonly(struct f2fs_sb_info *sbi) >>> return false; >>> } >>> >>> +static inline bool f2fs_dev_is_readonly(struct f2fs_sb_info *sbi) >>> +{ >>> + return f2fs_sb_has_readonly(sbi) || f2fs_hw_is_readonly(sbi); >>> +} >>> + >>> static inline bool f2fs_lfs_mode(struct f2fs_sb_info *sbi) >>> { >>> return F2FS_OPTION(sbi).fs_mode == FS_MODE_LFS; >>> @@ -4546,11 +4551,6 @@ static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs, >>> } >>> } >>> >>> -static inline bool f2fs_is_readonly(struct f2fs_sb_info *sbi) >>> -{ >>> - return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb); >>> -} >>> - >>> #define EFSBADCRC EBADMSG /* Bad CRC detected */ >>> #define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */ >>> >>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c >>> index 72269e7efd26..2c36f2dc2317 100644 >>> --- a/fs/f2fs/inline.c >>> +++ b/fs/f2fs/inline.c >>> @@ -203,8 +203,7 @@ int f2fs_convert_inline_inode(struct inode *inode) >>> struct page *ipage, *page; >>> int err = 0; >>> >>> - if (!f2fs_has_inline_data(inode) || >>> - f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb)) >>> + if (!f2fs_has_inline_data(inode) || f2fs_readonly(sbi->sb)) >>> return 0; >>> >>> err = f2fs_dquot_initialize(inode); >>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >>> index d016f398fcad..db7649010c12 100644 >>> --- a/fs/f2fs/super.c >>> +++ b/fs/f2fs/super.c >>> @@ -1382,15 +1382,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) >>> return -EINVAL; >>> } >>> >>> - if (f2fs_is_readonly(sbi) && test_opt(sbi, FLUSH_MERGE)) { >>> + if (f2fs_dev_is_readonly(sbi) && !f2fs_readonly(sbi->sb)) { >>> + f2fs_err(sbi, "Allow to mount readonly mode only"); >>> + return -EROFS; >>> + } >>> + >>> + if (f2fs_readonly(sbi->sb) && test_opt(sbi, FLUSH_MERGE)) { >>> f2fs_err(sbi, "FLUSH_MERGE not compatible with readonly mode"); >>> return -EINVAL; >>> } >>> >>> - if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) { >>> - f2fs_err(sbi, "Allow to mount readonly mode only"); >>> - return -EROFS; >>> - } >>> return 0; >>> } >>> >>> @@ -2122,7 +2123,7 @@ static void default_options(struct f2fs_sb_info *sbi) >>> set_opt(sbi, MERGE_CHECKPOINT); >>> F2FS_OPTION(sbi).unusable_cap = 0; >>> sbi->sb->s_flags |= SB_LAZYTIME; >>> - if (!f2fs_is_readonly(sbi)) >>> + if (!f2fs_readonly(sbi->sb) && !f2fs_dev_is_readonly(sbi)) >>> set_opt(sbi, FLUSH_MERGE); >>> if (f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi)) >>> set_opt(sbi, DISCARD); >>> -- >>> 2.36.1 > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On 04/11, Chao Yu wrote: > On 2023/4/10 10:19, Chao Yu wrote: > > On 2023/4/4 1:59, Jaegeuk Kim wrote: > > > On 04/02, Chao Yu wrote: > > > > This patch does below changes: > > > > > > > > - If f2fs enables readonly feature or device is readonly, allow to > > > > mount readonly mode only > > > > - Introduce f2fs_dev_is_readonly() to indicate whether image or device > > > > is readonly > > > > - remove unnecessary f2fs_hw_is_readonly() in f2fs_write_checkpoint() > > > > and f2fs_convert_inline_inode() > > > > - enable FLUSH_MERGE only if f2fs is mounted as rw and image/device > > > > is writable > > > > Jaegeuk, > > > > Please drop this patch, and help to check below one, thanks. :) > > > > f2fs: use f2fs_hw_is_readonly() instead of bdev_read_only() > > Oh, I didn't make it clear. > > I found you applied both v1 and v2 of this patch. Yeah, I was keeping the fixes locally. Please check again. > > So I mean how about keeping v2: > > [PATCH v2] f2fs: fix to check readonly condition correctly > https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev-test&id=2bce08d26c0fe3a4ce1cca03ee0ea0c59d98c0b2 > > and apply > > [PATCH] f2fs: use f2fs_hw_is_readonly() instead of bdev_read_only() > https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev-test&id=7ec21ff2e8f521842938415af741fc72f39c0135 > > to instead > > [PATCH] f2fs: fix to check readonly condition correctly > https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev-test&id=66eb18a8b892afe085097747521fb280a1577ea4 > > Thanks, > > > > > Thanks, > > > > > > > > What is the problem to solve here? > > > > > > > > > > > Signed-off-by: Chao Yu <chao@kernel.org> > > > > --- > > > > fs/f2fs/checkpoint.c | 2 +- > > > > fs/f2fs/f2fs.h | 10 +++++----- > > > > fs/f2fs/inline.c | 3 +-- > > > > fs/f2fs/super.c | 13 +++++++------ > > > > 4 files changed, 14 insertions(+), 14 deletions(-) > > > > > > > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > > > > index 8e1db5752fff..1eef597ed393 100644 > > > > --- a/fs/f2fs/checkpoint.c > > > > +++ b/fs/f2fs/checkpoint.c > > > > @@ -1604,7 +1604,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) > > > > unsigned long long ckpt_ver; > > > > int err = 0; > > > > - if (f2fs_readonly(sbi->sb) || f2fs_hw_is_readonly(sbi)) > > > > + if (f2fs_readonly(sbi->sb)) > > > > return -EROFS; > > > > if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) { > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > > > index 2d4a7ef62537..7de95133478a 100644 > > > > --- a/fs/f2fs/f2fs.h > > > > +++ b/fs/f2fs/f2fs.h > > > > @@ -4446,6 +4446,11 @@ static inline bool f2fs_hw_is_readonly(struct f2fs_sb_info *sbi) > > > > return false; > > > > } > > > > +static inline bool f2fs_dev_is_readonly(struct f2fs_sb_info *sbi) > > > > +{ > > > > + return f2fs_sb_has_readonly(sbi) || f2fs_hw_is_readonly(sbi); > > > > +} > > > > + > > > > static inline bool f2fs_lfs_mode(struct f2fs_sb_info *sbi) > > > > { > > > > return F2FS_OPTION(sbi).fs_mode == FS_MODE_LFS; > > > > @@ -4546,11 +4551,6 @@ static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs, > > > > } > > > > } > > > > -static inline bool f2fs_is_readonly(struct f2fs_sb_info *sbi) > > > > -{ > > > > - return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb); > > > > -} > > > > - > > > > #define EFSBADCRC EBADMSG /* Bad CRC detected */ > > > > #define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */ > > > > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c > > > > index 72269e7efd26..2c36f2dc2317 100644 > > > > --- a/fs/f2fs/inline.c > > > > +++ b/fs/f2fs/inline.c > > > > @@ -203,8 +203,7 @@ int f2fs_convert_inline_inode(struct inode *inode) > > > > struct page *ipage, *page; > > > > int err = 0; > > > > - if (!f2fs_has_inline_data(inode) || > > > > - f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb)) > > > > + if (!f2fs_has_inline_data(inode) || f2fs_readonly(sbi->sb)) > > > > return 0; > > > > err = f2fs_dquot_initialize(inode); > > > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > > > > index d016f398fcad..db7649010c12 100644 > > > > --- a/fs/f2fs/super.c > > > > +++ b/fs/f2fs/super.c > > > > @@ -1382,15 +1382,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) > > > > return -EINVAL; > > > > } > > > > - if (f2fs_is_readonly(sbi) && test_opt(sbi, FLUSH_MERGE)) { > > > > + if (f2fs_dev_is_readonly(sbi) && !f2fs_readonly(sbi->sb)) { > > > > + f2fs_err(sbi, "Allow to mount readonly mode only"); > > > > + return -EROFS; > > > > + } > > > > + > > > > + if (f2fs_readonly(sbi->sb) && test_opt(sbi, FLUSH_MERGE)) { > > > > f2fs_err(sbi, "FLUSH_MERGE not compatible with readonly mode"); > > > > return -EINVAL; > > > > } > > > > - if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) { > > > > - f2fs_err(sbi, "Allow to mount readonly mode only"); > > > > - return -EROFS; > > > > - } > > > > return 0; > > > > } > > > > @@ -2122,7 +2123,7 @@ static void default_options(struct f2fs_sb_info *sbi) > > > > set_opt(sbi, MERGE_CHECKPOINT); > > > > F2FS_OPTION(sbi).unusable_cap = 0; > > > > sbi->sb->s_flags |= SB_LAZYTIME; > > > > - if (!f2fs_is_readonly(sbi)) > > > > + if (!f2fs_readonly(sbi->sb) && !f2fs_dev_is_readonly(sbi)) > > > > set_opt(sbi, FLUSH_MERGE); > > > > if (f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi)) > > > > set_opt(sbi, DISCARD); > > > > -- > > > > 2.36.1 > > > > > > _______________________________________________ > > Linux-f2fs-devel mailing list > > Linux-f2fs-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index 8e1db5752fff..1eef597ed393 100644 --- a/fs/f2fs/checkpoint.c +++ b/fs/f2fs/checkpoint.c @@ -1604,7 +1604,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) unsigned long long ckpt_ver; int err = 0; - if (f2fs_readonly(sbi->sb) || f2fs_hw_is_readonly(sbi)) + if (f2fs_readonly(sbi->sb)) return -EROFS; if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) { diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 2d4a7ef62537..7de95133478a 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -4446,6 +4446,11 @@ static inline bool f2fs_hw_is_readonly(struct f2fs_sb_info *sbi) return false; } +static inline bool f2fs_dev_is_readonly(struct f2fs_sb_info *sbi) +{ + return f2fs_sb_has_readonly(sbi) || f2fs_hw_is_readonly(sbi); +} + static inline bool f2fs_lfs_mode(struct f2fs_sb_info *sbi) { return F2FS_OPTION(sbi).fs_mode == FS_MODE_LFS; @@ -4546,11 +4551,6 @@ static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs, } } -static inline bool f2fs_is_readonly(struct f2fs_sb_info *sbi) -{ - return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb); -} - #define EFSBADCRC EBADMSG /* Bad CRC detected */ #define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */ diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c index 72269e7efd26..2c36f2dc2317 100644 --- a/fs/f2fs/inline.c +++ b/fs/f2fs/inline.c @@ -203,8 +203,7 @@ int f2fs_convert_inline_inode(struct inode *inode) struct page *ipage, *page; int err = 0; - if (!f2fs_has_inline_data(inode) || - f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb)) + if (!f2fs_has_inline_data(inode) || f2fs_readonly(sbi->sb)) return 0; err = f2fs_dquot_initialize(inode); diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index d016f398fcad..db7649010c12 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -1382,15 +1382,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) return -EINVAL; } - if (f2fs_is_readonly(sbi) && test_opt(sbi, FLUSH_MERGE)) { + if (f2fs_dev_is_readonly(sbi) && !f2fs_readonly(sbi->sb)) { + f2fs_err(sbi, "Allow to mount readonly mode only"); + return -EROFS; + } + + if (f2fs_readonly(sbi->sb) && test_opt(sbi, FLUSH_MERGE)) { f2fs_err(sbi, "FLUSH_MERGE not compatible with readonly mode"); return -EINVAL; } - if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) { - f2fs_err(sbi, "Allow to mount readonly mode only"); - return -EROFS; - } return 0; } @@ -2122,7 +2123,7 @@ static void default_options(struct f2fs_sb_info *sbi) set_opt(sbi, MERGE_CHECKPOINT); F2FS_OPTION(sbi).unusable_cap = 0; sbi->sb->s_flags |= SB_LAZYTIME; - if (!f2fs_is_readonly(sbi)) + if (!f2fs_readonly(sbi->sb) && !f2fs_dev_is_readonly(sbi)) set_opt(sbi, FLUSH_MERGE); if (f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi)) set_opt(sbi, DISCARD);