Hello!
Since reiserfs_remount can be called without BKL held, we better take BKL in there.
Please apply the below patch. It is against 2.6.0-test2
Bye,
Oleg
===== fs/reiserfs/super.c 1.66 vs edited =====
--- 1.66/fs/reiserfs/super.c Sat Jun 21 00:16:06 2003
+++ edited/fs/reiserfs/super.c Tue Aug 5 12:22:10 2003
@@ -761,6 +761,7 @@
if (!reiserfs_parse_options(s, arg, &mount_options, &blocks, NULL))
return -EINVAL;
+ reiserfs_write_lock(s);
handle_attrs(s);
/* Add options that are safe here */
@@ -778,17 +779,22 @@
if(blocks) {
int rc = reiserfs_resize(s, blocks);
- if (rc != 0)
+ if (rc != 0) {
+ reiserfs_write_unlock(s);
return rc;
+ }
}
if (*mount_flags & MS_RDONLY) {
/* remount read-only */
- if (s->s_flags & MS_RDONLY)
+ if (s->s_flags & MS_RDONLY) {
/* it is read-only already */
+ reiserfs_write_unlock(s);
return 0;
+ }
/* try to remount file system with read-only permissions */
if (sb_umount_state(rs) == REISERFS_VALID_FS || REISERFS_SB(s)->s_mount_state != REISERFS_VALID_FS) {
+ reiserfs_write_unlock(s);
return 0;
}
@@ -800,8 +806,10 @@
s->s_dirt = 0;
} else {
/* remount read-write */
- if (!(s->s_flags & MS_RDONLY))
+ if (!(s->s_flags & MS_RDONLY)) {
+ reiserfs_write_unlock(s);
return 0; /* We are read-write already */
+ }
REISERFS_SB(s)->s_mount_state = sb_umount_state(rs) ;
s->s_flags &= ~MS_RDONLY ; /* now it is safe to call journal_begin */
@@ -824,6 +832,7 @@
if (!( *mount_flags & MS_RDONLY ) )
finish_unfinished( s );
+ reiserfs_write_unlock(s);
return 0;
}
Hello!
On Wed, Aug 06, 2003 at 10:28:13AM -0700, Mike Fedyk wrote:
> > Since reiserfs_remount can be called without BKL held, we better take BKL in there.
> > Please apply the below patch. It is against 2.6.0-test2
> is the BKL in reiserfs_write_unlock()?
Yes.
> Do we need to be adding more BKL usage, instead of the same or less at this
> point?
Reiserfs needs BKL for it's journal operations. This is not "more",
for some time BKL was taken in the VFS, then whoever removed that,
forgot to propagate BKL down to actual fs methods that need the BKL.
Bye,
Oleg
On Wed, Aug 06, 2003 at 01:38:58PM +0400, Oleg Drokin wrote:
> Hello!
>
> Since reiserfs_remount can be called without BKL held, we better take BKL in there.
> Please apply the below patch. It is against 2.6.0-test2
is the BKL in reiserfs_write_unlock()?
Do we need to be adding more BKL usage, instead of the same or less at this
point?
Hello!
On Thu, Aug 07, 2003 at 05:24:43PM +0400, Hans Reiser wrote:
> >Reiserfs needs BKL for it's journal operations. This is not "more",
> >for some time BKL was taken in the VFS, then whoever removed that,
> >forgot to propagate BKL down to actual fs methods that need the BKL.
> Is it known who removed it?
I think it was Andrew. At least this new emergency remount path without
BKL was introduced by his patch without any extra attribution.
Bye,
Oleg
Oleg Drokin wrote:
>Hello!
>
>On Wed, Aug 06, 2003 at 10:28:13AM -0700, Mike Fedyk wrote:
>
>
>>> Since reiserfs_remount can be called without BKL held, we better take BKL in there.
>>> Please apply the below patch. It is against 2.6.0-test2
>>>
>>>
>>is the BKL in reiserfs_write_unlock()?
>>
>>
>
>Yes.
>
>
>
>>Do we need to be adding more BKL usage, instead of the same or less at this
>>point?
>>
>>
>
>Reiserfs needs BKL for it's journal operations. This is not "more",
>for some time BKL was taken in the VFS, then whoever removed that,
>forgot to propagate BKL down to actual fs methods that need the BKL.
>
>Bye,
> Oleg
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
>
>
>
>
Is it known who removed it?
--
Hans
Oleg Drokin <[email protected]> wrote:
>
> Hello!
>
> On Thu, Aug 07, 2003 at 05:24:43PM +0400, Hans Reiser wrote:
>
> > >Reiserfs needs BKL for it's journal operations. This is not "more",
> > >for some time BKL was taken in the VFS, then whoever removed that,
> > >forgot to propagate BKL down to actual fs methods that need the BKL.
> > Is it known who removed it?
>
> I think it was Andrew. At least this new emergency remount path without
> BKL was introduced by his patch without any extra attribution.
Is that the only path? It appears that way to me.
The Locking document says that ->remoutn_fs is called under lock_kernel(),
so this should be fixed at the VFS level.
fs/super.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletion(-)
diff -puN fs/super.c~remount_fs-lock_kernel fs/super.c
--- 25/fs/super.c~remount_fs-lock_kernel 2003-08-07 08:17:38.000000000 -0700
+++ 25-akpm/fs/super.c 2003-08-07 08:23:42.000000000 -0700
@@ -507,8 +507,16 @@ static void do_emergency_remount(unsigne
sb->s_count++;
spin_unlock(&sb_lock);
down_read(&sb->s_umount);
- if (sb->s_root && sb->s_bdev && !(sb->s_flags & MS_RDONLY))
+ if (sb->s_root && sb->s_bdev && !(sb->s_flags & MS_RDONLY)) {
+ /*
+ * ->remount_fs needs lock_kernel().
+ *
+ * What lock protects sb->s_flags??
+ */
+ lock_kernel();
do_remount_sb(sb, MS_RDONLY, NULL, 1);
+ unlock_kernel();
+ }
drop_super(sb);
spin_lock(&sb_lock);
}
_
Hello!
On Thu, Aug 07, 2003 at 08:24:40AM -0700, Andrew Morton wrote:
> > I think it was Andrew. At least this new emergency remount path without
> > BKL was introduced by his patch without any extra attribution.
> Is that the only path? It appears that way to me.
> The Locking document says that ->remoutn_fs is called under lock_kernel(),
> so this should be fixed at the VFS level.
Hm. Indeed.
Bye,
Oleg