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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTinyCRL=Gu1eUG3gM6zyiKC+Jo87JoCgfwcpyB+f@mail.gmail.com>
Date:	Wed, 5 Jan 2011 11:26:05 +0200
From:	Amir Goldstein <amir73il@...il.com>
To:	"Theodore Ts'o" <tytso@....edu>
Cc:	Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH 6/6] ext4: Dynamically allocate the jbd2_inode in
 ext4_inode_info as necessary

On Wed, Jan 5, 2011 at 3:01 AM, Theodore Ts'o <tytso@....edu> wrote:
> Replace the jbd2_inode structure (which is 48 bytes) with a pointer
> and only allocate the jbd2_inode when it is needed --- that is, when
> the file system has a journal present and the inode has been opened
> for writing.  This allows us to further slim down the ext4_inode_info
> structure.
>
> Signed-off-by: "Theodore Ts'o" <tytso@....edu>
> ---
>  fs/ext4/ext4.h       |    2 +-
>  fs/ext4/ext4_jbd2.h  |    2 +-
>  fs/ext4/file.c       |   19 +++++++++++++++++++
>  fs/ext4/inode.c      |   15 +++++++++++----
>  fs/ext4/super.c      |   16 +++++++---------
>  fs/jbd2/journal.c    |   20 +++++++++++++-------
>  include/linux/jbd2.h |   20 ++++++++++++++++++--
>  7 files changed, 70 insertions(+), 24 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 8a2603d..50f35e8 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -811,7 +811,7 @@ struct ext4_inode_info {
>         */
>        struct rw_semaphore i_data_sem;
>        struct inode vfs_inode;
> -       struct jbd2_inode jinode;
> +       struct jbd2_inode *jinode;
>
>        struct ext4_ext_cache i_cached_extent;
>        /*
> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
> index b0bd792..d8b992e 100644
> --- a/fs/ext4/ext4_jbd2.h
> +++ b/fs/ext4/ext4_jbd2.h
> @@ -253,7 +253,7 @@ static inline int ext4_journal_force_commit(journal_t *journal)
>  static inline int ext4_jbd2_file_inode(handle_t *handle, struct inode *inode)
>  {
>        if (ext4_handle_valid(handle))
> -               return jbd2_journal_file_inode(handle, &EXT4_I(inode)->jinode);
> +               return jbd2_journal_file_inode(handle, EXT4_I(inode)->jinode);
>        return 0;
>  }
>
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 5a5c55d..45a656f 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -104,6 +104,7 @@ static int ext4_file_open(struct inode * inode, struct file * filp)
>  {
>        struct super_block *sb = inode->i_sb;
>        struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> +       struct ext4_inode_info *ei = EXT4_I(inode);
>        struct vfsmount *mnt = filp->f_path.mnt;
>        struct path path;
>        char buf[64], *cp;
> @@ -127,6 +128,24 @@ static int ext4_file_open(struct inode * inode, struct file * filp)
>                        ext4_mark_super_dirty(sb);
>                }
>        }
> +       /*
> +        * Set up the jbd2_inode if we are opening the inode for
> +        * writing and the journal is present
> +        */
> +       if (sbi->s_journal && !ei->jinode && (filp->f_mode & FMODE_WRITE)) {
> +               struct jbd2_inode *jinode = jbd2_alloc_inode(GFP_KERNEL);
> +
> +               spin_lock(&inode->i_lock);
> +               if (!ei->jinode) {
> +                       if (!jinode) {
> +                               spin_unlock(&inode->i_lock);
> +                               return -ENOMEM;
> +                       }
> +                       ei->jinode = jinode;
> +                       jbd2_journal_init_jbd_inode(ei->jinode, inode);

jinode = NULL;

> +               }
> +               spin_unlock(&inode->i_lock);

if (jinode)
    // ei->jinode was allocated after first test - can it happen?
    // if not, then 2nd test under spinlock is not needed
    jbd2_free_inode(jinode);

> +       }
>        return dquot_file_open(inode, filp);
>  }
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 2d60028..de3f2e3 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -55,10 +55,17 @@ static inline int ext4_begin_ordered_truncate(struct inode *inode,
>                                              loff_t new_size)
>  {
>        trace_ext4_begin_ordered_truncate(inode, new_size);
> -       return jbd2_journal_begin_ordered_truncate(
> -                                       EXT4_SB(inode->i_sb)->s_journal,
> -                                       &EXT4_I(inode)->jinode,
> -                                       new_size);
> +       /*
> +        * If jinode is zero, then we never opened the file for
> +        * writing, so there's no need to call
> +        * jbd2_journal_begin_ordered_truncate() since there's no
> +        * outstanding writes we need to flush.
> +        */
> +       if (!EXT4_I(inode)->jinode)
> +               return 0;
> +       return jbd2_journal_begin_ordered_truncate(EXT4_JOURNAL(inode),
> +                                                  EXT4_I(inode)->jinode,
> +                                                  new_size);
>  }
>
>  static void ext4_invalidatepage(struct page *page, unsigned long offset);
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index f5960d6..7f99ac7 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -818,12 +818,6 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
>        memset(&ei->i_cached_extent, 0, sizeof(struct ext4_ext_cache));
>        INIT_LIST_HEAD(&ei->i_prealloc_list);
>        spin_lock_init(&ei->i_prealloc_lock);
> -       /*
> -        * Note:  We can be called before EXT4_SB(sb)->s_journal is set,
> -        * therefore it can be null here.  Don't check it, just initialize
> -        * jinode.
> -        */
> -       jbd2_journal_init_jbd_inode(&ei->jinode, &ei->vfs_inode);
>        ei->i_reserved_data_blocks = 0;
>        ei->i_reserved_meta_blocks = 0;
>        ei->i_allocated_meta_blocks = 0;
> @@ -832,6 +826,7 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
>  #ifdef CONFIG_QUOTA
>        ei->i_reserved_quota = 0;
>  #endif
> +       ei->jinode = 0;
>        INIT_LIST_HEAD(&ei->i_completed_io_list);
>        spin_lock_init(&ei->i_completed_io_lock);
>        ei->cur_aio_dio = NULL;
> @@ -900,9 +895,12 @@ void ext4_clear_inode(struct inode *inode)
>        end_writeback(inode);
>        dquot_drop(inode);
>        ext4_discard_preallocations(inode);
> -       if (EXT4_JOURNAL(inode))
> -               jbd2_journal_release_jbd_inode(EXT4_SB(inode->i_sb)->s_journal,
> -                                      &EXT4_I(inode)->jinode);
> +       if (EXT4_JOURNAL(inode) && EXT4_I(inode)->jinode) {
> +               jbd2_journal_release_jbd_inode(EXT4_JOURNAL(inode),
> +                                              EXT4_I(inode)->jinode);
> +               jbd2_free_inode(EXT4_I(inode)->jinode);
> +               EXT4_I(inode)->jinode = 0;
> +       }
>  }
>
>  static inline void ext4_show_quota_options(struct seq_file *seq,
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 2447bd8..9e46869 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -94,6 +94,7 @@ EXPORT_SYMBOL(jbd2_journal_file_inode);
>  EXPORT_SYMBOL(jbd2_journal_init_jbd_inode);
>  EXPORT_SYMBOL(jbd2_journal_release_jbd_inode);
>  EXPORT_SYMBOL(jbd2_journal_begin_ordered_truncate);
> +EXPORT_SYMBOL(jbd2_inode_cache);
>
>  static int journal_convert_superblock_v1(journal_t *, journal_superblock_t *);
>  static void __journal_abort_soft (journal_t *journal, int errno);
> @@ -2286,17 +2287,19 @@ static void __exit jbd2_remove_jbd_stats_proc_entry(void)
>
>  #endif
>
> -struct kmem_cache *jbd2_handle_cache;
> +struct kmem_cache *jbd2_handle_cache, *jbd2_inode_cache;
>
>  static int __init journal_init_handle_cache(void)
>  {
> -       jbd2_handle_cache = kmem_cache_create("jbd2_journal_handle",
> -                               sizeof(handle_t),
> -                               0,              /* offset */
> -                               SLAB_TEMPORARY, /* flags */
> -                               NULL);          /* ctor */
> +       jbd2_handle_cache = KMEM_CACHE(jbd2_journal_handle, SLAB_TEMPORARY);
>        if (jbd2_handle_cache == NULL) {
> -               printk(KERN_EMERG "JBD: failed to create handle cache\n");
> +               printk(KERN_EMERG "JBD2: failed to create handle cache\n");
> +               return -ENOMEM;
> +       }
> +       jbd2_inode_cache = KMEM_CACHE(jbd2_inode, 0);
> +       if (jbd2_inode_cache == NULL) {
> +               printk(KERN_EMERG "JBD2: failed to create inode cache\n");
> +               kmem_cache_destroy(jbd2_handle_cache);
>                return -ENOMEM;
>        }
>        return 0;
> @@ -2306,6 +2309,9 @@ static void jbd2_journal_destroy_handle_cache(void)
>  {
>        if (jbd2_handle_cache)
>                kmem_cache_destroy(jbd2_handle_cache);
> +       if (jbd2_inode_cache)
> +               kmem_cache_destroy(jbd2_inode_cache);
> +
>  }
>
>  /*
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 2ae86aa..27e79c2 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -94,7 +94,7 @@ extern void jbd2_free(void *ptr, size_t size);
>  *
>  * This is an opaque datatype.
>  **/
> -typedef struct handle_s                handle_t;       /* Atomic operation type */
> +typedef struct jbd2_journal_handle handle_t;   /* Atomic operation type */
>
>
>  /**
> @@ -416,7 +416,7 @@ struct jbd2_revoke_table_s;
>  * in so it can be fixed later.
>  */
>
> -struct handle_s
> +struct jbd2_journal_handle
>  {
>        /* Which compound transaction is this update a part of? */
>        transaction_t           *h_transaction;
> @@ -1158,6 +1158,22 @@ static inline void jbd2_free_handle(handle_t *handle)
>        kmem_cache_free(jbd2_handle_cache, handle);
>  }
>
> +/*
> + * jbd2_inode management (optional, for those file systems that want to use
> + * dynamically allocated jbd2_inode structures)
> + */
> +extern struct kmem_cache *jbd2_inode_cache;
> +
> +static inline struct jbd2_inode *jbd2_alloc_inode(gfp_t gfp_flags)
> +{
> +       return kmem_cache_alloc(jbd2_inode_cache, gfp_flags);
> +}
> +
> +static inline void jbd2_free_inode(struct jbd2_inode *jinode)
> +{
> +       kmem_cache_free(jbd2_inode_cache, jinode);
> +}
> +
>  /* Primary revoke support */
>  #define JOURNAL_REVOKE_DEFAULT_HASH 256
>  extern int        jbd2_journal_init_revoke(journal_t *, int);
> --
> 1.7.3.1
>
> --
> 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
>
--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