All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: viro@zeniv.linux.org.uk
Cc: linux-fsdevel@vger.kernel.org
Subject: [PATCH 1/2] push MS_RDONLY check from ->write_super into caller
Date: Wed, 6 May 2009 22:16:05 +0200	[thread overview]
Message-ID: <20090506201605.GA19043@lst.de> (raw)

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@lst.de>

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))

             reply	other threads:[~2009-05-06 20:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-06 20:16 Christoph Hellwig [this message]
2009-05-06 22:19 ` [PATCH 1/2] push MS_RDONLY check from ->write_super into caller Al Viro
2009-05-06 22:33   ` Al Viro
2009-05-07  6:15   ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090506201605.GA19043@lst.de \
    --to=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.