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: <20130527144727.GA3877@quack.suse.cz> 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-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