* [RFC PATCH 2/5 v3] Btrfs: avoid transaction stuff when btrfs is readonly @ 2010-12-03 8:16 liubo 2010-12-15 8:45 ` Yan, Zheng 0 siblings, 1 reply; 6+ messages in thread From: liubo @ 2010-12-03 8:16 UTC (permalink / raw) To: Linux Btrfs; +Cc: Josef Bacik, Tsutomu Itoh, Yan, Zheng, Wenyi Liu, Mike Fedyk When the filesystem is readonly, avoid transaction stuff by checking MS_RDONLY at start transaction time. Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> --- fs/btrfs/transaction.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index 1fffbc0..14a597d 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -181,6 +181,9 @@ static struct btrfs_trans_handle *start_transaction(struct btrfs_root *root, struct btrfs_trans_handle *h; struct btrfs_transaction *cur_trans; int ret; + + if (root->fs_info->sb->s_flags & MS_RDONLY) + return ERR_PTR(-EROFS); again: h = kmem_cache_alloc(btrfs_trans_handle_cachep, GFP_NOFS); if (!h) -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 2/5 v3] Btrfs: avoid transaction stuff when btrfs is readonly 2010-12-03 8:16 [RFC PATCH 2/5 v3] Btrfs: avoid transaction stuff when btrfs is readonly liubo @ 2010-12-15 8:45 ` Yan, Zheng 2010-12-15 9:12 ` liubo 0 siblings, 1 reply; 6+ messages in thread From: Yan, Zheng @ 2010-12-15 8:45 UTC (permalink / raw) To: liubo Cc: Linux Btrfs, Josef Bacik, Tsutomu Itoh, Yan, Zheng, Wenyi Liu, Mike Fedyk On Fri, Dec 3, 2010 at 4:16 PM, liubo <liubo2009@cn.fujitsu.com> wrote: > When the filesystem is readonly, avoid transaction stuff by checking = MS_RDONLY > at start transaction time. > > Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> > --- > =A0fs/btrfs/transaction.c | =A0 =A03 +++ > =A01 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c > index 1fffbc0..14a597d 100644 > --- a/fs/btrfs/transaction.c > +++ b/fs/btrfs/transaction.c > @@ -181,6 +181,9 @@ static struct btrfs_trans_handle *start_transacti= on(struct btrfs_root *root, > =A0 =A0 =A0 =A0struct btrfs_trans_handle *h; > =A0 =A0 =A0 =A0struct btrfs_transaction *cur_trans; > =A0 =A0 =A0 =A0int ret; > + > + =A0 =A0 =A0 if (root->fs_info->sb->s_flags & MS_RDONLY) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return ERR_PTR(-EROFS); > =A0again: > =A0 =A0 =A0 =A0h =3D kmem_cache_alloc(btrfs_trans_handle_cachep, GFP_= NOFS); > =A0 =A0 =A0 =A0if (!h) There are cases that we need to start transaction when MS_RDONLY flag i= s set. =46or example, remount FS into read-only mode and log replay. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" = in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 2/5 v3] Btrfs: avoid transaction stuff when btrfs is readonly 2010-12-15 8:45 ` Yan, Zheng @ 2010-12-15 9:12 ` liubo 2010-12-15 16:03 ` Chris Mason 0 siblings, 1 reply; 6+ messages in thread From: liubo @ 2010-12-15 9:12 UTC (permalink / raw) To: Yan, Zheng Cc: Linux Btrfs, Josef Bacik, Tsutomu Itoh, Yan, Zheng, Wenyi Liu, Mike Fedyk On 12/15/2010 04:45 PM, Yan, Zheng wrote: > On Fri, Dec 3, 2010 at 4:16 PM, liubo <liubo2009@cn.fujitsu.com> wrote: >> When the filesystem is readonly, avoid transaction stuff by checking MS_RDONLY >> at start transaction time. >> >> Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> >> --- >> fs/btrfs/transaction.c | 3 +++ >> 1 files changed, 3 insertions(+), 0 deletions(-) >> >> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c >> index 1fffbc0..14a597d 100644 >> --- a/fs/btrfs/transaction.c >> +++ b/fs/btrfs/transaction.c >> @@ -181,6 +181,9 @@ static struct btrfs_trans_handle *start_transaction(struct btrfs_root *root, >> struct btrfs_trans_handle *h; >> struct btrfs_transaction *cur_trans; >> int ret; >> + >> + if (root->fs_info->sb->s_flags & MS_RDONLY) >> + return ERR_PTR(-EROFS); >> again: >> h = kmem_cache_alloc(btrfs_trans_handle_cachep, GFP_NOFS); >> if (!h) > > There are cases that we need to start transaction when MS_RDONLY flag is set. > For example, remount FS into read-only mode and log replay. However, is it weird to make changes to disk as fs is in readonly state? IMO, btrfs needs to limit the use of these "disk-change while readonly" cases, as it is not what readonly means. Since it has been here, we can bypass readonly in those cases(as I did in the 5th patch): ... flags = sb->s_flags; if (sb->s_flags & MS_RDONLY) sb->s_flags &= ~MS_RDONLY remount() sb->s_flags = flags; ... thanks, Liu Bo > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 2/5 v3] Btrfs: avoid transaction stuff when btrfs is readonly 2010-12-15 9:12 ` liubo @ 2010-12-15 16:03 ` Chris Mason 2010-12-15 16:05 ` Josef Bacik 2010-12-16 1:35 ` liubo 0 siblings, 2 replies; 6+ messages in thread From: Chris Mason @ 2010-12-15 16:03 UTC (permalink / raw) To: liubo Cc: Yan, Zheng, Linux Btrfs, Josef Bacik, Tsutomu Itoh, Yan, Zheng, Wenyi Liu, Mike Fedyk Excerpts from liubo's message of 2010-12-15 04:12:14 -0500: > On 12/15/2010 04:45 PM, Yan, Zheng wrote: > > On Fri, Dec 3, 2010 at 4:16 PM, liubo <liubo2009@cn.fujitsu.com> wrote: > >> When the filesystem is readonly, avoid transaction stuff by checking MS_RDONLY > >> at start transaction time. > >> > >> Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> > >> --- > >> fs/btrfs/transaction.c | 3 +++ > >> 1 files changed, 3 insertions(+), 0 deletions(-) > >> > >> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c > >> index 1fffbc0..14a597d 100644 > >> --- a/fs/btrfs/transaction.c > >> +++ b/fs/btrfs/transaction.c > >> @@ -181,6 +181,9 @@ static struct btrfs_trans_handle *start_transaction(struct btrfs_root *root, > >> struct btrfs_trans_handle *h; > >> struct btrfs_transaction *cur_trans; > >> int ret; > >> + > >> + if (root->fs_info->sb->s_flags & MS_RDONLY) > >> + return ERR_PTR(-EROFS); > >> again: > >> h = kmem_cache_alloc(btrfs_trans_handle_cachep, GFP_NOFS); > >> if (!h) > > > > There are cases that we need to start transaction when MS_RDONLY flag is set. > > For example, remount FS into read-only mode and log replay. > > However, is it weird to make changes to disk as fs is in readonly state? > IMO, btrfs needs to limit the use of these "disk-change while readonly" cases, > as it is not what readonly means. reiserfs and ext3 at least have always done this. Log replay is required even when the FS is readonly. > > Since it has been here, we can bypass readonly in those cases(as I did in the 5th patch): > > ... > flags = sb->s_flags; > if (sb->s_flags & MS_RDONLY) > sb->s_flags &= ~MS_RDONLY I think we should have a dedicated flag to reflect a filesystem that is forced readonly, and check that flag instead. -chris ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 2/5 v3] Btrfs: avoid transaction stuff when btrfs is readonly 2010-12-15 16:03 ` Chris Mason @ 2010-12-15 16:05 ` Josef Bacik 2010-12-16 1:35 ` liubo 1 sibling, 0 replies; 6+ messages in thread From: Josef Bacik @ 2010-12-15 16:05 UTC (permalink / raw) To: Chris Mason Cc: liubo, Yan, Zheng, Linux Btrfs, Josef Bacik, Tsutomu Itoh, Yan, Zheng, Wenyi Liu, Mike Fedyk On Wed, Dec 15, 2010 at 11:03:46AM -0500, Chris Mason wrote: > Excerpts from liubo's message of 2010-12-15 04:12:14 -0500: > > On 12/15/2010 04:45 PM, Yan, Zheng wrote: > > > On Fri, Dec 3, 2010 at 4:16 PM, liubo <liubo2009@cn.fujitsu.com> wrote: > > >> When the filesystem is readonly, avoid transaction stuff by checking MS_RDONLY > > >> at start transaction time. > > >> > > >> Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> > > >> --- > > >> fs/btrfs/transaction.c | 3 +++ > > >> 1 files changed, 3 insertions(+), 0 deletions(-) > > >> > > >> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c > > >> index 1fffbc0..14a597d 100644 > > >> --- a/fs/btrfs/transaction.c > > >> +++ b/fs/btrfs/transaction.c > > >> @@ -181,6 +181,9 @@ static struct btrfs_trans_handle *start_transaction(struct btrfs_root *root, > > >> struct btrfs_trans_handle *h; > > >> struct btrfs_transaction *cur_trans; > > >> int ret; > > >> + > > >> + if (root->fs_info->sb->s_flags & MS_RDONLY) > > >> + return ERR_PTR(-EROFS); > > >> again: > > >> h = kmem_cache_alloc(btrfs_trans_handle_cachep, GFP_NOFS); > > >> if (!h) > > > > > > There are cases that we need to start transaction when MS_RDONLY flag is set. > > > For example, remount FS into read-only mode and log replay. > > > > However, is it weird to make changes to disk as fs is in readonly state? > > IMO, btrfs needs to limit the use of these "disk-change while readonly" cases, > > as it is not what readonly means. > > reiserfs and ext3 at least have always done this. Log replay is > required even when the FS is readonly. > Just make sure the underlying disk isn't read only, we had problems with this on ext3 a bit ago. Thanks, Josef ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 2/5 v3] Btrfs: avoid transaction stuff when btrfs is readonly 2010-12-15 16:03 ` Chris Mason 2010-12-15 16:05 ` Josef Bacik @ 2010-12-16 1:35 ` liubo 1 sibling, 0 replies; 6+ messages in thread From: liubo @ 2010-12-16 1:35 UTC (permalink / raw) To: Chris Mason Cc: Yan, Zheng, Linux Btrfs, Josef Bacik, Tsutomu Itoh, Yan, Zheng, Wenyi Liu On 12/16/2010 12:03 AM, Chris Mason wrote: > Excerpts from liubo's message of 2010-12-15 04:12:14 -0500: >> On 12/15/2010 04:45 PM, Yan, Zheng wrote: >>> On Fri, Dec 3, 2010 at 4:16 PM, liubo <liubo2009@cn.fujitsu.com> wrote: >>>> When the filesystem is readonly, avoid transaction stuff by checking MS_RDONLY >>>> at start transaction time. >>>> >>>> Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> >>>> --- >>>> fs/btrfs/transaction.c | 3 +++ >>>> 1 files changed, 3 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c >>>> index 1fffbc0..14a597d 100644 >>>> --- a/fs/btrfs/transaction.c >>>> +++ b/fs/btrfs/transaction.c >>>> @@ -181,6 +181,9 @@ static struct btrfs_trans_handle *start_transaction(struct btrfs_root *root, >>>> struct btrfs_trans_handle *h; >>>> struct btrfs_transaction *cur_trans; >>>> int ret; >>>> + >>>> + if (root->fs_info->sb->s_flags & MS_RDONLY) >>>> + return ERR_PTR(-EROFS); >>>> again: >>>> h = kmem_cache_alloc(btrfs_trans_handle_cachep, GFP_NOFS); >>>> if (!h) >>> There are cases that we need to start transaction when MS_RDONLY flag is set. >>> For example, remount FS into read-only mode and log replay. >> However, is it weird to make changes to disk as fs is in readonly state? >> IMO, btrfs needs to limit the use of these "disk-change while readonly" cases, >> as it is not what readonly means. > > reiserfs and ext3 at least have always done this. Log replay is > required even when the FS is readonly. > My concern is: now we have a forced readonly FS, which is already broken, if we still write something to disk, would it become more broken? >> Since it has been here, we can bypass readonly in those cases(as I did in the 5th patch): >> >> ... >> flags = sb->s_flags; >> if (sb->s_flags & MS_RDONLY) >> sb->s_flags &= ~MS_RDONLY > > I think we should have a dedicated flag to reflect a filesystem that is > forced readonly, and check that flag instead. OK, we did have fs_state, a dedicated flag. thanks, Liu Bo > > -chris > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-12-16 1:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-12-03 8:16 [RFC PATCH 2/5 v3] Btrfs: avoid transaction stuff when btrfs is readonly liubo 2010-12-15 8:45 ` Yan, Zheng 2010-12-15 9:12 ` liubo 2010-12-15 16:03 ` Chris Mason 2010-12-15 16:05 ` Josef Bacik 2010-12-16 1:35 ` liubo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.