| 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
| ||
|
Date: Fri, 9 Aug 2019 15:23:23 -0600
From: Andreas Dilger <adilger@...ger.ca>
To: Harshad Shirwadkar <harshadshirwadkar@...il.com>
Cc: linux-ext4@...r.kernel.org
Subject: Re: [PATCH v2 07/12] ext4: add fields that are needed to track
changed files
On Aug 8, 2019, at 9:45 PM, Harshad Shirwadkar <harshadshirwadkar@...il.com> wrote:
>
> Ext4's fast commit feature tracks changed files and maintains them in
> a queue. We also remember for each file the logical block range that
> needs to be committed. This patch adds these fields to ext4_inode_info
> and ext4_sb_info and also adds initialization calls.
>
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@...il.com>
>
> ---
>
> Changelog:
>
> V2: Converted s_fc_lock from mutex to spinlock to improve parallelism
> performance.
> ---
> fs/ext4/ext4.h | 34 ++++++++++++++++++++++++++++++++++
> fs/ext4/ext4_jbd2.c | 13 +++++++++++++
> fs/ext4/ext4_jbd2.h | 2 ++
> fs/ext4/inode.c | 1 +
> fs/ext4/super.c | 7 +++++++
> 5 files changed, 57 insertions(+)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index becbda38b7db..0d15d4539dda 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -921,6 +921,27 @@ enum {
> I_DATA_SEM_QUOTA,
> };
>
> +/*
> + * Ext4 fast commit inode specific information
> + */
> +struct ext4_fast_commit_inode_info {
> + /* TID / SUB-TID when old_i_size and i_size were recorded */
> + tid_t fc_tid;
> + tid_t fc_subtid;
> +
> + /*
> + * Start of logical block range that needs to be committed in this fast
> + * commit
> + */
> + loff_t fc_lblk_start;
> +
> + /*
> + * End of logical block range that needs to be committed in this fast
> + * commit
> + */
> + loff_t fc_lblk_end;
Since these are logical block numbers within the journal, they certainly
don't need to be 64-bit values. loff_t is for byte offsets, this should
use ext4_lblk_t, which will also reduce the size of the struct by 8 bytes.
> +};
> +
>
> /*
> * fourth extended file system inode data in memory
> @@ -955,6 +976,9 @@ struct ext4_inode_info {
>
> struct list_head i_orphan; /* unlinked but open inodes */
>
> + struct list_head i_fc_list; /* inodes that need fast commit */
This comment should document what lock is protecting this list, along
with the other fields.
> + struct ext4_fast_commit_inode_info i_fc;
Since this increases the size of the inode, does it affect the number of
inodes that can fit into one page of ext4_inode_cachep?
> /*
> * i_disksize keeps track of what the inode size is ON DISK, not
> * in memory. During truncate, i_size is set to the new size by
> @@ -1529,6 +1553,16 @@ struct ext4_sb_info {
> /* Barrier between changing inodes' journal flags and writepages ops. */
> struct percpu_rw_semaphore s_journal_flag_rwsem;
> struct dax_device *s_daxdev;
> +
> + /* Ext4 fast commit stuff */
> + bool fc_replay; /* Fast commit replay in progress */
> + struct list_head s_fc_q; /* Inodes that need fast commit. */
This comment should document what lock is protecting this list, along
with the other fields.
> + __u32 s_fc_q_cnt; /* Number of inodes in the fc queue */
> + bool s_fc_eligible; /*
> + * Are changes after the last commit
> + * eligible for fast commit?
> + */
It is slightly more space efficient to put the bool values together
rather than interleaving them between 64-bit values.
> + spinlock_t s_fc_lock;
> };
>
> static inline struct ext4_sb_info *EXT4_SB(struct super_block *sb)
> diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
> index 7c70b08d104c..75b6db808837 100644
> --- a/fs/ext4/ext4_jbd2.c
> +++ b/fs/ext4/ext4_jbd2.c
> @@ -330,3 +330,16 @@ int __ext4_handle_dirty_super(const char *where, unsigned int line,
> mark_buffer_dirty(bh);
> return err;
> }
> +
> +void ext4_init_inode_fc_info(struct inode *inode)
> +{
> + handle_t *handle = ext4_journal_current_handle();
> + struct ext4_inode_info *ei = EXT4_I(inode);
> +
> + memset(&ei->i_fc, 0, sizeof(ei->i_fc));
> + if (ext4_handle_valid(handle)) {
> + ei->i_fc.fc_tid = handle->h_transaction->t_tid;
> + ei->i_fc.fc_subtid = handle->h_transaction->t_journal->j_subtid;
> + }
> + INIT_LIST_HEAD(&ei->i_fc_list);
> +}
> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
> index ef8fcf7d0d3b..2305c1acd415 100644
> --- a/fs/ext4/ext4_jbd2.h
> +++ b/fs/ext4/ext4_jbd2.h
> @@ -459,4 +459,6 @@ static inline int ext4_should_dioread_nolock(struct inode *inode)
> return 1;
> }
>
> +void ext4_init_inode_fc_info(struct inode *inode);
> +
> #endif /* _EXT4_JBD2_H */
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 420fe3deed39..f230a888eddd 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4996,6 +4996,7 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
> for (block = 0; block < EXT4_N_BLOCKS; block++)
> ei->i_data[block] = raw_inode->i_block[block];
> INIT_LIST_HEAD(&ei->i_orphan);
> + ext4_init_inode_fc_info(&ei->vfs_inode);
>
> /*
> * Set transaction id's of transactions that have to be committed
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 6bab59ae81f7..0b833e9b61c1 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1100,6 +1100,7 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
> ei->i_datasync_tid = 0;
> atomic_set(&ei->i_unwritten, 0);
> INIT_WORK(&ei->i_rsv_conversion_work, ext4_end_io_rsv_work);
> + ext4_init_inode_fc_info(&ei->vfs_inode);
> return &ei->vfs_inode;
> }
>
> @@ -1139,6 +1140,7 @@ static void init_once(void *foo)
> init_rwsem(&ei->i_data_sem);
> init_rwsem(&ei->i_mmap_sem);
> inode_init_once(&ei->vfs_inode);
> + ext4_init_inode_fc_info(&ei->vfs_inode);
> }
>
> static int __init init_inodecache(void)
> @@ -4301,6 +4303,11 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> INIT_LIST_HEAD(&sbi->s_orphan); /* unlinked but open files */
> mutex_init(&sbi->s_orphan_lock);
>
> + INIT_LIST_HEAD(&sbi->s_fc_q);
> + sbi->s_fc_q_cnt = 0;
> + sbi->s_fc_eligible = true;
> + spin_lock_init(&sbi->s_fc_lock);
> +
> sb->s_root = NULL;
>
> needs_recovery = (es->s_last_orphan != 0 ||
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
Cheers, Andreas
Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)
Powered by blists - more mailing lists