On Thu 08-05-08 15:44:27, Edward Shishkin wrote:
> Jan Kara wrote:
> > On Wed 30-04-08 22:13:35, Edward Shishkin wrote:
> >
> >> Jan Kara wrote:
> >>
> >>
> >>> Hi,
> >>>
> >>> I was just looking into reiserfs remount code and it seems it calls
> >>> finish_unfinished() whenever filesystem is remounted and isn't in
> >>> read-only
> >>> mode. Isn't this unnecessary? I thought finish_unfinished() is needed only
> >>> when we remount from read-only to read-write state for the first time...
> >>>
> >>>
> >> Perhaps, you are right (if we don't miss some points). But we need to keep
> >> a track
> >> of first successful remounts to rw state, and it requires a special flag in
> >> in-memory
> >> superblock (I don't see another way).
> >>
> >>
> >>> Well, I wouldn't mind the performance impact to remount that much
> >>>
> >>>
> >> yeah, definitely benchmarks lack "1000 remounts"statistics ;)
> >>
> >>
> >>> but it
> >>> would make my life with journaled quota a bit easier ;). Thanks for an
> >>> answer in advance.
> >>>
> > OK, so would you accept the patch below?
> >
> >
>
> I think it should work properly, however Linus doesn't like mysterious
> values.
> The attached patch might look better: it does the same things, but uses
> status flags.
Great. Thanks. The patch looks fine, only I think you've added some
whitespace noise before reiserfs_xattr_init() call...
Honza
> We need to call finish_unfinished() only when the filesystem is remounted
> read-write for the first time. This not only saves a few cycles on remount
> (rather irrelevant) but also makes reiserfs handle journaled quota on remount
> better - quota is the correctly reenabled on remount read-write.
> So:
> . add a field to the in-memory superblock for mount status flags;
> . keep a track of the first completed finish_unfinished().
>
> Signed-off-by: Edward Shishkin <edward.shishkin@xxxxxxxxx>
> ---
> linux-2.6.25-mm1/fs/reiserfs/inode.c | 2 +-
> linux-2.6.25-mm1/fs/reiserfs/super.c | 10 ++++++----
> linux-2.6.25-mm1/include/linux/reiserfs_fs_sb.h | 13 +++++++++----
> 3 files changed, 16 insertions(+), 9 deletions(-)
>
> --- linux-2.6.25-mm1/fs/reiserfs/inode.c.orig
> +++ linux-2.6.25-mm1/fs/reiserfs/inode.c
> @@ -1454,7 +1454,7 @@
> nlink==0: processing of open-unlinked and half-truncated files
> during mount (fs/reiserfs/super.c:finish_unfinished()). */
> if ((inode->i_nlink == 0) &&
> - !REISERFS_SB(inode->i_sb)->s_is_unlinked_ok) {
> + !(REISERFS_SB(inode->i_sb)->s_mnt_status_flags & UNLINKED_OK)) {
> reiserfs_warning(inode->i_sb,
> "vs-13075: reiserfs_read_locked_inode: "
> "dead inode read from disk %K. "
> --- linux-2.6.25-mm1/fs/reiserfs/super.c.orig
> +++ linux-2.6.25-mm1/fs/reiserfs/super.c
> @@ -189,7 +189,7 @@
> #endif
>
> done = 0;
> - REISERFS_SB(s)->s_is_unlinked_ok = 1;
> + REISERFS_SB(s)->s_mnt_status_flags |= UNLINKED_OK;
> while (!retval) {
> retval = search_item(s, &max_cpu_key, &path);
> if (retval != ITEM_NOT_FOUND) {
> @@ -298,7 +298,8 @@
> printk("done\n");
> done++;
> }
> - REISERFS_SB(s)->s_is_unlinked_ok = 0;
> + REISERFS_SB(s)->s_mnt_status_flags &= ~UNLINKED_OK;
> + REISERFS_SB(s)->s_mnt_status_flags |= UNFINISHED_OK;
>
> #ifdef CONFIG_QUOTA
> /* Turn quotas off */
> @@ -1256,8 +1257,9 @@
> s->s_dirt = 0;
>
> if (!(*mount_flags & MS_RDONLY)) {
> - finish_unfinished(s);
> - reiserfs_xattr_init(s, *mount_flags);
> + if (!(REISERFS_SB(s)->s_mnt_status_flags & UNFINISHED_OK))
> + finish_unfinished(s);
> + reiserfs_xattr_init(s, *mount_flags);
> }
>
> out_ok:
> --- linux-2.6.25-mm1/include/linux/reiserfs_fs_sb.h.orig
> +++ linux-2.6.25-mm1/include/linux/reiserfs_fs_sb.h
> @@ -341,6 +341,14 @@
> } reiserfs_proc_info_data_t;
> #endif
>
> +/* (re)mount status flags */
> +#define UNLINKED_OK 1 /* set up when it's ok for reiserfs_read_inode2()
> + * to read from disk inode with nlink==0.
> + * Currently this is only used during
> + * finish_unfinished() processing at mount time */
> +#define UNFINISHED_OK 2 /* indicates that finish_unfinished() was
> + * completed successfully */
> +
> /* reiserfs union of in-core super block data */
> struct reiserfs_sb_info {
> struct buffer_head *s_sbh; /* Buffer containing the super block */
> @@ -389,10 +397,7 @@
> int s_bmaps_without_search;
> int s_direct2indirect;
> int s_indirect2direct;
> - /* set up when it's ok for reiserfs_read_inode2() to read from
> - disk inode with nlink==0. Currently this is only used during
> - finish_unfinished() processing at mount time */
> - int s_is_unlinked_ok;
> + unsigned s_mnt_status_flags; /* these flags are set in (re)mount time */
> reiserfs_proc_info_data_t s_proc_info_data;
> struct proc_dir_entry *procdir;
> int reserved_blocks; /* amount of blocks reserved for further allocations */
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
