reiserfs: use open_bdev_excl

PATCH In review Christoph Hellwig 3 participants 7 messages Dec 26, 2007 – Apr 29, 2008 VFS

Share & tools

Christoph Hellwig

author

2007-12-26 15:31 UTC #0 lore raw

PATCH 1/1 reiserfs: use open_bdev_excl

Use the proper helper to open a blockdevice by name for filesystem use, this makes sure it's properly claimed (also added for open-by-number) and gets rid of the struct file abuse.

Tested by mounting a reiserfs filesystem with external journal.

Signed-off-by Christoph Hellwig <[email protected]>

fs/reiserfs/journal.c +23 ~~-27~~

Index: linux-2.6-xfs/fs/reiserfs/journal.c
===================================================================
@@ -2574,11 +2574,9 @@ static int release_journal_dev(struct su
 
 	result = 0;
 
-	if (journal->j_dev_file != NULL) {
-		result = filp_close(journal->j_dev_file, NULL);
-		journal->j_dev_file = NULL;
-		journal->j_dev_bd = NULL;
-	} else if (journal->j_dev_bd != NULL) {
+	if (journal->j_dev_bd != NULL) {
+		if (journal->j_dev_bd->bd_dev != super->s_dev)
+			bd_release(journal->j_dev_bd);
 		result = blkdev_put(journal->j_dev_bd);
 		journal->j_dev_bd = NULL;
 	}

@@ -2603,7 +2601,6 @@ static int journal_init_dev(struct super
 	result = 0;
 
 	journal->j_dev_bd = NULL;
-	journal->j_dev_file = NULL;
 	jdev = SB_ONDISK_JOURNAL_DEVICE(super) ?
 	    new_decode_dev(SB_ONDISK_JOURNAL_DEVICE(super)) : super->s_dev;

@@ -2620,35 +2617,34 @@ static int journal_init_dev(struct super
 					 "cannot init journal device '%s': %i",
 					 __bdevname(jdev, b), result);
 			return result;
-		} else if (jdev != super->s_dev)
+		} else if (jdev != super->s_dev) {
+			result = bd_claim(journal->j_dev_bd, journal);
+			if (result) {
+				blkdev_put(journal->j_dev_bd);
+				return result;
+			}
+
 			set_blocksize(journal->j_dev_bd, super->s_blocksize);
+		}
+
 		return 0;
 	}
 
-	journal->j_dev_file = filp_open(jdev_name, 0, 0);
-	if (!IS_ERR(journal->j_dev_file)) {
-		struct inode *jdev_inode = journal->j_dev_file->f_mapping->host;
-		if (!S_ISBLK(jdev_inode->i_mode)) {
-			reiserfs_warning(super, "journal_init_dev: '%s' is "
-					 "not a block device", jdev_name);
-			result = -ENOTBLK;
-			release_journal_dev(super, journal);
-		} else {
-			/* ok */
-			journal->j_dev_bd = I_BDEV(jdev_inode);
-			set_blocksize(journal->j_dev_bd, super->s_blocksize);
-			reiserfs_info(super,
-				      "journal_init_dev: journal device: %s\n",
-				      bdevname(journal->j_dev_bd, b));
-		}
-	} else {
-		result = PTR_ERR(journal->j_dev_file);
-		journal->j_dev_file = NULL;
+	journal->j_dev_bd = open_bdev_excl(jdev_name, 0, journal);
+	if (IS_ERR(journal->j_dev_bd)) {
+		result = PTR_ERR(journal->j_dev_bd);
+		journal->j_dev_bd = NULL;
 		reiserfs_warning(super,
 				 "journal_init_dev: Cannot open '%s': %i",
 				 jdev_name, result);
+		return result;
 	}
-	return result;
+
+	set_blocksize(journal->j_dev_bd, super->s_blocksize);
+	reiserfs_info(super,
+		      "journal_init_dev: journal device: %s\n",
+		      bdevname(journal->j_dev_bd, b));
+	return 0;
 }
 
 /**

include/linux/reiserfs_fs_sb.h +0 -1

Index: linux-2.6-xfs/include/linux/reiserfs_fs_sb.h
===================================================================
@@ -177,7 +177,6 @@ struct reiserfs_journal {
 	struct reiserfs_journal_cnode *j_last;	/* newest journal block */
 	struct reiserfs_journal_cnode *j_first;	/*  oldest journal block.  start here for traverse */
 
-	struct file *j_dev_file;
 	struct block_device *j_dev_bd;
 	int j_1st_reserved_block;	/* first block on s_dev of reserved area journal */

Andrew Morton

2008-02-07 05:16 UTC #2 lore raw

Quoted 9 lines · On Thu, 7 Feb 2008 05:45:13 +0100 Christoph Hellwig <[email protected]> wrote:

Quoted 5 lines · Christoph Hellwig
Use the proper helper to open a blockdevice by name for filesystem use, this makes sure it's properly claimed (also added for open-by-number) and gets rid of the struct file abuse.
Tested by mounting a reiserfs filesystem with external journal.
Folks, what do we do with this patch? It's been out for more than a month but didn't actually get picked up in any tree.

It's still sitting in the great pile of things I got sent over Christmas and haven't looked at yet. It didn't look like a bugfix.

Quoted 2 lines

I'd really like
to see this going in soon.

How come?

Christoph Hellwig

author

2008-02-07 05:19 UTC #3 lore raw

Quoted 7 lines · Andrew Morton

Quoted 5 lines · On Thu, 7 Feb 2008 05:45:13 +0100 Christoph Hellwig <[email protected]> wrote:
Quoted 4 lines · Christoph Hellwig
Use the proper helper to open a blockdevice by name for filesystem use, this makes sure it's properly claimed (also added for open-by-number) and gets rid of the struct file abuse.

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Tested by mounting a reiserfs filesystem with external journal.
Folks, what do we do with this patch? It's been out for more than a month but didn't actually get picked up in any tree.
It's still sitting in the great pile of things I got sent over Christmas and haven't looked at yet. It didn't look like a bugfix.

Well, it's fixing that reiserds doesn't bd_claim it's journal device so people can open the block device with O_EXCL against the documetned semantics. It's probably also fixing various corner cases that open_bdev_excl deals with and a hand-crafted filp_open doesn't.

Quoted 4 lines

Quoted 2 lines
I'd really like
to see this going in soon.
How come?

I just hate sitting on a stack of patches. Even more so if I'd like to see the public API removed in there eventually go away.

Christoph Hellwig

author

2008-04-29 18:13 UTC #4 lore raw

In reply to Christoph Hellwig

Quoted 5 lines · Christoph Hellwig

Use the proper helper to open a blockdevice by name for filesystem use, this makes sure it's properly claimed (also added for open-by-number) and gets rid of the struct file abuse.
Tested by mounting a reiserfs filesystem with external journal.

Any reason this still didn't get sent to Linus?

Quoted 92 lines

Signed-off-by Christoph Hellwig <[email protected]>

Index: linux-2.6-xfs/fs/reiserfs/journal.c
===================================================================

fs/reiserfs/journal.c +23 ~~-27~~

@@ -2574,11 +2574,9 @@ static int release_journal_dev(struct su
 
 	result = 0;
 
-	if (journal->j_dev_file != NULL) {
-		result = filp_close(journal->j_dev_file, NULL);
-		journal->j_dev_file = NULL;
-		journal->j_dev_bd = NULL;
-	} else if (journal->j_dev_bd != NULL) {
+	if (journal->j_dev_bd != NULL) {
+		if (journal->j_dev_bd->bd_dev != super->s_dev)
+			bd_release(journal->j_dev_bd);
 		result = blkdev_put(journal->j_dev_bd);
 		journal->j_dev_bd = NULL;
 	}

@@ -2603,7 +2601,6 @@ static int journal_init_dev(struct super
 	result = 0;
 
 	journal->j_dev_bd = NULL;
-	journal->j_dev_file = NULL;
 	jdev = SB_ONDISK_JOURNAL_DEVICE(super) ?
 	    new_decode_dev(SB_ONDISK_JOURNAL_DEVICE(super)) : super->s_dev;

@@ -2620,35 +2617,34 @@ static int journal_init_dev(struct super
 					 "cannot init journal device '%s': %i",
 					 __bdevname(jdev, b), result);
 			return result;
-		} else if (jdev != super->s_dev)
+		} else if (jdev != super->s_dev) {
+			result = bd_claim(journal->j_dev_bd, journal);
+			if (result) {
+				blkdev_put(journal->j_dev_bd);
+				return result;
+			}
+
 			set_blocksize(journal->j_dev_bd, super->s_blocksize);
+		}
+
 		return 0;
 	}
 
