Re: [Ocfs2-devel] [PATCH] jbd2: Create proc entry with bdevname+i_ino.

Sunil Mushran
2008-09-16T18:02:09Z

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.

Sunil Mushran wrote:
> 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
>
> Theodore Tso wrote:
>
>> 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 172393695ea4daea0061aa7c4f7ee0d56fbda239 Mon Sep 17 00:00:00 2001
>> From: Theodore Ts'o <tytso@mit.edu>
>> 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.
>>
>> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
>> ---
>> 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(-)
>>
>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index f58cc03..64e1c21 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -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;
>> }
>>
>> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
>> index f2ad061..b091e53 100644
>> --- a/fs/jbd2/commit.c
>> +++ b/fs/jbd2/commit.c
>> @@ -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;
>> }
>>
>> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
>> index 8207a01..0c4dcc2 100644
>> --- a/fs/jbd2/journal.c
>> +++ b/fs/jbd2/journal.c
>> @@ -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;
>> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
>> index 3dd2090..66c3499 100644
>> --- a/include/linux/jbd2.h
>> +++ b/include/linux/jbd2.h
>> @@ -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@oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
>