* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).