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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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:
Acked-by: Jan Kara <jack@xxxxxxx>

								Honza

> >From 172393695ea4daea0061aa7c4f7ee0d56fbda239 Mon Sep 17 00:00:00 2001
> From: Theodore Ts'o <tytso@xxxxxxx>
> 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@xxxxxxx>
> ---
>  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
> -- 
> 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@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@xxxxxxx>
SuSE CR Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux