Add a central MS_RDONLY to sync_super instead of having it in the ->write_super methods (and even that not consistently). The other two callers are already protected: sync_filesystems has the same s_umount protected MS_RDONLY check and file_fsync can only be called on a writeable file descriptor. Also make sure to clear s_dirt if it was set on a read-only superblock to avoid calling into these filesystems again and again. Signed-off-by: Christoph Hellwig <hch@xxxxxx> Index: vfs-2.6/fs/affs/super.c =================================================================== --- vfs-2.6.orig/fs/affs/super.c 2009-05-06 21:12:59.419666674 +0200 +++ vfs-2.6/fs/affs/super.c 2009-05-06 21:21:06.586663246 +0200 @@ -54,18 +54,15 @@ affs_write_super(struct super_block *sb) int clean = 2; struct affs_sb_info *sbi = AFFS_SB(sb); - if (!(sb->s_flags & MS_RDONLY)) { - // if (sbi->s_bitmap[i].bm_bh) { - // if (buffer_dirty(sbi->s_bitmap[i].bm_bh)) { - // clean = 0; - AFFS_ROOT_TAIL(sb, sbi->s_root_bh)->bm_flag = cpu_to_be32(clean); - secs_to_datestamp(get_seconds(), - &AFFS_ROOT_TAIL(sb, sbi->s_root_bh)->disk_change); - affs_fix_checksum(sb, sbi->s_root_bh); - mark_buffer_dirty(sbi->s_root_bh); - sb->s_dirt = !clean; /* redo until bitmap synced */ - } else - sb->s_dirt = 0; + // if (sbi->s_bitmap[i].bm_bh) { + // if (buffer_dirty(sbi->s_bitmap[i].bm_bh)) { + // clean = 0; + AFFS_ROOT_TAIL(sb, sbi->s_root_bh)->bm_flag = cpu_to_be32(clean); + secs_to_datestamp(get_seconds(), + &AFFS_ROOT_TAIL(sb, sbi->s_root_bh)->disk_change); + affs_fix_checksum(sb, sbi->s_root_bh); + mark_buffer_dirty(sbi->s_root_bh); + sb->s_dirt = !clean; /* redo until bitmap synced */ pr_debug("AFFS: write_super() at %lu, clean=%d\n", get_seconds(), clean); } Index: vfs-2.6/fs/bfs/inode.c =================================================================== --- vfs-2.6.orig/fs/bfs/inode.c 2009-05-06 21:12:59.432658761 +0200 +++ vfs-2.6/fs/bfs/inode.c 2009-05-06 21:14:22.613661552 +0200 @@ -219,7 +219,7 @@ static void bfs_put_super(struct super_b lock_kernel(); - if (s->s_dirt) + if (s->s_dirt && !(s->s_flags & MS_RDONLY)) bfs_write_super(s); brelse(info->si_sbh); @@ -253,8 +253,7 @@ static void bfs_write_super(struct super struct bfs_sb_info *info = BFS_SB(s); mutex_lock(&info->bfs_lock); - if (!(s->s_flags & MS_RDONLY)) - mark_buffer_dirty(info->si_sbh); + mark_buffer_dirty(info->si_sbh); s->s_dirt = 0; mutex_unlock(&info->bfs_lock); } Index: vfs-2.6/fs/ext2/super.c =================================================================== --- vfs-2.6.orig/fs/ext2/super.c 2009-05-06 21:12:59.465658841 +0200 +++ vfs-2.6/fs/ext2/super.c 2009-05-06 21:15:55.284661659 +0200 @@ -116,7 +116,7 @@ static void ext2_put_super (struct super lock_kernel(); - if (sb->s_dirt) + if (sb->s_dirt && !(sb->s_flags & MS_RDONLY)) ext2_write_super(sb); ext2_xattr_put_super(sb); @@ -1135,19 +1135,18 @@ void ext2_write_super (struct super_bloc { struct ext2_super_block * es; lock_kernel(); - if (!(sb->s_flags & MS_RDONLY)) { - es = EXT2_SB(sb)->s_es; - if (es->s_state & cpu_to_le16(EXT2_VALID_FS)) { - ext2_debug ("setting valid to 0\n"); - es->s_state &= cpu_to_le16(~EXT2_VALID_FS); - es->s_free_blocks_count = cpu_to_le32(ext2_count_free_blocks(sb)); - es->s_free_inodes_count = cpu_to_le32(ext2_count_free_inodes(sb)); - es->s_mtime = cpu_to_le32(get_seconds()); - ext2_sync_super(sb, es); - } else - ext2_commit_super (sb, es); - } + es = EXT2_SB(sb)->s_es; + + if (es->s_state & cpu_to_le16(EXT2_VALID_FS)) { + ext2_debug ("setting valid to 0\n"); + es->s_state &= cpu_to_le16(~EXT2_VALID_FS); + es->s_free_blocks_count = cpu_to_le32(ext2_count_free_blocks(sb)); + es->s_free_inodes_count = cpu_to_le32(ext2_count_free_inodes(sb)); + es->s_mtime = cpu_to_le32(get_seconds()); + ext2_sync_super(sb, es); + } else + ext2_commit_super (sb, es); sb->s_dirt = 0; unlock_kernel(); } Index: vfs-2.6/fs/fat/inode.c =================================================================== --- vfs-2.6.orig/fs/fat/inode.c 2009-05-06 21:12:59.548659280 +0200 +++ vfs-2.6/fs/fat/inode.c 2009-05-06 21:16:42.422661350 +0200 @@ -442,9 +442,7 @@ static void fat_clear_inode(struct inode static void fat_write_super(struct super_block *sb) { sb->s_dirt = 0; - - if (!(sb->s_flags & MS_RDONLY)) - fat_clusters_flush(sb); + fat_clusters_flush(sb); } static void fat_put_super(struct super_block *sb) @@ -453,7 +451,7 @@ static void fat_put_super(struct super_b lock_kernel(); - if (sb->s_dirt) + if (sb->s_dirt && !(sb->s_flags & MS_RDONLY)) fat_write_super(sb); if (sbi->nls_disk) { Index: vfs-2.6/fs/hfs/super.c =================================================================== --- vfs-2.6.orig/fs/hfs/super.c 2009-05-06 21:12:59.562658897 +0200 +++ vfs-2.6/fs/hfs/super.c 2009-05-06 21:17:17.361661811 +0200 @@ -50,8 +50,7 @@ MODULE_LICENSE("GPL"); static void hfs_write_super(struct super_block *sb) { sb->s_dirt = 0; - if (sb->s_flags & MS_RDONLY) - return; + /* sync everything to the buffers */ hfs_mdb_commit(sb); } @@ -67,7 +66,7 @@ static void hfs_put_super(struct super_b { lock_kernel(); - if (sb->s_dirt) + if (sb->s_dirt && !(sb->s_flags & MS_RDONLY)) hfs_write_super(sb); hfs_mdb_close(sb); /* release the MDB's resources */ Index: vfs-2.6/fs/hfsplus/super.c =================================================================== --- vfs-2.6.orig/fs/hfsplus/super.c 2009-05-06 21:12:59.611659198 +0200 +++ vfs-2.6/fs/hfsplus/super.c 2009-05-06 21:18:07.448661459 +0200 @@ -158,9 +158,6 @@ static void hfsplus_write_super(struct s dprint(DBG_SUPER, "hfsplus_write_super\n"); sb->s_dirt = 0; - if (sb->s_flags & MS_RDONLY) - /* warn? */ - return; vhdr->free_blocks = cpu_to_be32(HFSPLUS_SB(sb).free_blocks); vhdr->next_alloc = cpu_to_be32(HFSPLUS_SB(sb).next_alloc); @@ -202,8 +199,9 @@ static void hfsplus_put_super(struct sup lock_kernel(); - if (sb->s_dirt) + if (sb->s_dirt && !(sb->s_flags & MS_RDONLY)) hfsplus_write_super(sb); + if (!(sb->s_flags & MS_RDONLY) && HFSPLUS_SB(sb).s_vhdr) { struct hfsplus_vh *vhdr = HFSPLUS_SB(sb).s_vhdr; Index: vfs-2.6/fs/jffs2/fs.c =================================================================== --- vfs-2.6.orig/fs/jffs2/fs.c 2009-05-06 21:18:29.645659579 +0200 +++ vfs-2.6/fs/jffs2/fs.c 2009-05-06 21:18:36.248661832 +0200 @@ -407,9 +407,6 @@ void jffs2_write_super (struct super_blo struct jffs2_sb_info *c = JFFS2_SB_INFO(sb); sb->s_dirt = 0; - if (sb->s_flags & MS_RDONLY) - return; - D1(printk(KERN_DEBUG "jffs2_write_super()\n")); jffs2_garbage_collect_trigger(c); jffs2_erase_pending_blocks(c, 0); Index: vfs-2.6/fs/jffs2/super.c =================================================================== --- vfs-2.6.orig/fs/jffs2/super.c 2009-05-06 21:12:59.669659039 +0200 +++ vfs-2.6/fs/jffs2/super.c 2009-05-06 21:18:47.434661682 +0200 @@ -176,7 +176,7 @@ static void jffs2_put_super (struct supe lock_kernel(); - if (sb->s_dirt) + if (sb->s_dirt&& !(sb->s_flags & MS_RDONLY)) jffs2_write_super(sb); mutex_lock(&c->alloc_sem); Index: vfs-2.6/fs/nilfs2/super.c =================================================================== --- vfs-2.6.orig/fs/nilfs2/super.c 2009-05-06 21:12:59.706659768 +0200 +++ vfs-2.6/fs/nilfs2/super.c 2009-05-06 21:19:52.668661692 +0200 @@ -318,7 +318,7 @@ static void nilfs_put_super(struct super lock_kernel(); - if (sb->s_dirt) + if (sb->s_dirt && !(sb->s_flags & MS_RDONLY)) nilfs_write_super(sb); nilfs_detach_segment_constructor(sbi); @@ -368,21 +368,21 @@ static void nilfs_write_super(struct sup { struct nilfs_sb_info *sbi = NILFS_SB(sb); struct the_nilfs *nilfs = sbi->s_nilfs; + struct nilfs_super_block **sbp = nilfs->ns_sbp; + int dupsb; + u64 t; down_write(&nilfs->ns_sem); - if (!(sb->s_flags & MS_RDONLY)) { - struct nilfs_super_block **sbp = nilfs->ns_sbp; - u64 t = get_seconds(); - int dupsb; - - if (!nilfs_discontinued(nilfs) && t >= nilfs->ns_sbwtime[0] && - t < nilfs->ns_sbwtime[0] + NILFS_SB_FREQ) { - up_write(&nilfs->ns_sem); - return; - } - dupsb = sbp[1] && t > nilfs->ns_sbwtime[1] + NILFS_ALTSB_FREQ; - nilfs_commit_super(sbi, dupsb); + t = get_seconds(); + + if (!nilfs_discontinued(nilfs) && t >= nilfs->ns_sbwtime[0] && + t < nilfs->ns_sbwtime[0] + NILFS_SB_FREQ) { + up_write(&nilfs->ns_sem); + return; } + dupsb = sbp[1] && t > nilfs->ns_sbwtime[1] + NILFS_ALTSB_FREQ; + nilfs_commit_super(sbi, dupsb); + sb->s_dirt = 0; up_write(&nilfs->ns_sem); } Index: vfs-2.6/fs/super.c =================================================================== --- vfs-2.6.orig/fs/super.c 2009-05-06 21:12:18.139659181 +0200 +++ vfs-2.6/fs/super.c 2009-05-06 21:24:41.541661530 +0200 @@ -422,8 +422,20 @@ restart: down_read(&sb->s_umount); lock_super(sb); - if (sb->s_root && sb->s_dirt) - sb->s_op->write_super(sb); + if (sb->s_root) { + /* + * Avoid loops if a filesystem left s_dirt + * set after remount r/o. + * + * Might be worth an audit that it always gets + * cleared and then be turned into WARN_ON. + */ + if (sb->s_flags & MS_RDONLY) + sb->s_dirt = 0; + + if (sb->s_dirt) + sb->s_op->write_super(sb); + } unlock_super(sb); up_read(&sb->s_umount); Index: vfs-2.6/fs/sysv/inode.c =================================================================== --- vfs-2.6.orig/fs/sysv/inode.c 2009-05-06 21:12:59.891659572 +0200 +++ vfs-2.6/fs/sysv/inode.c 2009-05-06 21:20:24.210661655 +0200 @@ -38,9 +38,6 @@ static void sysv_write_super(struct supe unsigned long time = get_seconds(), old_time; lock_kernel(); - if (sb->s_flags & MS_RDONLY) - goto clean; - /* * If we are going to write out the super block, * then attach current time stamp. @@ -53,7 +50,7 @@ static void sysv_write_super(struct supe *sbi->s_sb_time = cpu_to_fs32(sbi, time); mark_buffer_dirty(sbi->s_bh2); } -clean: + sb->s_dirt = 0; unlock_kernel(); } @@ -76,7 +73,7 @@ static void sysv_put_super(struct super_ lock_kernel(); - if (sb->s_dirt) + if (sb->s_dirt && !(sb->s_flags & MS_RDONLY)) sysv_write_super(sb); if (!(sb->s_flags & MS_RDONLY)) { Index: vfs-2.6/fs/ufs/super.c =================================================================== --- vfs-2.6.orig/fs/ufs/super.c 2009-05-06 21:12:59.952659235 +0200 +++ vfs-2.6/fs/ufs/super.c 2009-05-06 21:21:04.195661283 +0200 @@ -1138,15 +1138,14 @@ static void ufs_write_super(struct super usb1 = ubh_get_usb_first(uspi); usb3 = ubh_get_usb_third(uspi); - if (!(sb->s_flags & MS_RDONLY)) { - usb1->fs_time = cpu_to_fs32(sb, get_seconds()); - if ((flags & UFS_ST_MASK) == UFS_ST_SUN - || (flags & UFS_ST_MASK) == UFS_ST_SUNOS - || (flags & UFS_ST_MASK) == UFS_ST_SUNx86) - ufs_set_fs_state(sb, usb1, usb3, - UFS_FSOK - fs32_to_cpu(sb, usb1->fs_time)); - ufs_put_cstotal(sb); - } + usb1->fs_time = cpu_to_fs32(sb, get_seconds()); + if ((flags & UFS_ST_MASK) == UFS_ST_SUN || + (flags & UFS_ST_MASK) == UFS_ST_SUNOS || + (flags & UFS_ST_MASK) == UFS_ST_SUNx86) + ufs_set_fs_state(sb, usb1, usb3, + UFS_FSOK - fs32_to_cpu(sb, usb1->fs_time)); + ufs_put_cstotal(sb); + sb->s_dirt = 0; UFSD("EXIT\n"); unlock_kernel(); @@ -1158,7 +1157,7 @@ static void ufs_put_super(struct super_b UFSD("ENTER\n"); - if (sb->s_dirt) + if (sb->s_dirt && !(sb->s_flags & MS_RDONLY)) ufs_write_super(sb); if (!(sb->s_flags & MS_RDONLY)) -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html