jbd2: Create proc entry with bdevname+i_ino.
Subsystems (3)
PATCH 1/1 jbd2: Create proc entry with bdevname+i_ino.
jbd2 currently creates statistics files for each loaded journal in /proc/fs/jbd2/<bdevname>. ocfs2 loads multiple journals when recovering other nodes, and the multiple journals on a given bdev collide in the proc namespace.
This patch changes the location to /proc/fs/jbd2/<bdevname>-<i_ino> as proposed by Jan Kara. ocfs2 can load multiple journals safely without collision. There are, to my knowledge, no programmatic users, so the name change shouldn't (haha) cause any problems.
ocfs2 has 64bit inode numbers, so a collision could theoretically happen with inodes above 32bits, but ocfs2 journals are generally created at the front of the volume during mkfs(8). In addition, ocfs2 restricts inodes to 32bits unless the 'inode64' option is specified.
--- fs/jbd2/journal.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
fs/jbd2/journal.c +8 -4
@@ -901,9 +901,11 @@ static struct proc_dir_entry *proc_jbd2_stats;static void jbd2_stats_proc_init (journal_t *journal) { -char name[BDEVNAME_SIZE ]; +char dname[BDEVNAME_SIZE ]; +char name[NAME_MAX ]; -bdevname (journal->j_dev, name); +bdevname (journal->j_dev, dname); +snprintf (name,NAME_MAX ,"%s-%lu" , dname, journal->j_inode->i_ino); journal->j_proc_entry =proc_mkdir (name, proc_jbd2_stats);if (journal->j_proc_entry) { proc_create_data("history", S_IRUGO, journal->j_proc_entry,
@@ -915,9 +917,11 @@ static void jbd2_stats_proc_init(journal_t *journal)static void jbd2_stats_proc_exit (journal_t *journal) { -char name[BDEVNAME_SIZE ]; +char dname[BDEVNAME_SIZE ]; +char name[NAME_MAX ]; -bdevname (journal->j_dev, name); +bdevname (journal->j_dev, dname); +snprintf (name,NAME_MAX ,"%s-%lu" , dname, journal->j_inode->i_ino);remove_proc_entry ("info" , journal->j_proc_entry);remove_proc_entry ("history" , journal->j_proc_entry);remove_proc_entry (name, proc_jbd2_stats);
Signature · 22 lines
1.5.6.3 -- None of our men are "experts." We have most unfortunately found it necessary to get rid of a man as soon as he thinks himself an expert -- because no one ever considers himself expert if he really knows his job. A man who knows a job sees so much more to be done than he has done, that he is always pressing forward and never gives up an instant of thought to how good and how efficient he is. Thinking always ahead, thinking always of trying to do more, brings a state of mind in which nothing is impossible. The moment one gets into the "expert" state of mind a great number of things become impossible. - From Henry Ford Sr., "My Life and Work" Joel Becker Principal Software Developer Oracle E-mail: joel.becker at oracle.com Phone: (650) 506-8127
Quoted 4 lines · Joel Becker
jbd2 currently creates statistics files for each loaded journal in /proc/fs/jbd2/<bdevname>. ocfs2 loads multiple journals when recovering other nodes, and the multiple journals on a given bdev collide in the proc namespace.
Did this patch ever get reviewied? We'd really like it in so that Ocfs2 can use JBD2...
Thanks,
--Mark
--
Mark Fasheh
Theodore Tso
maintainerQuoted 8 lines · Mark Fasheh
Quoted 4 lines · Joel Becker
jbd2 currently creates statistics files for each loaded journal in /proc/fs/jbd2/<bdevname>. ocfs2 loads multiple journals when recovering other nodes, and the multiple journals on a given bdev collide in the proc namespace.
Did this patch ever get reviewied? We'd really like it in so that Ocfs2 can use JBD2...
Thanks for pinging me; it fell through the cracks. I'll make sure it gets put into the queue for merging at the next merge window....
- Ted
Quoted 5 lines · Theodore Tso
Quoted 2 lines
Did this patch ever get reviewied? We'd really like it in so that Ocfs2 can use JBD2...
Thanks for pinging me; it fell through the cracks. I'll make sure it gets put into the queue for merging at the next merge window....
Great, thanks Ted! We'll be pushing Ocfs2/JBD2 support upstream next merge window too, so this lines things up well for us.
Btw, if anyone is interested in checking out the patches,
Pull:git://git.kernel.org/pub/scm/linux/kernel/git/mfasheh/ocfs2.git jbd2
or via gitweb:
http://git.kernel.org/?p=linux/kernel/git/mfasheh/ocfs2.git;a=shortlog;h=jbd2
Of course, they'll also hit lkml along with the rest of our Ocfs2 features.
--Mark
--
Mark Fasheh
Quoted 7 lines · Theodore Tso
Quoted 6 lines · Mark Fasheh
Quoted 4 lines · Joel Becker
jbd2 currently creates statistics files for each loaded journal in /proc/fs/jbd2/<bdevname>. ocfs2 loads multiple journals when recovering other nodes, and the multiple journals on a given bdev collide in the proc namespace.
Quoted 5 lines
Quoted 2 lines
Did this patch ever get reviewied? We'd really like it in so that Ocfs2 can use JBD2...
Thanks for pinging me; it fell through the cracks. I'll make sure it gets put into the queue for merging at the next merge window....
Could you please do it the proper way as describe in my reply to the thread?
Quoted 8 lines · Christoph Hellwig
Quoted 7 lines · Theodore Tso
Quoted 6 lines · Mark Fasheh
Quoted 4 lines · Joel Becker
jbd2 currently creates statistics files for each loaded journal in /proc/fs/jbd2/<bdevname>. ocfs2 loads multiple journals when recovering other nodes, and the multiple journals on a given bdev collide in the proc namespace.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Did this patch ever get reviewied? We'd really like it in so that Ocfs2 can use JBD2...
Thanks for pinging me; it fell through the cracks. I'll make sure it gets put into the queue for merging at the next merge window....
Could you please do it the proper way as describe in my reply to the thread?
Christoph,
Not sure what you mean here. Do you mean switching over
wholesale without the JBD compat config option? We're currently tracking a spinlock deadlock in JBD2, so I don't know if we're ready for that.
If you don't get this, I'll try to track you down tomorrow.
Joel
Signature · 8 lines
"You look in her eyes, the music begins to play. Hopeless romantics, here we go again." Joel Becker Principal Software Developer Oracle E-mail: [email protected] Phone: (650) 506-8127
Quoted 6 lines · Joel Becker
Christoph,
Not sure what you mean here. Do you mean switching over
wholesale without the JBD compat config option? We're currently tracking a spinlock deadlock in JBD2, so I don't know if we're ready for that.
If you don't get this, I'll try to track you down tomorrow.
Sorry, not related to jbd2 in ocfs2, but the bdevname bits in this thread. I saw Ted posted another patch later that basically does the same for the journal dev as the VFS does for sb->s_id so this is okay. I still think we should do the / substitution in get_sb_bdev, too and will send a separate patch for it.
Theodore Tso
maintainerThis is what plan to push instead. It should address ocfs2's requirement. In addition, it reduces jbd2 and ext4's stack usage, removes 30 lines of code, and fixes a potential problem with pesky block devices which contain '/' character in their names. Four for the price of one!
Could you try this out confirm this fixes the problem for you? (Currently it only passes the "It builds, ship it!" test, but it's pretty straightforward; and I'm on an airplane at the moment. :-)
Thanks, regards,
- Ted
From 172393695ea4daea0061aa7c4f7ee0d56fbda239 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Sun, 14 Sep 2008 12:22:35 -0400
Subject: [PATCH] jbd2: clean up how the journal device name is printed
Calculate the journal device name once and stash it away in the journal_s structure. This avoids needing to call bdevname() everywhere and reduces stack usage by not needing to allocate an on-stack buffer. In addition, we eliminate the '/' that can appear in device names (e.g. "cciss/c0d0p9" --- see kernel bugzilla #11321) that can cause problems when creating proc directory names, and include the inode number to support ocfs2 which creates multiple journals with different inode numbers.
--- fs/ext4/super.c | 12 +++--------- fs/jbd2/commit.c | 11 +++-------- fs/jbd2/journal.c | 48 ++++++++++++++++-------------------------------- include/linux/jbd2.h | 3 ++- 4 files changed, 24 insertions(+), 50 deletions(-)
fs/ext4/super.c +3 -9
@@ -1476,15 +1476,9 @@ static int ext4_setup_super(struct super_block *sb, struct ext4_super_block *es,EXT4_INODES_PER_GROUP (sb), sbi->s_mount_opt); -if (EXT4_SB (sb)->s_journal->j_inode ==NULL ) { -char b[BDEVNAME_SIZE ]; - -printk (KERN_INFO "EXT4 FS on %s, external journal on %s\n" , - sb->s_id,bdevname (EXT4_SB (sb)->s_journal->j_dev, b)); - }else { -printk (KERN_INFO "EXT4 FS on %s, internal journal\n" , - sb->s_id); - } +printk (KERN_INFO "EXT4 FS on %s, %s journal on %s\n" , + sb->s_id,EXT4_SB (sb)->s_journal->j_inode ?"internal" : +"external" ,EXT4_SB (sb)->s_journal->j_devname);return res; }
fs/jbd2/commit.c +3 -8
@@ -147,12 +147,9 @@ static int journal_submit_commit_record(journal_t *journal,* to remember if we sent a barrier request */ if (ret == -EOPNOTSUPP && barrier_done) { -char b[BDEVNAME_SIZE ]; -printk (KERN_WARNING -"JBD: barrier-based sync failed on %s - " -"disabling barriers\n" , -bdevname (journal->j_dev, b)); +"JBD: barrier-based sync failed on %s - " +"disabling barriers\n" , journal->j_devname);spin_lock (&journal->j_state_lock); journal->j_flags &= ~JBD2_BARRIER ;spin_unlock (&journal->j_state_lock);
@@ -681,11 +678,9 @@ start_journal_io:*/ err =journal_finish_inode_data_buffers (journal, commit_transaction);if (err) { -char b[BDEVNAME_SIZE ]; -printk (KERN_WARNING "JBD2: Detected IO errors while flushing file data " -"on %s\n" ,bdevname (journal->j_fs_dev, b)); +"on %s\n" , journal->j_devname); err =0 ; }
fs/jbd2/journal.c +16 -32
111 lines not on this page · raw message
include/linux/jbd2.h +2 -1
@@ -850,7 +850,8 @@ struct journal_s*/ struct block_device *j_dev;int j_blocksize; -unsigned long long j_blk_offset; +unsigned long long j_blk_offset; +char j_devname[BDEVNAME_SIZE +24 ];/* * Device which holds the client fs. For internal journal this will be
Signature · 1 line
1.5.6.1.205.ge2c7.dirty
Quoted 5 lines · Theodore Tso
This is what plan to push instead. It should address ocfs2's requirement. In addition, it reduces jbd2 and ext4's stack usage, removes 30 lines of code, and fixes a potential problem with pesky block devices which contain '/' character in their names. Four for the price of one!
I like it! I don't have time to test it (KS/LPC), but hopefully
Sunil can.
Joel
Signature · 9 lines
Life's Little Instruction Book #222 "Think twice before burdening a friend with a secret." Joel Becker Principal Software Developer Oracle E-mail: joel.becker at oracle.com Phone: (650) 506-8127
Thanks.
So we are testing this atop Joel's jbd2 branch (after reversing the top patch).
http://oss.oracle.com/git/?p=jlbec/linux-2.6.git;a=shortlog;h=jbd2
Quoted 230 lines · Theodore Tso
This is what plan to push instead. It should address ocfs2's requirement. In addition, it reduces jbd2 and ext4's stack usage, removes 30 lines of code, and fixes a potential problem with pesky block devices which contain '/' character in their names. Four for the price of one!
Could you try this out confirm this fixes the problem for you? (Currently it only passes the "It builds, ship it!" test, but it's pretty straightforward; and I'm on an airplane at the moment. :-)
Thanks, regards,
- Ted
From
172393695ea4daea0061aa7c4f7ee0d56fbda239Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Sun, 14 Sep 2008 12:22:35 -0400
Subject: [PATCH] jbd2: clean up how the journal device name is printedCalculate the journal device name once and stash it away in the
journal_sstructure. This avoids needing to callbdevname()everywhere and reduces stack usage by not needing to allocate an on-stack buffer. In addition, we eliminate the '/' that can appear in device names (e.g. "cciss/c0d0p9" --- see kernel bugzilla #11321) that can cause problems when creating proc directory names, and include the inode number to support ocfs2 which creates multiple journals with different inode numbers.Signed-off-by "Theodore Ts'o" <[email protected]>--- fs/ext4/super.c | 12 +++--------- fs/jbd2/commit.c | 11 +++-------- fs/jbd2/journal.c | 48 ++++++++++++++++-------------------------------- include/linux/jbd2.h | 3 ++- 4 files changed, 24 insertions(+), 50 deletions(-)fs/ext4/super.c +3
-9@@ -1476,15 +1476,9 @@ static int ext4_setup_super(struct super_block *sb, struct ext4_super_block *es,EXT4_INODES_PER_GROUP (sb), sbi->s_mount_opt); -if (EXT4_SB (sb)->s_journal->j_inode ==NULL ) { -char b[BDEVNAME_SIZE ]; - -printk (KERN_INFO "EXT4 FS on %s, external journal on %s\n" , - sb->s_id,bdevname (EXT4_SB (sb)->s_journal->j_dev, b)); - }else { -printk (KERN_INFO "EXT4 FS on %s, internal journal\n" , - sb->s_id); - } +printk (KERN_INFO "EXT4 FS on %s, %s journal on %s\n" , + sb->s_id,EXT4_SB (sb)->s_journal->j_inode ?"internal" : +"external" ,EXT4_SB (sb)->s_journal->j_devname);return res; }fs/jbd2/commit.c +3
-8@@ -147,12 +147,9 @@ static int journal_submit_commit_record(journal_t *journal,* to remember if we sent a barrier request */ if (ret == -EOPNOTSUPP && barrier_done) { -char b[BDEVNAME_SIZE ]; -printk (KERN_WARNING -"JBD: barrier-based sync failed on %s - " -"disabling barriers\n" , -bdevname (journal->j_dev, b)); +"JBD: barrier-based sync failed on %s - " +"disabling barriers\n" , journal->j_devname);spin_lock (&journal->j_state_lock); journal->j_flags &= ~JBD2_BARRIER ;spin_unlock (&journal->j_state_lock);@@ -681,11 +678,9 @@ start_journal_io:*/ err =journal_finish_inode_data_buffers (journal, commit_transaction);if (err) { -char b[BDEVNAME_SIZE ]; -printk (KERN_WARNING "JBD2: Detected IO errors while flushing file data " -"on %s\n" ,bdevname (journal->j_fs_dev, b)); +"on %s\n" , journal->j_devname); err =0 ; }fs/jbd2/journal.c +16
-32@@ -597,13 +597,9 @@ int jbd2_journal_bmap(journal_t *journal, unsigned long blocknr,if (ret) *retp = ret;else { -char b[BDEVNAME_SIZE ]; -printk (KERN_ALERT "%s: journal block not found " "at offset %lu on %s\n" , - __func__, - blocknr, -bdevname (journal->j_dev, b)); + __func__, blocknr, journal->j_devname); err = -EIO ;__journal_abort_soft (journal, err); }@@ -901,10 +897,7 @@ static struct proc_dir_entry *proc_jbd2_stats;static void jbd2_stats_proc_init (journal_t *journal) { -char name[BDEVNAME_SIZE ]; - -bdevname (journal->j_dev, name); - journal->j_proc_entry =proc_mkdir (name, proc_jbd2_stats); + journal->j_proc_entry =proc_mkdir (journal->j_devname, proc_jbd2_stats);if (journal->j_proc_entry) {proc_create_data ("history" ,S_IRUGO , journal->j_proc_entry, &jbd2_seq_history_fops, journal);@@ -915,12 +908,9 @@ static void jbd2_stats_proc_init(journal_t *journal)static void jbd2_stats_proc_exit (journal_t *journal) { -char name[BDEVNAME_SIZE ]; - -bdevname (journal->j_dev, name);remove_proc_entry ("info" , journal->j_proc_entry);remove_proc_entry ("history" , journal->j_proc_entry); -remove_proc_entry (name, proc_jbd2_stats); +remove_proc_entry (journal->j_devname, proc_jbd2_stats); }static void journal_init_stats (journal_t *journal)@@ -1018,6 +1008,7 @@ journal_t * jbd2_journal_init_dev(struct block_device *bdev, {journal_t *journal =journal_init_common ();struct buffer_head *bh; +char *p;int n;if (!journal)@@ -1039,6 +1030,10 @@ journal_t * jbd2_journal_init_dev(struct block_device *bdev, journal->j_fs_dev = fs_dev; journal->j_blk_offset = start; journal->j_maxlen = len; +bdevname (journal->j_dev, journal->j_devname); + p = journal->j_devname; +while ((p =strchr (p,'/' ))) + *p ='!' ;jbd2_stats_proc_init (journal); bh =__getblk (journal->j_dev, start, journal->j_blocksize);@@ -1061,6 +1056,7 @@ journal_t * jbd2_journal_init_inode (struct inode *inode) {struct buffer_head *bh;journal_t *journal =journal_init_common (); +char *p;int err;int n;unsigned long long blocknr;@@ -1070,6 +1066,12 @@ journal_t * jbd2_journal_init_inode (struct inode *inode) journal->j_dev = journal->j_fs_dev = inode->i_sb->s_bdev; journal->j_inode = inode; +bdevname (journal->j_dev, journal->j_devname); + p = journal->j_devname; +while ((p =strchr (p,'/' ))) + *p ='!' ; + p = journal->j_devname +strlen (journal->j_devname); +sprintf (p,":%lu\n" , journal->j_inode->i_ino); jbd_debug(1, "journal %p: inode %s/%ld, size %Ld, bits %d, blksize %ld\n", journal, inode->i_sb->s_id, inode->i_ino,@@ -1761,23 +1763,6 @@ int jbd2_journal_wipe(journal_t *journal, int write) }/* -* journal_dev_name: format a character string to describe on what -* device this journal is present. -*/ - -static const char *journal_dev_name (journal_t *journal,char *buffer) -{ -struct block_device *bdev; - -if (journal->j_inode) - bdev = journal->j_inode->i_sb->s_bdev; -else - bdev = journal->j_dev; - -return bdevname (bdev, buffer); -} - -/* * Journal abort has very specific semantics, which we describe * for journal abort. * @@ -1793,13 +1778,12 @@ static const char *journal_dev_name(journal_t *journal, char *buffer)void __jbd2_journal_abort_hard (journal_t *journal) {transaction_t *transaction; -char b[BDEVNAME_SIZE ];if (journal->j_flags &JBD2_ABORT )return ;printk (KERN_ERR "Aborting journal on device %s.\n" , -journal_dev_name (journal, b)); + journal->j_devname);spin_lock (&journal->j_state_lock); journal->j_flags |=JBD2_ABORT ;include/linux/jbd2.h +2
-1@@ -850,7 +850,8 @@ struct journal_s*/ struct block_device *j_dev;int j_blocksize; -unsigned long long j_blk_offset; +unsigned long long j_blk_offset; +char j_devname[BDEVNAME_SIZE +24 ];/* * Device which holds the client fs. For internal journal this will be
The newline looks like a typo here.
@@ -1070,6 +1066,12 @@ journal_t * jbd2_journal_init_inode (struct inode *inode)
+ sprintf(p, ":%lu\n", journal->j_inode->i_ino);
Other than that, the test ran fine. Marcos successfully ran multiple recoveries in a 6 node cluster.
Quoted 247 lines · Sunil Mushran
Thanks.
So we are testing this atop Joel's jbd2 branch (after reversing the top patch).
http://oss.oracle.com/git/?p=jlbec/linux-2.6.git;a=shortlog;h=jbd2
Quoted 222 lines · Theodore Tso
This is what plan to push instead. It should address ocfs2's requirement. In addition, it reduces jbd2 and ext4's stack usage, removes 30 lines of code, and fixes a potential problem with pesky block devices which contain '/' character in their names. Four for the price of one!
Could you try this out confirm this fixes the problem for you? (Currently it only passes the "It builds, ship it!" test, but it's pretty straightforward; and I'm on an airplane at the moment. :-)
Thanks, regards,
- Ted
From
172393695ea4daea0061aa7c4f7ee0d56fbda239Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Sun, 14 Sep 2008 12:22:35 -0400
Subject: [PATCH] jbd2: clean up how the journal device name is printedCalculate the journal device name once and stash it away in the
journal_sstructure. This avoids needing to callbdevname()everywhere and reduces stack usage by not needing to allocate an on-stack buffer. In addition, we eliminate the '/' that can appear in device names (e.g. "cciss/c0d0p9" --- see kernel bugzilla #11321) that can cause problems when creating proc directory names, and include the inode number to support ocfs2 which creates multiple journals with different inode numbers.Signed-off-by "Theodore Ts'o" <[email protected]>--- fs/ext4/super.c | 12 +++--------- fs/jbd2/commit.c | 11 +++-------- fs/jbd2/journal.c | 48 ++++++++++++++++-------------------------------- include/linux/jbd2.h | 3 ++- 4 files changed, 24 insertions(+), 50 deletions(-)fs/ext4/super.c +3
-9@@ -1476,15 +1476,9 @@ static int ext4_setup_super(struct super_block *sb, struct ext4_super_block *es,EXT4_INODES_PER_GROUP (sb), sbi->s_mount_opt); -if (EXT4_SB (sb)->s_journal->j_inode ==NULL ) { -char b[BDEVNAME_SIZE ]; - -printk (KERN_INFO "EXT4 FS on %s, external journal on %s\n" , - sb->s_id,bdevname (EXT4_SB (sb)->s_journal->j_dev, b)); - }else { -printk (KERN_INFO "EXT4 FS on %s, internal journal\n" , - sb->s_id); - } +printk (KERN_INFO "EXT4 FS on %s, %s journal on %s\n" , + sb->s_id,EXT4_SB (sb)->s_journal->j_inode ?"internal" : +"external" ,EXT4_SB (sb)->s_journal->j_devname);return res; }fs/jbd2/commit.c +3
-8@@ -147,12 +147,9 @@ static int journal_submit_commit_record(journal_t *journal,* to remember if we sent a barrier request */ if (ret == -EOPNOTSUPP && barrier_done) { -char b[BDEVNAME_SIZE ]; -printk (KERN_WARNING -"JBD: barrier-based sync failed on %s - " -"disabling barriers\n" , -bdevname (journal->j_dev, b)); +"JBD: barrier-based sync failed on %s - " +"disabling barriers\n" , journal->j_devname);spin_lock (&journal->j_state_lock); journal->j_flags &= ~JBD2_BARRIER ;spin_unlock (&journal->j_state_lock);@@ -681,11 +678,9 @@ start_journal_io:*/ err =journal_finish_inode_data_buffers (journal, commit_transaction);if (err) { -char b[BDEVNAME_SIZE ]; -printk (KERN_WARNING "JBD2: Detected IO errors while flushing file data " -"on %s\n" ,bdevname (journal->j_fs_dev, b)); +"on %s\n" , journal->j_devname); err =0 ; }fs/jbd2/journal.c +16
-32@@ -597,13 +597,9 @@ int jbd2_journal_bmap(journal_t *journal, unsigned long blocknr,if (ret) *retp = ret;else { -char b[BDEVNAME_SIZE ]; -printk (KERN_ALERT "%s: journal block not found " "at offset %lu on %s\n" , - __func__, - blocknr, -bdevname (journal->j_dev, b)); + __func__, blocknr, journal->j_devname); err = -EIO ;__journal_abort_soft (journal, err); }@@ -901,10 +897,7 @@ static struct proc_dir_entry *proc_jbd2_stats;static void jbd2_stats_proc_init (journal_t *journal) { -char name[BDEVNAME_SIZE ]; - -bdevname (journal->j_dev, name); - journal->j_proc_entry =proc_mkdir (name, proc_jbd2_stats); + journal->j_proc_entry =proc_mkdir (journal->j_devname, proc_jbd2_stats);if (journal->j_proc_entry) {proc_create_data ("history" ,S_IRUGO , journal->j_proc_entry, &jbd2_seq_history_fops, journal);@@ -915,12 +908,9 @@ static void jbd2_stats_proc_init(journal_t *journal)static void jbd2_stats_proc_exit (journal_t *journal) { -char name[BDEVNAME_SIZE ]; - -bdevname (journal->j_dev, name);remove_proc_entry ("info" , journal->j_proc_entry);remove_proc_entry ("history" , journal->j_proc_entry); -remove_proc_entry (name, proc_jbd2_stats); +remove_proc_entry (journal->j_devname, proc_jbd2_stats); }static void journal_init_stats (journal_t *journal)@@ -1018,6 +1008,7 @@ journal_t * jbd2_journal_init_dev(struct block_device *bdev, {journal_t *journal =journal_init_common ();struct buffer_head *bh; +char *p;int n;if (!journal)@@ -1039,6 +1030,10 @@ journal_t * jbd2_journal_init_dev(struct block_device *bdev, journal->j_fs_dev = fs_dev; journal->j_blk_offset = start; journal->j_maxlen = len; +bdevname (journal->j_dev, journal->j_devname); + p = journal->j_devname; +while ((p =strchr (p,'/' ))) + *p ='!' ;jbd2_stats_proc_init (journal); bh =__getblk (journal->j_dev, start, journal->j_blocksize);@@ -1061,6 +1056,7 @@ journal_t * jbd2_journal_init_inode (struct inode *inode) {struct buffer_head *bh;journal_t *journal =journal_init_common (); +char *p;int err;int n;unsigned long long blocknr;@@ -1070,6 +1066,12 @@ journal_t * jbd2_journal_init_inode (struct inode *inode) journal->j_dev = journal->j_fs_dev = inode->i_sb->s_bdev; journal->j_inode = inode; +bdevname (journal->j_dev, journal->j_devname); + p = journal->j_devname; +while ((p =strchr (p,'/' ))) + *p ='!' ; + p = journal->j_devname +strlen (journal->j_devname); +sprintf (p,":%lu\n" , journal->j_inode->i_ino); jbd_debug(1, "journal %p: inode %s/%ld, size %Ld, bits %d, blksize %ld\n", journal, inode->i_sb->s_id, inode->i_ino,@@ -1761,23 +1763,6 @@ int jbd2_journal_wipe(journal_t *journal, int write) }/* -* journal_dev_name: format a character string to describe on what -* device this journal is present. -*/ - -static const char *journal_dev_name (journal_t *journal,char *buffer) -{ -struct block_device *bdev; - -if (journal->j_inode) - bdev = journal->j_inode->i_sb->s_bdev; -else - bdev = journal->j_dev; - -return bdevname (bdev, buffer); -} - -/* * Journal abort has very specific semantics, which we describe * for journal abort. * @@ -1793,13 +1778,12 @@ static const char *journal_dev_name(journal_t *journal, char *buffer)void __jbd2_journal_abort_hard (journal_t *journal) {transaction_t *transaction; -char b[BDEVNAME_SIZE ];if (journal->j_flags &JBD2_ABORT )return ;printk (KERN_ERR "Aborting journal on device %s.\n" , -journal_dev_name (journal, b)); + journal->j_devname);spin_lock (&journal->j_state_lock); journal->j_flags |=JBD2_ABORT ;include/linux/jbd2.h +0
-0include/linux/jbd2.h struct journal_s · line 857
Quoted 9 lines
include/linux/jbd2.h +2
-1@@ -850,7 +850,8 @@ struct journal_s*/ struct block_device *j_dev;int j_blocksize; -unsigned long long j_blk_offset; +unsigned long long j_blk_offset; +char j_devname[BDEVNAME_SIZE +24 ];/* * Device which holds the client fs. For internal journal this will be _______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel
Quoted 9 lines
This is what plan to push instead. It should address ocfs2's requirement. In addition, it reduces jbd2 and ext4's stack usage, removes 30 lines of code, and fixes a potential problem with pesky block devices which contain '/' character in their names. Four for the price of one!
Could you try this out confirm this fixes the problem for you? (Currently it only passes the "It builds, ship it!" test, but it's pretty straightforward; and I'm on an airplane at the moment. :-)
The patch looks fine. You can add:
Honza
Quoted 222 lines
Quoted 1 line
From
172393695ea4daea0061aa7c4f7ee0d56fbda239Mon Sep 17 00:00:00 2001From: Theodore Ts'o <[email protected]>
Date: Sun, 14 Sep 2008 12:22:35 -0400
Subject: [PATCH] jbd2: clean up how the journal device name is printedCalculate the journal device name once and stash it away in the
journal_sstructure. This avoids needing to callbdevname()everywhere and reduces stack usage by not needing to allocate an on-stack buffer. In addition, we eliminate the '/' that can appear in device names (e.g. "cciss/c0d0p9" --- see kernel bugzilla #11321) that can cause problems when creating proc directory names, and include the inode number to support ocfs2 which creates multiple journals with different inode numbers.Signed-off-by "Theodore Ts'o" <[email protected]>--- fs/ext4/super.c | 12 +++--------- fs/jbd2/commit.c | 11 +++-------- fs/jbd2/journal.c | 48 ++++++++++++++++-------------------------------- include/linux/jbd2.h | 3 ++- 4 files changed, 24 insertions(+), 50 deletions(-)fs/ext4/super.c +3
-9@@ -1476,15 +1476,9 @@ static int ext4_setup_super(struct super_block *sb, struct ext4_super_block *es,EXT4_INODES_PER_GROUP (sb), sbi->s_mount_opt); -if (EXT4_SB (sb)->s_journal->j_inode ==NULL ) { -char b[BDEVNAME_SIZE ]; - -printk (KERN_INFO "EXT4 FS on %s, external journal on %s\n" , - sb->s_id,bdevname (EXT4_SB (sb)->s_journal->j_dev, b)); - }else { -printk (KERN_INFO "EXT4 FS on %s, internal journal\n" , - sb->s_id); - } +printk (KERN_INFO "EXT4 FS on %s, %s journal on %s\n" , + sb->s_id,EXT4_SB (sb)->s_journal->j_inode ?"internal" : +"external" ,EXT4_SB (sb)->s_journal->j_devname);return res; }fs/jbd2/commit.c +3
-8@@ -147,12 +147,9 @@ static int journal_submit_commit_record(journal_t *journal,* to remember if we sent a barrier request */ if (ret == -EOPNOTSUPP && barrier_done) { -char b[BDEVNAME_SIZE ]; -printk (KERN_WARNING -"JBD: barrier-based sync failed on %s - " -"disabling barriers\n" , -bdevname (journal->j_dev, b)); +"JBD: barrier-based sync failed on %s - " +"disabling barriers\n" , journal->j_devname);spin_lock (&journal->j_state_lock); journal->j_flags &= ~JBD2_BARRIER ;spin_unlock (&journal->j_state_lock);@@ -681,11 +678,9 @@ start_journal_io:*/ err =journal_finish_inode_data_buffers (journal, commit_transaction);if (err) { -char b[BDEVNAME_SIZE ]; -printk (KERN_WARNING "JBD2: Detected IO errors while flushing file data " -"on %s\n" ,bdevname (journal->j_fs_dev, b)); +"on %s\n" , journal->j_devname); err =0 ; }fs/jbd2/journal.c +16
-32@@ -597,13 +597,9 @@ int jbd2_journal_bmap(journal_t *journal, unsigned long blocknr,if (ret) *retp = ret;else { -char b[BDEVNAME_SIZE ]; -printk (KERN_ALERT "%s: journal block not found " "at offset %lu on %s\n" , - __func__, - blocknr, -bdevname (journal->j_dev, b)); + __func__, blocknr, journal->j_devname); err = -EIO ;__journal_abort_soft (journal, err); }@@ -901,10 +897,7 @@ static struct proc_dir_entry *proc_jbd2_stats;static void jbd2_stats_proc_init (journal_t *journal) { -char name[BDEVNAME_SIZE ]; - -bdevname (journal->j_dev, name); - journal->j_proc_entry =proc_mkdir (name, proc_jbd2_stats); + journal->j_proc_entry =proc_mkdir (journal->j_devname, proc_jbd2_stats);if (journal->j_proc_entry) {proc_create_data ("history" ,S_IRUGO , journal->j_proc_entry, &jbd2_seq_history_fops, journal);@@ -915,12 +908,9 @@ static void jbd2_stats_proc_init(journal_t *journal)static void jbd2_stats_proc_exit (journal_t *journal) { -char name[BDEVNAME_SIZE ]; - -bdevname (journal->j_dev, name);remove_proc_entry ("info" , journal->j_proc_entry);remove_proc_entry ("history" , journal->j_proc_entry); -remove_proc_entry (name, proc_jbd2_stats); +remove_proc_entry (journal->j_devname, proc_jbd2_stats); }static void journal_init_stats (journal_t *journal)@@ -1018,6 +1008,7 @@ journal_t * jbd2_journal_init_dev(struct block_device *bdev, {journal_t *journal =journal_init_common ();struct buffer_head *bh; +char *p;int n;if (!journal)@@ -1039,6 +1030,10 @@ journal_t * jbd2_journal_init_dev(struct block_device *bdev, journal->j_fs_dev = fs_dev; journal->j_blk_offset = start; journal->j_maxlen = len; +bdevname (journal->j_dev, journal->j_devname); + p = journal->j_devname; +while ((p =strchr (p,'/' ))) + *p ='!' ;jbd2_stats_proc_init (journal); bh =__getblk (journal->j_dev, start, journal->j_blocksize);@@ -1061,6 +1056,7 @@ journal_t * jbd2_journal_init_inode (struct inode *inode) {struct buffer_head *bh;journal_t *journal =journal_init_common (); +char *p;int err;int n;unsigned long long blocknr;@@ -1070,6 +1066,12 @@ journal_t * jbd2_journal_init_inode (struct inode *inode) journal->j_dev = journal->j_fs_dev = inode->i_sb->s_bdev; journal->j_inode = inode; +bdevname (journal->j_dev, journal->j_devname); + p = journal->j_devname; +while ((p =strchr (p,'/' ))) + *p ='!' ; + p = journal->j_devname +strlen (journal->j_devname); +sprintf (p,":%lu\n" , journal->j_inode->i_ino); jbd_debug(1, "journal %p: inode %s/%ld, size %Ld, bits %d, blksize %ld\n", journal, inode->i_sb->s_id, inode->i_ino,@@ -1761,23 +1763,6 @@ int jbd2_journal_wipe(journal_t *journal, int write) }/* -* journal_dev_name: format a character string to describe on what -* device this journal is present. -*/ - -static const char *journal_dev_name (journal_t *journal,char *buffer) -{ -struct block_device *bdev; - -if (journal->j_inode) - bdev = journal->j_inode->i_sb->s_bdev; -else - bdev = journal->j_dev; - -return bdevname (bdev, buffer); -} - -/* * Journal abort has very specific semantics, which we describe * for journal abort. * @@ -1793,13 +1778,12 @@ static const char *journal_dev_name(journal_t *journal, char *buffer)void __jbd2_journal_abort_hard (journal_t *journal) {transaction_t *transaction; -char b[BDEVNAME_SIZE ];if (journal->j_flags &JBD2_ABORT )return ;printk (KERN_ERR "Aborting journal on device %s.\n" , -journal_dev_name (journal, b)); + journal->j_devname);spin_lock (&journal->j_state_lock); journal->j_flags |=JBD2_ABORT ;include/linux/jbd2.h +2
-1@@ -850,7 +850,8 @@ struct journal_s*/ struct block_device *j_dev;int j_blocksize; -unsigned long long j_blk_offset; +unsigned long long j_blk_offset; +char j_devname[BDEVNAME_SIZE +24 ];/* * Device which holds the client fs. For internal journal this will be Signature · 6 lines
1.5.6.1.205.ge2c7.dirty -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
Jan Kara <[email protected]> SuSE CR Labs