From: Shentino Subject: Re: [PATCH 11/12] ext4: fix the number of credits needed for acl ops with inline data Date: Sun, 10 Feb 2013 10:15:14 -0800 Message-ID: References: <1360446832-12724-1-git-send-email-tytso@mit.edu> <1360446832-12724-12-git-send-email-tytso@mit.edu> <5117A3E2.6090802@tao.ma> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: "Theodore Ts'o" , Ext4 Developers List , Tao Ma To: Tao Ma Return-path: Received: from mail-ve0-f182.google.com ([209.85.128.182]:53120 "EHLO mail-ve0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756178Ab3BJSPz (ORCPT ); Sun, 10 Feb 2013 13:15:55 -0500 Received: by mail-ve0-f182.google.com with SMTP id ox1so4619094veb.13 for ; Sun, 10 Feb 2013 10:15:54 -0800 (PST) In-Reply-To: <5117A3E2.6090802@tao.ma> Sender: linux-ext4-owner@vger.kernel.org List-ID: Quick question from a n00b. What are "credits"? I imagine I'm not the only onlooker curious about ti either. On Sun, Feb 10, 2013 at 5:42 AM, Tao Ma wrote: > On 02/10/2013 05:53 AM, Theodore Ts'o wrote: >> Operations which modify extended attributes may need extra journal >> credits if inline data is used, since there is a chance that some >> extended attributes may need to get pushed to an external attribute >> block. >> >> Changes to reflect this was made in xattr.c, but they were missed in >> fs/ext4/acl.c. To fix this, abstract the calculation of the number of >> credits needed for xattr operations to an inline function defined in >> ext4_jbd2.h, and use it in acl.c and xattr.c. > Oh, yes, I forgot about the acl. > > Reviewed-by: Tao Ma > > Thanks, > Tao >> >> Also move the function declarations used in inline.c from xattr.h >> (where they are non-obviously hidden, and caused problems since >> ext4_jbd2.h needs to use the function ext4_has_inline_data), and move >> them to ext4.h. >> >> Signed-off-by: "Theodore Ts'o" >> Cc: Tao Ma >> --- >> fs/ext4/acl.c | 4 +-- >> fs/ext4/ext4.h | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> fs/ext4/ext4_jbd2.h | 14 +++++++++++ >> fs/ext4/xattr.c | 9 +------ >> fs/ext4/xattr.h | 68 --------------------------------------------------- >> 5 files changed, 87 insertions(+), 78 deletions(-) >> >> diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c >> index 406cf8b..39a54a0 100644 >> --- a/fs/ext4/acl.c >> +++ b/fs/ext4/acl.c >> @@ -325,7 +325,7 @@ ext4_acl_chmod(struct inode *inode) >> return error; >> retry: >> handle = ext4_journal_start(inode, EXT4_HT_XATTR, >> - EXT4_DATA_TRANS_BLOCKS(inode->i_sb)); >> + ext4_jbd2_credits_xattr(inode)); >> if (IS_ERR(handle)) { >> error = PTR_ERR(handle); >> ext4_std_error(inode->i_sb, error); >> @@ -423,7 +423,7 @@ ext4_xattr_set_acl(struct dentry *dentry, const char *name, const void *value, >> >> retry: >> handle = ext4_journal_start(inode, EXT4_HT_XATTR, >> - EXT4_DATA_TRANS_BLOCKS(inode->i_sb)); >> + ext4_jbd2_credits_xattr(inode)); >> if (IS_ERR(handle)) { >> error = PTR_ERR(handle); >> goto release_and_out; >> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h >> index c2c64da..9897cdf 100644 >> --- a/fs/ext4/ext4.h >> +++ b/fs/ext4/ext4.h >> @@ -2443,6 +2443,75 @@ extern const struct file_operations ext4_file_operations; >> extern loff_t ext4_llseek(struct file *file, loff_t offset, int origin); >> extern void ext4_unwritten_wait(struct inode *inode); >> >> +/* inline.c */ >> +extern int ext4_has_inline_data(struct inode *inode); >> +extern int ext4_get_inline_size(struct inode *inode); >> +extern int ext4_get_max_inline_size(struct inode *inode); >> +extern int ext4_find_inline_data_nolock(struct inode *inode); >> +extern void ext4_write_inline_data(struct inode *inode, >> + struct ext4_iloc *iloc, >> + void *buffer, loff_t pos, >> + unsigned int len); >> +extern int ext4_prepare_inline_data(handle_t *handle, struct inode *inode, >> + unsigned int len); >> +extern int ext4_init_inline_data(handle_t *handle, struct inode *inode, >> + unsigned int len); >> +extern int ext4_destroy_inline_data(handle_t *handle, struct inode *inode); >> + >> +extern int ext4_readpage_inline(struct inode *inode, struct page *page); >> +extern int ext4_try_to_write_inline_data(struct address_space *mapping, >> + struct inode *inode, >> + loff_t pos, unsigned len, >> + unsigned flags, >> + struct page **pagep); >> +extern int ext4_write_inline_data_end(struct inode *inode, >> + loff_t pos, unsigned len, >> + unsigned copied, >> + struct page *page); >> +extern struct buffer_head * >> +ext4_journalled_write_inline_data(struct inode *inode, >> + unsigned len, >> + struct page *page); >> +extern int ext4_da_write_inline_data_begin(struct address_space *mapping, >> + struct inode *inode, >> + loff_t pos, unsigned len, >> + unsigned flags, >> + struct page **pagep, >> + void **fsdata); >> +extern int ext4_da_write_inline_data_end(struct inode *inode, loff_t pos, >> + unsigned len, unsigned copied, >> + struct page *page); >> +extern int ext4_try_add_inline_entry(handle_t *handle, struct dentry *dentry, >> + struct inode *inode); >> +extern int ext4_try_create_inline_dir(handle_t *handle, >> + struct inode *parent, >> + struct inode *inode); >> +extern int ext4_read_inline_dir(struct file *filp, >> + void *dirent, filldir_t filldir, >> + int *has_inline_data); >> +extern struct buffer_head *ext4_find_inline_entry(struct inode *dir, >> + const struct qstr *d_name, >> + struct ext4_dir_entry_2 **res_dir, >> + int *has_inline_data); >> +extern int ext4_delete_inline_entry(handle_t *handle, >> + struct inode *dir, >> + struct ext4_dir_entry_2 *de_del, >> + struct buffer_head *bh, >> + int *has_inline_data); >> +extern int empty_inline_dir(struct inode *dir, int *has_inline_data); >> +extern struct buffer_head *ext4_get_first_inline_block(struct inode *inode, >> + struct ext4_dir_entry_2 **parent_de, >> + int *retval); >> +extern int ext4_inline_data_fiemap(struct inode *inode, >> + struct fiemap_extent_info *fieinfo, >> + int *has_inline); >> +extern int ext4_try_to_evict_inline_data(handle_t *handle, >> + struct inode *inode, >> + int needed); >> +extern void ext4_inline_data_truncate(struct inode *inode, int *has_inline); >> + >> +extern int ext4_convert_inline_data(struct inode *inode); >> + >> /* namei.c */ >> extern const struct inode_operations ext4_dir_inode_operations; >> extern const struct inode_operations ext4_special_inode_operations; >> @@ -2539,6 +2608,7 @@ extern void ext4_mmp_csum_set(struct super_block *sb, struct mmp_struct *mmp); >> extern int ext4_mmp_csum_verify(struct super_block *sb, >> struct mmp_struct *mmp); >> >> + >> /* BH_Uninit flag: blocks are allocated but uninitialized on disk */ >> enum ext4_state_bits { >> BH_Uninit /* blocks are allocated but uninitialized on disk */ >> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h >> index c1fc2dc..4c216b1 100644 >> --- a/fs/ext4/ext4_jbd2.h >> +++ b/fs/ext4/ext4_jbd2.h >> @@ -104,6 +104,20 @@ >> #define EXT4_MAXQUOTAS_INIT_BLOCKS(sb) (MAXQUOTAS*EXT4_QUOTA_INIT_BLOCKS(sb)) >> #define EXT4_MAXQUOTAS_DEL_BLOCKS(sb) (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/xattr.c b/fs/ext4/xattr.c >> index 2efc560..cc31da0 100644 >> --- a/fs/ext4/xattr.c >> +++ b/fs/ext4/xattr.c >> @@ -1165,16 +1165,9 @@ ext4_xattr_set(struct inode *inode, int name_index, const char *name, >> { >> handle_t *handle; >> int error, retries = 0; >> - int credits = EXT4_DATA_TRANS_BLOCKS(inode->i_sb); >> + int credits = ext4_jbd2_credits_xattr(inode); >> >> retry: >> - /* >> - * In case of inline data, we may push out the data to a block, >> - * So reserve the journal space first. >> - */ >> - if (ext4_has_inline_data(inode)) >> - credits += ext4_writepage_trans_blocks(inode) + 1; >> - >> 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 69eda78..aa25deb 100644 >> --- a/fs/ext4/xattr.h >> +++ b/fs/ext4/xattr.h >> @@ -125,74 +125,6 @@ extern int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode, >> struct ext4_xattr_info *i, >> struct ext4_xattr_ibody_find *is); >> >> -extern int ext4_has_inline_data(struct inode *inode); >> -extern int ext4_get_inline_size(struct inode *inode); >> -extern int ext4_get_max_inline_size(struct inode *inode); >> -extern int ext4_find_inline_data_nolock(struct inode *inode); >> -extern void ext4_write_inline_data(struct inode *inode, >> - struct ext4_iloc *iloc, >> - void *buffer, loff_t pos, >> - unsigned int len); >> -extern int ext4_prepare_inline_data(handle_t *handle, struct inode *inode, >> - unsigned int len); >> -extern int ext4_init_inline_data(handle_t *handle, struct inode *inode, >> - unsigned int len); >> -extern int ext4_destroy_inline_data(handle_t *handle, struct inode *inode); >> - >> -extern int ext4_readpage_inline(struct inode *inode, struct page *page); >> -extern int ext4_try_to_write_inline_data(struct address_space *mapping, >> - struct inode *inode, >> - loff_t pos, unsigned len, >> - unsigned flags, >> - struct page **pagep); >> -extern int ext4_write_inline_data_end(struct inode *inode, >> - loff_t pos, unsigned len, >> - unsigned copied, >> - struct page *page); >> -extern struct buffer_head * >> -ext4_journalled_write_inline_data(struct inode *inode, >> - unsigned len, >> - struct page *page); >> -extern int ext4_da_write_inline_data_begin(struct address_space *mapping, >> - struct inode *inode, >> - loff_t pos, unsigned len, >> - unsigned flags, >> - struct page **pagep, >> - void **fsdata); >> -extern int ext4_da_write_inline_data_end(struct inode *inode, loff_t pos, >> - unsigned len, unsigned copied, >> - struct page *page); >> -extern int ext4_try_add_inline_entry(handle_t *handle, struct dentry *dentry, >> - struct inode *inode); >> -extern int ext4_try_create_inline_dir(handle_t *handle, >> - struct inode *parent, >> - struct inode *inode); >> -extern int ext4_read_inline_dir(struct file *filp, >> - void *dirent, filldir_t filldir, >> - int *has_inline_data); >> -extern struct buffer_head *ext4_find_inline_entry(struct inode *dir, >> - const struct qstr *d_name, >> - struct ext4_dir_entry_2 **res_dir, >> - int *has_inline_data); >> -extern int ext4_delete_inline_entry(handle_t *handle, >> - struct inode *dir, >> - struct ext4_dir_entry_2 *de_del, >> - struct buffer_head *bh, >> - int *has_inline_data); >> -extern int empty_inline_dir(struct inode *dir, int *has_inline_data); >> -extern struct buffer_head *ext4_get_first_inline_block(struct inode *inode, >> - struct ext4_dir_entry_2 **parent_de, >> - int *retval); >> -extern int ext4_inline_data_fiemap(struct inode *inode, >> - struct fiemap_extent_info *fieinfo, >> - int *has_inline); >> -extern int ext4_try_to_evict_inline_data(handle_t *handle, >> - struct inode *inode, >> - int needed); >> -extern void ext4_inline_data_truncate(struct inode *inode, int *has_inline); >> - >> -extern int ext4_convert_inline_data(struct inode *inode); >> - >> #ifdef CONFIG_EXT4_FS_SECURITY >> extern int ext4_init_security(handle_t *handle, struct inode *inode, >> struct inode *dir, const struct qstr *qstr); >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html