From ef5dc1f7d039e2098e09207e0bcc44a8aaed2f7f Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Wed, 21 Jun 2017 22:30:03 -0400 Subject: [PATCH] add patch improve-journal-credit-handling-in-set-xattr-paths --- improve-journal-credit-handling-in-set-xattr-paths | 198 +++++++++++++++++++++ series | 1 + timestamps | 7 +- 3 files changed, 203 insertions(+), 3 deletions(-) create mode 100644 improve-journal-credit-handling-in-set-xattr-paths diff --git a/improve-journal-credit-handling-in-set-xattr-paths b/improve-journal-credit-handling-in-set-xattr-paths new file mode 100644 index 00000000..2b06bcfa --- /dev/null +++ b/improve-journal-credit-handling-in-set-xattr-paths @@ -0,0 +1,198 @@ +ext4: improve journal credit handling in set xattr paths + +From: Tahsin Erdogan + +Both ext4_set_acl() and ext4_set_context() need to be made aware of +ea_inode feature when it comes to credits calculation. + +Also add a sufficient credits check in ext4_xattr_set_handle() right +after xattr write lock is grabbed. Original credits calculation is done +outside the lock so there is a possiblity that the initially calculated +credits are not sufficient anymore. + +Signed-off-by: Tahsin Erdogan +Signed-off-by: Theodore Ts'o +--- +v2: fixed checkpatch.pl warning about replacing spaces with tab + + fs/ext4/acl.c | 7 ++++--- + fs/ext4/ext4_jbd2.h | 14 -------------- + fs/ext4/super.c | 6 +++--- + fs/ext4/xattr.c | 55 +++++++++++++++++++++++++++++++++++++++++------------ + fs/ext4/xattr.h | 1 + + 5 files changed, 51 insertions(+), 32 deletions(-) + +diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c +index 3ec0e46de95f..74f7ac539e00 100644 +--- a/fs/ext4/acl.c ++++ b/fs/ext4/acl.c +@@ -231,14 +231,15 @@ int + ext4_set_acl(struct inode *inode, struct posix_acl *acl, int type) + { + handle_t *handle; +- int error, retries = 0; ++ int error, credits, retries = 0; ++ size_t acl_size = acl ? ext4_acl_size(acl->a_count) : 0; + + error = dquot_initialize(inode); + if (error) + return error; + retry: +- handle = ext4_journal_start(inode, EXT4_HT_XATTR, +- ext4_jbd2_credits_xattr(inode)); ++ credits = ext4_xattr_set_credits(inode, acl_size); ++ handle = ext4_journal_start(inode, EXT4_HT_XATTR, credits); + if (IS_ERR(handle)) + return PTR_ERR(handle); + +diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h +index f97611171023..a5bda70feed5 100644 +--- a/fs/ext4/ext4_jbd2.h ++++ b/fs/ext4/ext4_jbd2.h +@@ -104,20 +104,6 @@ + #define EXT4_MAXQUOTAS_INIT_BLOCKS(sb) (EXT4_MAXQUOTAS*EXT4_QUOTA_INIT_BLOCKS(sb)) + #define EXT4_MAXQUOTAS_DEL_BLOCKS(sb) (EXT4_MAXQUOTAS*EXT4_QUOTA_DEL_BLOCKS(sb)) + +-static inline int ext4_jbd2_credits_xattr(struct inode *inode) +-{ +- int credits = EXT4_DATA_TRANS_BLOCKS(inode->i_sb); +- +- /* +- * In case of inline data, we may push out the data to a block, +- * so we need to reserve credits for this eventuality +- */ +- if (ext4_has_inline_data(inode)) +- credits += ext4_writepage_trans_blocks(inode) + 1; +- return credits; +-} +- +- + /* + * Ext4 handle operation types -- for logging purposes + */ +diff --git a/fs/ext4/super.c b/fs/ext4/super.c +index d37c81f327e7..b02a23ec92ca 100644 +--- a/fs/ext4/super.c ++++ b/fs/ext4/super.c +@@ -1143,7 +1143,7 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len, + void *fs_data) + { + handle_t *handle = fs_data; +- int res, res2, retries = 0; ++ int res, res2, credits, retries = 0; + + res = ext4_convert_inline_data(inode); + if (res) +@@ -1178,8 +1178,8 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len, + if (res) + return res; + retry: +- handle = ext4_journal_start(inode, EXT4_HT_MISC, +- ext4_jbd2_credits_xattr(inode)); ++ credits = ext4_xattr_set_credits(inode, len); ++ handle = ext4_journal_start(inode, EXT4_HT_MISC, credits); + if (IS_ERR(handle)) + return PTR_ERR(handle); + +diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c +index 97d33ecf0818..fd017faaf221 100644 +--- a/fs/ext4/xattr.c ++++ b/fs/ext4/xattr.c +@@ -1473,6 +1473,17 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index, + + ext4_write_lock_xattr(inode, &no_expand); + ++ /* Check journal credits under write lock. */ ++ if (ext4_handle_valid(handle)) { ++ int credits; ++ ++ credits = ext4_xattr_set_credits(inode, value_len); ++ if (!ext4_handle_has_enough_credits(handle, credits)) { ++ error = -ENOSPC; ++ goto cleanup; ++ } ++ } ++ + error = ext4_reserve_inode_write(handle, inode, &is.iloc); + if (error) + goto cleanup; +@@ -1570,6 +1581,36 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index, + return error; + } + ++int ext4_xattr_set_credits(struct inode *inode, size_t value_len) ++{ ++ struct super_block *sb = inode->i_sb; ++ int credits; ++ ++ if (!EXT4_SB(sb)->s_journal) ++ return 0; ++ ++ credits = EXT4_DATA_TRANS_BLOCKS(inode->i_sb); ++ ++ /* ++ * In case of inline data, we may push out the data to a block, ++ * so we need to reserve credits for this eventuality ++ */ ++ if (ext4_has_inline_data(inode)) ++ credits += ext4_writepage_trans_blocks(inode) + 1; ++ ++ if (ext4_has_feature_ea_inode(sb)) { ++ int nrblocks = (value_len + sb->s_blocksize - 1) >> ++ sb->s_blocksize_bits; ++ ++ /* For new inode */ ++ credits += EXT4_SINGLEDATA_TRANS_BLOCKS(sb) + 3; ++ ++ /* For data blocks of EA inode */ ++ credits += ext4_meta_trans_blocks(inode, nrblocks, 0); ++ } ++ return credits; ++} ++ + /* + * ext4_xattr_set() + * +@@ -1585,24 +1626,14 @@ ext4_xattr_set(struct inode *inode, int name_index, const char *name, + handle_t *handle; + struct super_block *sb = inode->i_sb; + int error, retries = 0; +- int credits = ext4_jbd2_credits_xattr(inode); ++ int credits; + + error = dquot_initialize(inode); + if (error) + return error; + +- if (ext4_has_feature_ea_inode(sb)) { +- int nrblocks = (value_len + sb->s_blocksize - 1) >> +- sb->s_blocksize_bits; +- +- /* For new inode */ +- credits += EXT4_SINGLEDATA_TRANS_BLOCKS(sb) + 3; +- +- /* For data blocks of EA inode */ +- credits += ext4_meta_trans_blocks(inode, nrblocks, 0); +- } +- + retry: ++ credits = ext4_xattr_set_credits(inode, value_len); + handle = ext4_journal_start(inode, EXT4_HT_XATTR, credits); + if (IS_ERR(handle)) { + error = PTR_ERR(handle); +diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h +index b6ef99d1a061..e82c5fe36a26 100644 +--- a/fs/ext4/xattr.h ++++ b/fs/ext4/xattr.h +@@ -160,6 +160,7 @@ extern ssize_t ext4_listxattr(struct dentry *, char *, size_t); + extern int ext4_xattr_get(struct inode *, int, const char *, void *, size_t); + extern int ext4_xattr_set(struct inode *, int, const char *, const void *, size_t, int); + extern int ext4_xattr_set_handle(handle_t *, struct inode *, int, const char *, const void *, size_t, int); ++extern int ext4_xattr_set_credits(struct inode *inode, size_t value_len); + + extern int ext4_xattr_inode_unlink(struct inode *inode, unsigned long ea_ino); + extern int ext4_xattr_delete_inode(handle_t *handle, struct inode *inode, +-- +2.13.1.611.g7e3b11ae1-goog + + diff --git a/series b/series index 9b995726..84806b2e 100644 --- a/series +++ b/series @@ -21,6 +21,7 @@ fix-ext4_xattr_cmp fix-credits-calculation-for-xattr-inode retry-storing-value-in-external-inode-with-xattr-block-too ext4_xattr_delete_inode-should-return-accurate-errors +improve-journal-credit-handling-in-set-xattr-paths #################################################### # unstable patches diff --git a/timestamps b/timestamps index 1f673ff8..5226eb94 100755 --- a/timestamps +++ b/timestamps @@ -53,6 +53,7 @@ touch -d @1498097670 fix-ext4_xattr_cmp touch -d @1498097780 fix-credits-calculation-for-xattr-inode touch -d @1498098032 retry-storing-value-in-external-inode-with-xattr-block-too touch -d @1498098278 ext4_xattr_delete_inode-should-return-accurate-errors -touch -d @1498098343 series -touch -d @1498098348 status -touch -d @1498098358 timestamps +touch -d @1498098520 improve-journal-credit-handling-in-set-xattr-paths +touch -d @1498098547 series +touch -d @1498098552 status +touch -d @1498098559 timestamps -- 2.11.4.GIT