-	journal->j_dev_file = filp_open(jdev_name, 0, 0);
-	if (!IS_ERR(journal->j_dev_file)) {
-		struct inode *jdev_inode = journal->j_dev_file->f_mapping->host;
-		if (!S_ISBLK(jdev_inode->i_mode)) {
-			reiserfs_warning(super, "journal_init_dev: '%s' is "
-					 "not a block device", jdev_name);
-			result = -ENOTBLK;
-			release_journal_dev(super, journal);
-		} else {
-			/* ok */
-			journal->j_dev_bd = I_BDEV(jdev_inode);
-			set_blocksize(journal->j_dev_bd, super->s_blocksize);
-			reiserfs_info(super,
-				      "journal_init_dev: journal device: %s\n",
-				      bdevname(journal->j_dev_bd, b));
-		}
-	} else {
-		result = PTR_ERR(journal->j_dev_file);
-		journal->j_dev_file = NULL;
+	journal->j_dev_bd = open_bdev_excl(jdev_name, 0, journal);
+	if (IS_ERR(journal->j_dev_bd)) {
+		result = PTR_ERR(journal->j_dev_bd);
+		journal->j_dev_bd = NULL;
 		reiserfs_warning(super,
 				 "journal_init_dev: Cannot open '%s': %i",
 				 jdev_name, result);
+		return result;
 	}
-	return result;
+
+	set_blocksize(journal->j_dev_bd, super->s_blocksize);
+	reiserfs_info(super,
+		      "journal_init_dev: journal device: %s\n",
+		      bdevname(journal->j_dev_bd, b));
+	return 0;
 }
 
 /**

Index: linux-2.6-xfs/include/linux/reiserfs_fs_sb.h
===================================================================

include/linux/reiserfs_fs_sb.h struct reiserfs_journal { · line 182

Quoted 7 lines

include/linux/reiserfs_fs_sb.h +0 -1

@@ -177,7 +177,6 @@ struct reiserfs_journal {
 	struct reiserfs_journal_cnode *j_last;	/* newest journal block */
 	struct reiserfs_journal_cnode *j_first;	/*  oldest journal block.  start here for traverse */
 
-	struct file *j_dev_file;
 	struct block_device *j_dev_bd;
 	int j_1st_reserved_block;	/* first block on s_dev of reserved area journal */

---end quoted text---

Andrew Morton

2008-04-29 18:24 UTC #5 lore raw

Quoted 8 lines · On Tue, 29 Apr 2008 20:13:22 +0200 Christoph Hellwig <[email protected]> wrote:

Quoted 5 lines · Christoph Hellwig
Use the proper helper to open a blockdevice by name for filesystem use, this makes sure it's properly claimed (also added for open-by-number) and gets rid of the struct file abuse.
Tested by mounting a reiserfs filesystem with external journal.
Any reason this still didn't get sent to Linus?

no, I just filed it in the wrong place. It'll get there.

Edward Shishkin

Acked-by

2008-04-29 22:49 UTC #6 lore raw

In reply to Christoph Hellwig

Quoted 7 lines · Christoph Hellwig

Quoted 4 lines · Christoph Hellwig
Use the proper helper to open a blockdevice by name for filesystem use, this makes sure it's properly claimed (also added for open-by-number) and gets rid of the struct file abuse.

Quoted 11 lines

Quoted 3 lines
Tested by mounting a reiserfs filesystem with external journal.
Any reason this still didn't get sent to Linus?
Quoted 3 lines
Signed-off-by Christoph Hellwig <[email protected]>

Acked-by Edward Shishkin <[email protected]>

Quoted 105 lines

Quoted 90 lines

Index: linux-2.6-xfs/fs/reiserfs/journal.c
===================================================================

fs/reiserfs/journal.c +23 ~~-27~~

@@ -2574,11 +2574,9 @@ static int release_journal_dev(struct su
 
 	result = 0;
 
-	if (journal->j_dev_file != NULL) {
-		result = filp_close(journal->j_dev_file, NULL);
-		journal->j_dev_file = NULL;
-		journal->j_dev_bd = NULL;
-	} else if (journal->j_dev_bd != NULL) {
+	if (journal->j_dev_bd != NULL) {
+		if (journal->j_dev_bd->bd_dev != super->s_dev)
+			bd_release(journal->j_dev_bd);
 		result = blkdev_put(journal->j_dev_bd);
 		journal->j_dev_bd = NULL;
 	}

@@ -2603,7 +2601,6 @@ static int journal_init_dev(struct super
 	result = 0;
 
 	journal->j_dev_bd = NULL;
-	journal->j_dev_file = NULL;
 	jdev = SB_ONDISK_JOURNAL_DEVICE(super) ?
 	    new_decode_dev(SB_ONDISK_JOURNAL_DEVICE(super)) : super->s_dev;

@@ -2620,35 +2617,34 @@ static int journal_init_dev(struct super
 					 "cannot init journal device '%s': %i",
 					 __bdevname(jdev, b), result);
 			return result;
-		} else if (jdev != super->s_dev)
+		} else if (jdev != super->s_dev) {
+			result = bd_claim(journal->j_dev_bd, journal);
+			if (result) {
+				blkdev_put(journal->j_dev_bd);
+				return result;
+			}
+
 			set_blocksize(journal->j_dev_bd, super->s_blocksize);
+		}
+
 		return 0;
 	}
 
-	journal->j_dev_file = filp_open(jdev_name, 0, 0);
-	if (!IS_ERR(journal->j_dev_file)) {
-		struct inode *jdev_inode = journal->j_dev_file->f_mapping->host;
-		if (!S_ISBLK(jdev_inode->i_mode)) {
-			reiserfs_warning(super, "journal_init_dev: '%s' is "
-					 "not a block device", jdev_name);
-			result = -ENOTBLK;
-			release_journal_dev(super, journal);
-		} else {
-			/* ok */
-			journal->j_dev_bd = I_BDEV(jdev_inode);
-			set_blocksize(journal->j_dev_bd, super->s_blocksize);
-			reiserfs_info(super,
-				      "journal_init_dev: journal device: %s\n",
-				      bdevname(journal->j_dev_bd, b));
-		}
-	} else {
-		result = PTR_ERR(journal->j_dev_file);
-		journal->j_dev_file = NULL;
+	journal->j_dev_bd = open_bdev_excl(jdev_name, 0, journal);
+	if (IS_ERR(journal->j_dev_bd)) {
+		result = PTR_ERR(journal->j_dev_bd);
+		journal->j_dev_bd = NULL;
 		reiserfs_warning(super,
 				 "journal_init_dev: Cannot open '%s': %i",
 				 jdev_name, result);
+		return result;
 	}
-	return result;
+
+	set_blocksize(journal->j_dev_bd, super->s_blocksize);
+	reiserfs_info(super,
+		      "journal_init_dev: journal device: %s\n",
+		      bdevname(journal->j_dev_bd, b));
+	return 0;
 }
 
 /**

Index: linux-2.6-xfs/include/linux/reiserfs_fs_sb.h
===================================================================

include/linux/reiserfs_fs_sb.h struct reiserfs_journal { · line 182

Quoted 7 lines

include/linux/reiserfs_fs_sb.h +0 -1

@@ -177,7 +177,6 @@ struct reiserfs_journal {
 	struct reiserfs_journal_cnode *j_last;	/* newest journal block */
 	struct reiserfs_journal_cnode *j_first;	/*  oldest journal block.  start here for traverse */
 
-	struct file *j_dev_file;
 	struct block_device *j_dev_bd;
 	int j_1st_reserved_block;	/* first block on s_dev of reserved area journal */

---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html