| 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: Mon, 27 May 2013 16:47:27 +0200
From: Jan Kara <jack@...e.cz>
To: Li Wang <liwang@...ntukylin.com>
Cc: linux-ext4@...r.kernel.org, Theodore Ts'o <tytso@....edu>,
Andreas Dilger <adilger.kernel@...ger.ca>,
Dmitry Monakhov <dmonakhov@...nvz.org>,
Zheng Liu <gnehzuil.liu@...il.com>,
linux-kernel@...r.kernel.org,
Yunchuan Wen <yunchuanwen@...ntukylin.com>,
Jan Kara <jack@...e.cz>
Subject: Re: [PATCH] ext4: Avoid unnecessarily writing back dirty pages
before hole punching
On Mon 27-05-13 22:25:36, Li Wang wrote:
> For hole punching, currently ext4 will synchronously write back the
> dirty pages fit into the hole, since the data on the disk responding
> to those pages are to be deleted, it is benefical to directly release
> those pages, no matter they are dirty or not, except the ordered case.
>
> Signed-off-by: Li Wang <liwang@...ntukylin.com>
> Signed-off-by: Yunchuan Wen <yunchuanwen@...ntukylin.com>
> Cc: Dmitry Monakhov <dmonakhov@...nvz.org>
> Cc: Jan Kara <jack@...e.cz>
The patch looks good to me except for one nitpick below. Feel free to
add:
Reviewed-by: Jan Kara <jack@...e.cz>
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -2305,29 +2305,10 @@ done:
> return 0;
> }
>
> -/*
> - * File truncate and transaction commit interact with each other in a
> - * non-trivial way. If a transaction writing data block A is
> - * committing, we cannot discard the data by truncate until we have
> - * written them. Otherwise if we crashed after the transaction with
> - * write has committed but before the transaction with truncate has
> - * committed, we could see stale data in block A. This function is a
> - * helper to solve this problem. It starts writeout of the truncated
> - * part in case it is in the committing transaction.
> - *
> - * Filesystem code must call this function when inode is journaled in
> - * ordered mode before truncation happens and after the inode has been
> - * placed on orphan list with the new inode size. The second condition
> - * avoids the race that someone writes new data and we start
> - * committing the transaction after this function has been called but
> - * before a transaction for truncate is started (and furthermore it
> - * allows us to optimize the case where the addition to orphan list
> - * happens in the same transaction as write --- we don't have to write
> - * any data in such case).
> - */
> -int jbd2_journal_begin_ordered_truncate(journal_t *journal,
> +
> +int jbd2_journal_begin_ordered_discard(journal_t *journal,
> struct jbd2_inode *jinode,
> - loff_t new_size)
> + loff_t start, loff_t end)
Why don't you call this function jbd2_journal_begin_ordered_punch_hole()?
They are the same (well, except the end vs length difference but that seems
minor).
Honza
> {
> transaction_t *inode_trans, *commit_trans;
> int ret = 0;
> @@ -2346,10 +2327,12 @@ int jbd2_journal_begin_ordered_truncate(journal_t *journal,
> spin_unlock(&journal->j_list_lock);
> if (inode_trans == commit_trans) {
> ret = filemap_fdatawrite_range(jinode->i_vfs_inode->i_mapping,
> - new_size, LLONG_MAX);
> + start, end);
> if (ret)
> jbd2_journal_abort(journal, ret);
> }
> out:
> return ret;
> }
> +
> +
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 6e051f4..9543a5a 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -1126,12 +1126,49 @@ extern int jbd2_journal_clear_err (journal_t *);
> extern int jbd2_journal_bmap(journal_t *, unsigned long, unsigned long long *);
> extern int jbd2_journal_force_commit(journal_t *);
> extern int jbd2_journal_file_inode(handle_t *handle, struct jbd2_inode *inode);
> -extern int jbd2_journal_begin_ordered_truncate(journal_t *journal,
> - struct jbd2_inode *inode, loff_t new_size);
> +extern int jbd2_journal_begin_ordered_discard(journal_t *,
> + struct jbd2_inode *,
> + loff_t, loff_t);
> extern void jbd2_journal_init_jbd_inode(struct jbd2_inode *jinode, struct inode *inode);
> extern void jbd2_journal_release_jbd_inode(journal_t *journal, struct jbd2_inode *jinode);
>
> /*
> + * File truncate and transaction commit interact with each other in a
> + * non-trivial way. If a transaction writing data block A is
> + * committing, we cannot discard the data by truncate until we have
> + * written them. Otherwise if we crashed after the transaction with
> + * write has committed but before the transaction with truncate has
> + * committed, we could see stale data in block A. This function is a
> + * helper to solve this problem. It starts writeout of the truncated
> + * part in case it is in the committing transaction.
> + *
> + * Filesystem code must call this function when inode is journaled in
> + * ordered mode before truncation happens and after the inode has been
> + * placed on orphan list with the new inode size. The second condition
> + * avoids the race that someone writes new data and we start
> + * committing the transaction after this function has been called but
> + * before a transaction for truncate is started (and furthermore it
> + * allows us to optimize the case where the addition to orphan list
> + * happens in the same transaction as write --- we don't have to write
> + * any data in such case).
> + */
> +static inline int jbd2_journal_begin_ordered_truncate(journal_t *journal,
> + struct jbd2_inode *jinode,
> + loff_t new_size)
> +{
> + return jbd2_journal_begin_ordered_discard(journal, jinode,
> + new_size, LLONG_MAX);
> +}
> +
> +static inline int jbd2_journal_begin_ordered_punch_hole(journal_t *journal,
> + struct jbd2_inode *jinode,
> + loff_t start, loff_t length)
> +{
> + return jbd2_journal_begin_ordered_discard(journal, jinode,
> + start, start + length - 1);
> +}
> +
> +/*
> * journal_head management
> */
> struct journal_head *jbd2_journal_add_journal_head(struct buffer_head *bh);
> --
> 1.7.9.5
>
>
--
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists