| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20080920031408.GA9633@atrey.karlin.mff.cuni.cz>
Date: Sat, 20 Sep 2008 05:14:08 +0200
From: Jan Kara <jack@...e.cz>
To: Theodore Tso <tytso@....EDU>
Cc: Mark Fasheh <mfasheh@...e.com>,
"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
ocfs2-devel@....oracle.com, sunil.mushran@...cle.com
Subject: Re: [PATCH] jbd2: Create proc entry with bdevname+i_ino.
> 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@...e.cz>
Honza
> >From 172393695ea4daea0061aa7c4f7ee0d56fbda239 Mon Sep 17 00:00:00 2001
> From: Theodore Ts'o <tytso@....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@....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
> --
> 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@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jan Kara <jack@...e.cz>
SuSE CR Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists