From: Andreas Dilger <adilger@dilger.ca> To: Tahsin Erdogan <tahsin@google.com> Cc: Jan Kara <jack@suse.com>, "Theodore Ts'o" <tytso@mit.edu>, Dave Kleikamp <shaggy@kernel.org>, Alexander Viro <viro@zeniv.linux.org.uk>, Mark Fasheh <mfasheh@versity.com>, Joel Becker <jlbec@evilplan.org>, Jens Axboe <axboe@fb.com>, Deepa Dinamani <deepa.kernel@gmail.com>, Mike Christie <mchristi@redhat.com>, Fabian Frederick <fabf@skynet.be>, linux-ext4 <linux-ext4@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>, jfs-discussion@lists.sourceforge.net, linux-fsdevel <linux-fsdevel@vger.kernel.org>, ocfs2-devel@oss.oracle.com, reiserfs-devel@vger.kernel.org Subject: Re: [PATCH v2 26/28] ext4: cleanup transaction restarts during inode deletion Date: Wed, 14 Jun 2017 18:11:10 -0600 [thread overview] Message-ID: <A15C59A1-EC1C-4A58-927C-C086CD90A524@dilger.ca> (raw) In-Reply-To: <20170614141714.18330-1-tahsin@google.com> [-- Attachment #1: Type: text/plain, Size: 15153 bytes --] On Jun 14, 2017, at 8:17 AM, Tahsin Erdogan <tahsin@google.com> wrote: > > During inode deletion, journal credits that will be needed are hard to > determine, that is why we have journal extend/restart calls in several > places. Whenever a transaction is restarted, filesystem must be in a > consistent state because there is no atomicity guarantee beyond a > restart call. > > Add ext4_xattr_ensure_credits() helper function which takes care of > journal extend/restart logic. It also handles getting jbd2 write access > and dirty metadata calls. This function is called at every iteration of > handling an ea_inode reference. Another option that might be less complex is to just add the xattr inodes to the orphan list in the main transaction (which should be a fixed number of credits), and then truncate/unlink the xattr inodes after the main transaction has completed rather than making the transactions arbitrarily large. At one point we even had a separate unlink thread to handle this in the background to reduce the unlink latency for very large files, which also avoids issues with nested transactions. Cheers, Andreas > Signed-off-by: Tahsin Erdogan <tahsin@google.com> > --- > v2: made ext4_xattr_ensure_credits() static > > fs/ext4/inode.c | 66 ++++----------- > fs/ext4/xattr.c | 257 ++++++++++++++++++++++++++++++++++++-------------------- > fs/ext4/xattr.h | 3 +- > 3 files changed, 183 insertions(+), 143 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index cf91532765a4..4d6936f0d8a4 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -239,7 +239,11 @@ void ext4_evict_inode(struct inode *inode) > */ > sb_start_intwrite(inode->i_sb); > > - handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, extra_credits); > + if (!IS_NOQUOTA(inode)) > + extra_credits += EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb); > + > + handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, > + ext4_blocks_for_truncate(inode)+extra_credits); > if (IS_ERR(handle)) { > ext4_std_error(inode->i_sb, PTR_ERR(handle)); > /* > @@ -251,36 +255,9 @@ void ext4_evict_inode(struct inode *inode) > sb_end_intwrite(inode->i_sb); > goto no_delete; > } > + > if (IS_SYNC(inode)) > ext4_handle_sync(handle); > - > - /* > - * Delete xattr inode before deleting the main inode. > - */ > - err = ext4_xattr_delete_inode(handle, inode, &ea_inode_array); > - if (err) { > - ext4_warning(inode->i_sb, > - "couldn't delete inode's xattr (err %d)", err); > - goto stop_handle; > - } > - > - if (!IS_NOQUOTA(inode)) > - extra_credits += 2 * EXT4_QUOTA_DEL_BLOCKS(inode->i_sb); > - > - if (!ext4_handle_has_enough_credits(handle, > - ext4_blocks_for_truncate(inode) + extra_credits)) { > - err = ext4_journal_extend(handle, > - ext4_blocks_for_truncate(inode) + extra_credits); > - if (err > 0) > - err = ext4_journal_restart(handle, > - ext4_blocks_for_truncate(inode) + extra_credits); > - if (err != 0) { > - ext4_warning(inode->i_sb, > - "couldn't extend journal (err %d)", err); > - goto stop_handle; > - } > - } > - > inode->i_size = 0; > err = ext4_mark_inode_dirty(handle, inode); > if (err) { > @@ -298,25 +275,17 @@ void ext4_evict_inode(struct inode *inode) > } > } > > - /* > - * ext4_ext_truncate() doesn't reserve any slop when it > - * restarts journal transactions; therefore there may not be > - * enough credits left in the handle to remove the inode from > - * the orphan list and set the dtime field. > - */ > - if (!ext4_handle_has_enough_credits(handle, extra_credits)) { > - err = ext4_journal_extend(handle, extra_credits); > - if (err > 0) > - err = ext4_journal_restart(handle, extra_credits); > - if (err != 0) { > - ext4_warning(inode->i_sb, > - "couldn't extend journal (err %d)", err); > - stop_handle: > - ext4_journal_stop(handle); > - ext4_orphan_del(NULL, inode); > - sb_end_intwrite(inode->i_sb); > - goto no_delete; > - } > + /* Remove xattr references. */ > + err = ext4_xattr_delete_inode(handle, inode, &ea_inode_array, > + extra_credits); > + if (err) { > + ext4_warning(inode->i_sb, "xattr delete (err %d)", err); > + stop_handle: > + ext4_journal_stop(handle); > + ext4_orphan_del(NULL, inode); > + sb_end_intwrite(inode->i_sb); > + ext4_xattr_inode_array_free(ea_inode_array); > + goto no_delete; > } > > /* > @@ -342,7 +311,6 @@ void ext4_evict_inode(struct inode *inode) > ext4_clear_inode(inode); > else > ext4_free_inode(handle, inode); > - > ext4_journal_stop(handle); > sb_end_intwrite(inode->i_sb); > ext4_xattr_inode_array_free(ea_inode_array); > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c > index 3ee7e2f68476..abc7d5f84e5f 100644 > --- a/fs/ext4/xattr.c > +++ b/fs/ext4/xattr.c > @@ -108,6 +108,10 @@ const struct xattr_handler *ext4_xattr_handlers[] = { > #define EXT4_GET_MB_CACHE(inode) (((struct ext4_sb_info *) \ > inode->i_sb->s_fs_info)->s_mb_cache) > > +static int > +ext4_expand_inode_array(struct ext4_xattr_inode_array **ea_inode_array, > + struct inode *inode); > + > #ifdef CONFIG_LOCKDEP > void ext4_xattr_inode_set_class(struct inode *ea_inode) > { > @@ -653,6 +657,127 @@ static void ext4_xattr_update_super_block(handle_t *handle, > } > } > > +static int ext4_xattr_ensure_credits(handle_t *handle, struct inode *inode, > + int credits, struct buffer_head *bh, > + bool dirty, bool block_csum) > +{ > + int error; > + > + if (!ext4_handle_valid(handle)) > + return 0; > + > + if (handle->h_buffer_credits >= credits) > + return 0; > + > + error = ext4_journal_extend(handle, credits - handle->h_buffer_credits); > + if (!error) > + return 0; > + if (error < 0) { > + ext4_warning(inode->i_sb, "Extend journal (error %d)", error); > + return error; > + } > + > + if (bh && dirty) { > + if (block_csum) > + ext4_xattr_block_csum_set(inode, bh); > + error = ext4_handle_dirty_metadata(handle, NULL, bh); > + if (error) { > + ext4_warning(inode->i_sb, "Handle metadata (error %d)", > + error); > + return error; > + } > + } > + > + error = ext4_journal_restart(handle, credits); > + if (error) { > + ext4_warning(inode->i_sb, "Restart journal (error %d)", error); > + return error; > + } > + > + if (bh) { > + error = ext4_journal_get_write_access(handle, bh); > + if (error) { > + ext4_warning(inode->i_sb, > + "Get write access failed (error %d)", > + error); > + return error; > + } > + } > + return 0; > +} > + > +static void > +ext4_xattr_inode_remove_all(handle_t *handle, struct inode *parent, > + struct buffer_head *bh, > + struct ext4_xattr_entry *first, bool block_csum, > + struct ext4_xattr_inode_array **ea_inode_array, > + int extra_credits) > +{ > + struct inode *ea_inode; > + struct ext4_xattr_entry *entry; > + bool dirty = false; > + unsigned int ea_ino; > + int err; > + int credits; > + > + /* One credit for dec ref on ea_inode, one for orphan list addition, */ > + credits = 2 + extra_credits; > + > + for (entry = first; !IS_LAST_ENTRY(entry); > + entry = EXT4_XATTR_NEXT(entry)) { > + if (!entry->e_value_inum) > + continue; > + ea_ino = le32_to_cpu(entry->e_value_inum); > + err = ext4_xattr_inode_iget(parent, ea_ino, &ea_inode); > + if (err) > + continue; > + > + err = ext4_expand_inode_array(ea_inode_array, ea_inode); > + if (err) { > + ext4_warning_inode(ea_inode, > + "Expand inode array err=%d", err); > + iput(ea_inode); > + continue; > + } > + > + err = ext4_xattr_ensure_credits(handle, parent, credits, bh, > + dirty, block_csum); > + if (err) { > + ext4_warning_inode(ea_inode, "Ensure credits err=%d", > + err); > + continue; > + } > + > + inode_lock(ea_inode); > + clear_nlink(ea_inode); > + ext4_orphan_add(handle, ea_inode); > + inode_unlock(ea_inode); > + > + /* > + * Forget about ea_inode within the same transaction that decrements the ref > + * count. This avoids duplicate decrements in case the rest of the work > + * spills over to subsequent transactions. > + */ > + entry->e_value_inum = 0; > + entry->e_value_size = 0; > + > + dirty = true; > + } > + > + if (dirty) { > + /* > + * Note that we are deliberately skipping csum calculation for > + * the final update because we do not expect any journal > + * restarts until xattr block is freed. > + */ > + > + err = ext4_handle_dirty_metadata(handle, NULL, bh); > + if (err) > + ext4_warning_inode(parent, > + "handle dirty metadata err=%d", err); > + } > +} > + > /* > * Release the xattr block BH: If the reference count is > 1, decrement it; > * otherwise free the block. > @@ -1985,42 +2110,6 @@ ext4_expand_inode_array(struct ext4_xattr_inode_array **ea_inode_array, > return 0; > } > > -/** > - * Add xattr inode to orphan list > - */ > -static int > -ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode, int credits, > - struct ext4_xattr_inode_array *ea_inode_array) > -{ > - int idx = 0, error = 0; > - struct inode *ea_inode; > - > - if (ea_inode_array == NULL) > - return 0; > - > - for (; idx < ea_inode_array->count; ++idx) { > - if (!ext4_handle_has_enough_credits(handle, credits)) { > - error = ext4_journal_extend(handle, credits); > - if (error > 0) > - error = ext4_journal_restart(handle, credits); > - > - if (error != 0) { > - ext4_warning(inode->i_sb, > - "couldn't extend journal " > - "(err %d)", error); > - return error; > - } > - } > - ea_inode = ea_inode_array->inodes[idx]; > - inode_lock(ea_inode); > - ext4_orphan_add(handle, ea_inode); > - inode_unlock(ea_inode); > - /* the inode's i_count will be released by caller */ > - } > - > - return 0; > -} > - > /* > * ext4_xattr_delete_inode() > * > @@ -2033,16 +2122,23 @@ ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode, int credits, > */ > int > ext4_xattr_delete_inode(handle_t *handle, struct inode *inode, > - struct ext4_xattr_inode_array **ea_inode_array) > + struct ext4_xattr_inode_array **ea_inode_array, > + int extra_credits) > { > struct buffer_head *bh = NULL; > struct ext4_xattr_ibody_header *header; > struct ext4_inode *raw_inode; > - struct ext4_iloc iloc; > - struct ext4_xattr_entry *entry; > - struct inode *ea_inode; > - unsigned int ea_ino; > - int credits = 3, error = 0; > + struct ext4_iloc iloc = { .bh = NULL }; > + int error; > + > + error = ext4_xattr_ensure_credits(handle, inode, extra_credits, > + NULL /* bh */, > + false /* dirty */, > + false /* block_csum */); > + if (error) { > + EXT4_ERROR_INODE(inode, "ensure credits (error %d)", error); > + goto cleanup; > + } > > if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR)) > goto delete_external_ea; > @@ -2050,31 +2146,20 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode, > error = ext4_get_inode_loc(inode, &iloc); > if (error) > goto cleanup; > + > + error = ext4_journal_get_write_access(handle, iloc.bh); > + if (error) > + goto cleanup; > + > raw_inode = ext4_raw_inode(&iloc); > header = IHDR(inode, raw_inode); > - for (entry = IFIRST(header); !IS_LAST_ENTRY(entry); > - entry = EXT4_XATTR_NEXT(entry)) { > - if (!entry->e_value_inum) > - continue; > - ea_ino = le32_to_cpu(entry->e_value_inum); > - error = ext4_xattr_inode_iget(inode, ea_ino, &ea_inode); > - if (error) > - continue; > - error = ext4_expand_inode_array(ea_inode_array, ea_inode); > - if (error) { > - iput(ea_inode); > - brelse(iloc.bh); > - goto cleanup; > - } > - entry->e_value_inum = 0; > - } > - brelse(iloc.bh); > + ext4_xattr_inode_remove_all(handle, inode, iloc.bh, IFIRST(header), > + false /* block_csum */, ea_inode_array, > + extra_credits); > > delete_external_ea: > if (!EXT4_I(inode)->i_file_acl) { > - /* add xattr inode to orphan list */ > - error = ext4_xattr_inode_orphan_add(handle, inode, credits, > - *ea_inode_array); > + error = 0; > goto cleanup; > } > bh = sb_bread(inode->i_sb, EXT4_I(inode)->i_file_acl); > @@ -2092,46 +2177,32 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode, > goto cleanup; > } > > - for (entry = BFIRST(bh); !IS_LAST_ENTRY(entry); > - entry = EXT4_XATTR_NEXT(entry)) { > - if (!entry->e_value_inum) > - continue; > - ea_ino = le32_to_cpu(entry->e_value_inum); > - error = ext4_xattr_inode_iget(inode, ea_ino, &ea_inode); > - if (error) > - continue; > - error = ext4_expand_inode_array(ea_inode_array, ea_inode); > - if (error) > - goto cleanup; > - entry->e_value_inum = 0; > - } > - > - /* add xattr inode to orphan list */ > - error = ext4_xattr_inode_orphan_add(handle, inode, credits, > - *ea_inode_array); > - if (error) > - goto cleanup; > - > - if (!IS_NOQUOTA(inode)) > - credits += 2 * EXT4_QUOTA_DEL_BLOCKS(inode->i_sb); > - > - if (!ext4_handle_has_enough_credits(handle, credits)) { > - error = ext4_journal_extend(handle, credits); > - if (error > 0) > - error = ext4_journal_restart(handle, credits); > + if (ext4_has_feature_ea_inode(inode->i_sb)) { > + error = ext4_journal_get_write_access(handle, bh); > if (error) { > - ext4_warning(inode->i_sb, > - "couldn't extend journal (err %d)", error); > + EXT4_ERROR_INODE(inode, "write access %llu", > + EXT4_I(inode)->i_file_acl); > goto cleanup; > } > + ext4_xattr_inode_remove_all(handle, inode, bh, > + BFIRST(bh), > + true /* block_csum */, > + ea_inode_array, > + extra_credits); > } > > ext4_xattr_release_block(handle, inode, bh); > + /* Update i_file_acl within the same transaction that releases block. */ > EXT4_I(inode)->i_file_acl = 0; > - > + error = ext4_mark_inode_dirty(handle, inode); > + if (error) { > + EXT4_ERROR_INODE(inode, "mark inode dirty (error %d)", > + error); > + goto cleanup; > + } > cleanup: > + brelse(iloc.bh); > brelse(bh); > - > return error; > } > > diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h > index adf761518a73..b2005a2716d9 100644 > --- a/fs/ext4/xattr.h > +++ b/fs/ext4/xattr.h > @@ -169,7 +169,8 @@ 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, > - struct ext4_xattr_inode_array **array); > + struct ext4_xattr_inode_array **array, > + int extra_credits); > extern void ext4_xattr_inode_array_free(struct ext4_xattr_inode_array *array); > > extern int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize, > -- > 2.13.1.508.gb3defc5cc-goog > Cheers, Andreas [-- Attachment #2: Message signed with OpenPGP --] [-- Type: application/pgp-signature, Size: 195 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Andreas Dilger <adilger@dilger.ca> To: Tahsin Erdogan <tahsin@google.com> Cc: Jan Kara <jack@suse.com>, Theodore Ts'o <tytso@mit.edu>, Dave Kleikamp <shaggy@kernel.org>, Alexander Viro <viro@zeniv.linux.org.uk>, Mark Fasheh <mfasheh@versity.com>, Joel Becker <jlbec@evilplan.org>, Jens Axboe <axboe@fb.com>, Deepa Dinamani <deepa.kernel@gmail.com>, Mike Christie <mchristi@redhat.com>, Fabian Frederick <fabf@skynet.be>, linux-ext4 <linux-ext4@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>, jfs-discussion@lists.sourceforge.net, linux-fsdevel <linux-fsdevel@vger.kernel.org>, ocfs2-devel@oss.oracle.com, reiserfs-devel@vger.kernel.org Subject: [Ocfs2-devel] [PATCH v2 26/28] ext4: cleanup transaction restarts during inode deletion Date: Wed, 14 Jun 2017 18:11:10 -0600 [thread overview] Message-ID: <A15C59A1-EC1C-4A58-927C-C086CD90A524@dilger.ca> (raw) In-Reply-To: <20170614141714.18330-1-tahsin@google.com> On Jun 14, 2017, at 8:17 AM, Tahsin Erdogan <tahsin@google.com> wrote: > > During inode deletion, journal credits that will be needed are hard to > determine, that is why we have journal extend/restart calls in several > places. Whenever a transaction is restarted, filesystem must be in a > consistent state because there is no atomicity guarantee beyond a > restart call. > > Add ext4_xattr_ensure_credits() helper function which takes care of > journal extend/restart logic. It also handles getting jbd2 write access > and dirty metadata calls. This function is called at every iteration of > handling an ea_inode reference. Another option that might be less complex is to just add the xattr inodes to the orphan list in the main transaction (which should be a fixed number of credits), and then truncate/unlink the xattr inodes after the main transaction has completed rather than making the transactions arbitrarily large. At one point we even had a separate unlink thread to handle this in the background to reduce the unlink latency for very large files, which also avoids issues with nested transactions. Cheers, Andreas > Signed-off-by: Tahsin Erdogan <tahsin@google.com> > --- > v2: made ext4_xattr_ensure_credits() static > > fs/ext4/inode.c | 66 ++++----------- > fs/ext4/xattr.c | 257 ++++++++++++++++++++++++++++++++++++-------------------- > fs/ext4/xattr.h | 3 +- > 3 files changed, 183 insertions(+), 143 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index cf91532765a4..4d6936f0d8a4 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -239,7 +239,11 @@ void ext4_evict_inode(struct inode *inode) > */ > sb_start_intwrite(inode->i_sb); > > - handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, extra_credits); > + if (!IS_NOQUOTA(inode)) > + extra_credits += EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb); > + > + handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, > + ext4_blocks_for_truncate(inode)+extra_credits); > if (IS_ERR(handle)) { > ext4_std_error(inode->i_sb, PTR_ERR(handle)); > /* > @@ -251,36 +255,9 @@ void ext4_evict_inode(struct inode *inode) > sb_end_intwrite(inode->i_sb); > goto no_delete; > } > + > if (IS_SYNC(inode)) > ext4_handle_sync(handle); > - > - /* > - * Delete xattr inode before deleting the main inode. > - */ > - err = ext4_xattr_delete_inode(handle, inode, &ea_inode_array); > - if (err) { > - ext4_warning(inode->i_sb, > - "couldn't delete inode's xattr (err %d)", err); > - goto stop_handle; > - } > - > - if (!IS_NOQUOTA(inode)) > - extra_credits += 2 * EXT4_QUOTA_DEL_BLOCKS(inode->i_sb); > - > - if (!ext4_handle_has_enough_credits(handle, > - ext4_blocks_for_truncate(inode) + extra_credits)) { > - err = ext4_journal_extend(handle, > - ext4_blocks_for_truncate(inode) + extra_credits); > - if (err > 0) > - err = ext4_journal_restart(handle, > - ext4_blocks_for_truncate(inode) + extra_credits); > - if (err != 0) { > - ext4_warning(inode->i_sb, > - "couldn't extend journal (err %d)", err); > - goto stop_handle; > - } > - } > - > inode->i_size = 0; > err = ext4_mark_inode_dirty(handle, inode); > if (err) { > @@ -298,25 +275,17 @@ void ext4_evict_inode(struct inode *inode) > } > } > > - /* > - * ext4_ext_truncate() doesn't reserve any slop when it > - * restarts journal transactions; therefore there may not be > - * enough credits left in the handle to remove the inode from > - * the orphan list and set the dtime field. > - */ > - if (!ext4_handle_has_enough_credits(handle, extra_credits)) { > - err = ext4_journal_extend(handle, extra_credits); > - if (err > 0) > - err = ext4_journal_restart(handle, extra_credits); > - if (err != 0) { > - ext4_warning(inode->i_sb, > - "couldn't extend journal (err %d)", err); > - stop_handle: > - ext4_journal_stop(handle); > - ext4_orphan_del(NULL, inode); > - sb_end_intwrite(inode->i_sb); > - goto no_delete; > - } > + /* Remove xattr references. */ > + err = ext4_xattr_delete_inode(handle, inode, &ea_inode_array, > + extra_credits); > + if (err) { > + ext4_warning(inode->i_sb, "xattr delete (err %d)", err); > + stop_handle: > + ext4_journal_stop(handle); > + ext4_orphan_del(NULL, inode); > + sb_end_intwrite(inode->i_sb); > + ext4_xattr_inode_array_free(ea_inode_array); > + goto no_delete; > } > > /* > @@ -342,7 +311,6 @@ void ext4_evict_inode(struct inode *inode) > ext4_clear_inode(inode); > else > ext4_free_inode(handle, inode); > - > ext4_journal_stop(handle); > sb_end_intwrite(inode->i_sb); > ext4_xattr_inode_array_free(ea_inode_array); > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c > index 3ee7e2f68476..abc7d5f84e5f 100644 > --- a/fs/ext4/xattr.c > +++ b/fs/ext4/xattr.c > @@ -108,6 +108,10 @@ const struct xattr_handler *ext4_xattr_handlers[] = { > #define EXT4_GET_MB_CACHE(inode) (((struct ext4_sb_info *) \ > inode->i_sb->s_fs_info)->s_mb_cache) > > +static int > +ext4_expand_inode_array(struct ext4_xattr_inode_array **ea_inode_array, > + struct inode *inode); > + > #ifdef CONFIG_LOCKDEP > void ext4_xattr_inode_set_class(struct inode *ea_inode) > { > @@ -653,6 +657,127 @@ static void ext4_xattr_update_super_block(handle_t *handle, > } > } > > +static int ext4_xattr_ensure_credits(handle_t *handle, struct inode *inode, > + int credits, struct buffer_head *bh, > + bool dirty, bool block_csum) > +{ > + int error; > + > + if (!ext4_handle_valid(handle)) > + return 0; > + > + if (handle->h_buffer_credits >= credits) > + return 0; > + > + error = ext4_journal_extend(handle, credits - handle->h_buffer_credits); > + if (!error) > + return 0; > + if (error < 0) { > + ext4_warning(inode->i_sb, "Extend journal (error %d)", error); > + return error; > + } > + > + if (bh && dirty) { > + if (block_csum) > + ext4_xattr_block_csum_set(inode, bh); > + error = ext4_handle_dirty_metadata(handle, NULL, bh); > + if (error) { > + ext4_warning(inode->i_sb, "Handle metadata (error %d)", > + error); > + return error; > + } > + } > + > + error = ext4_journal_restart(handle, credits); > + if (error) { > + ext4_warning(inode->i_sb, "Restart journal (error %d)", error); > + return error; > + } > + > + if (bh) { > + error = ext4_journal_get_write_access(handle, bh); > + if (error) { > + ext4_warning(inode->i_sb, > + "Get write access failed (error %d)", > + error); > + return error; > + } > + } > + return 0; > +} > + > +static void > +ext4_xattr_inode_remove_all(handle_t *handle, struct inode *parent, > + struct buffer_head *bh, > + struct ext4_xattr_entry *first, bool block_csum, > + struct ext4_xattr_inode_array **ea_inode_array, > + int extra_credits) > +{ > + struct inode *ea_inode; > + struct ext4_xattr_entry *entry; > + bool dirty = false; > + unsigned int ea_ino; > + int err; > + int credits; > + > + /* One credit for dec ref on ea_inode, one for orphan list addition, */ > + credits = 2 + extra_credits; > + > + for (entry = first; !IS_LAST_ENTRY(entry); > + entry = EXT4_XATTR_NEXT(entry)) { > + if (!entry->e_value_inum) > + continue; > + ea_ino = le32_to_cpu(entry->e_value_inum); > + err = ext4_xattr_inode_iget(parent, ea_ino, &ea_inode); > + if (err) > + continue; > + > + err = ext4_expand_inode_array(ea_inode_array, ea_inode); > + if (err) { > + ext4_warning_inode(ea_inode, > + "Expand inode array err=%d", err); > + iput(ea_inode); > + continue; > + } > + > + err = ext4_xattr_ensure_credits(handle, parent, credits, bh, > + dirty, block_csum); > + if (err) { > + ext4_warning_inode(ea_inode, "Ensure credits err=%d", > + err); > + continue; > + } > + > + inode_lock(ea_inode); > + clear_nlink(ea_inode); > + ext4_orphan_add(handle, ea_inode); > + inode_unlock(ea_inode); > + > + /* > + * Forget about ea_inode within the same transaction that decrements the ref > + * count. This avoids duplicate decrements in case the rest of the work > + * spills over to subsequent transactions. > + */ > + entry->e_value_inum = 0; > + entry->e_value_size = 0; > + > + dirty = true; > + } > + > + if (dirty) { > + /* > + * Note that we are deliberately skipping csum calculation for > + * the final update because we do not expect any journal > + * restarts until xattr block is freed. > + */ > + > + err = ext4_handle_dirty_metadata(handle, NULL, bh); > + if (err) > + ext4_warning_inode(parent, > + "handle dirty metadata err=%d", err); > + } > +} > + > /* > * Release the xattr block BH: If the reference count is > 1, decrement it; > * otherwise free the block. > @@ -1985,42 +2110,6 @@ ext4_expand_inode_array(struct ext4_xattr_inode_array **ea_inode_array, > return 0; > } > > -/** > - * Add xattr inode to orphan list > - */ > -static int > -ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode, int credits, > - struct ext4_xattr_inode_array *ea_inode_array) > -{ > - int idx = 0, error = 0; > - struct inode *ea_inode; > - > - if (ea_inode_array == NULL) > - return 0; > - > - for (; idx < ea_inode_array->count; ++idx) { > - if (!ext4_handle_has_enough_credits(handle, credits)) { > - error = ext4_journal_extend(handle, credits); > - if (error > 0) > - error = ext4_journal_restart(handle, credits); > - > - if (error != 0) { > - ext4_warning(inode->i_sb, > - "couldn't extend journal " > - "(err %d)", error); > - return error; > - } > - } > - ea_inode = ea_inode_array->inodes[idx]; > - inode_lock(ea_inode); > - ext4_orphan_add(handle, ea_inode); > - inode_unlock(ea_inode); > - /* the inode's i_count will be released by caller */ > - } > - > - return 0; > -} > - > /* > * ext4_xattr_delete_inode() > * > @@ -2033,16 +2122,23 @@ ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode, int credits, > */ > int > ext4_xattr_delete_inode(handle_t *handle, struct inode *inode, > - struct ext4_xattr_inode_array **ea_inode_array) > + struct ext4_xattr_inode_array **ea_inode_array, > + int extra_credits) > { > struct buffer_head *bh = NULL; > struct ext4_xattr_ibody_header *header; > struct ext4_inode *raw_inode; > - struct ext4_iloc iloc; > - struct ext4_xattr_entry *entry; > - struct inode *ea_inode; > - unsigned int ea_ino; > - int credits = 3, error = 0; > + struct ext4_iloc iloc = { .bh = NULL }; > + int error; > + > + error = ext4_xattr_ensure_credits(handle, inode, extra_credits, > + NULL /* bh */, > + false /* dirty */, > + false /* block_csum */); > + if (error) { > + EXT4_ERROR_INODE(inode, "ensure credits (error %d)", error); > + goto cleanup; > + } > > if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR)) > goto delete_external_ea; > @@ -2050,31 +2146,20 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode, > error = ext4_get_inode_loc(inode, &iloc); > if (error) > goto cleanup; > + > + error = ext4_journal_get_write_access(handle, iloc.bh); > + if (error) > + goto cleanup; > + > raw_inode = ext4_raw_inode(&iloc); > header = IHDR(inode, raw_inode); > - for (entry = IFIRST(header); !IS_LAST_ENTRY(entry); > - entry = EXT4_XATTR_NEXT(entry)) { > - if (!entry->e_value_inum) > - continue; > - ea_ino = le32_to_cpu(entry->e_value_inum); > - error = ext4_xattr_inode_iget(inode, ea_ino, &ea_inode); > - if (error) > - continue; > - error = ext4_expand_inode_array(ea_inode_array, ea_inode); > - if (error) { > - iput(ea_inode); > - brelse(iloc.bh); > - goto cleanup; > - } > - entry->e_value_inum = 0; > - } > - brelse(iloc.bh); > + ext4_xattr_inode_remove_all(handle, inode, iloc.bh, IFIRST(header), > + false /* block_csum */, ea_inode_array, > + extra_credits); > > delete_external_ea: > if (!EXT4_I(inode)->i_file_acl) { > - /* add xattr inode to orphan list */ > - error = ext4_xattr_inode_orphan_add(handle, inode, credits, > - *ea_inode_array); > + error = 0; > goto cleanup; > } > bh = sb_bread(inode->i_sb, EXT4_I(inode)->i_file_acl); > @@ -2092,46 +2177,32 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode, > goto cleanup; > } > > - for (entry = BFIRST(bh); !IS_LAST_ENTRY(entry); > - entry = EXT4_XATTR_NEXT(entry)) { > - if (!entry->e_value_inum) > - continue; > - ea_ino = le32_to_cpu(entry->e_value_inum); > - error = ext4_xattr_inode_iget(inode, ea_ino, &ea_inode); > - if (error) > - continue; > - error = ext4_expand_inode_array(ea_inode_array, ea_inode); > - if (error) > - goto cleanup; > - entry->e_value_inum = 0; > - } > - > - /* add xattr inode to orphan list */ > - error = ext4_xattr_inode_orphan_add(handle, inode, credits, > - *ea_inode_array); > - if (error) > - goto cleanup; > - > - if (!IS_NOQUOTA(inode)) > - credits += 2 * EXT4_QUOTA_DEL_BLOCKS(inode->i_sb); > - > - if (!ext4_handle_has_enough_credits(handle, credits)) { > - error = ext4_journal_extend(handle, credits); > - if (error > 0) > - error = ext4_journal_restart(handle, credits); > + if (ext4_has_feature_ea_inode(inode->i_sb)) { > + error = ext4_journal_get_write_access(handle, bh); > if (error) { > - ext4_warning(inode->i_sb, > - "couldn't extend journal (err %d)", error); > + EXT4_ERROR_INODE(inode, "write access %llu", > + EXT4_I(inode)->i_file_acl); > goto cleanup; > } > + ext4_xattr_inode_remove_all(handle, inode, bh, > + BFIRST(bh), > + true /* block_csum */, > + ea_inode_array, > + extra_credits); > } > > ext4_xattr_release_block(handle, inode, bh); > + /* Update i_file_acl within the same transaction that releases block. */ > EXT4_I(inode)->i_file_acl = 0; > - > + error = ext4_mark_inode_dirty(handle, inode); > + if (error) { > + EXT4_ERROR_INODE(inode, "mark inode dirty (error %d)", > + error); > + goto cleanup; > + } > cleanup: > + brelse(iloc.bh); > brelse(bh); > - > return error; > } > > diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h > index adf761518a73..b2005a2716d9 100644 > --- a/fs/ext4/xattr.h > +++ b/fs/ext4/xattr.h > @@ -169,7 +169,8 @@ 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, > - struct ext4_xattr_inode_array **array); > + struct ext4_xattr_inode_array **array, > + int extra_credits); > extern void ext4_xattr_inode_array_free(struct ext4_xattr_inode_array *array); > > extern int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize, > -- > 2.13.1.508.gb3defc5cc-goog > Cheers, Andreas -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 195 bytes Desc: Message signed with OpenPGP Url : http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20170614/9174eb88/attachment.bin
next prev parent reply other threads:[~2017-06-15 0:11 UTC|newest] Thread overview: 100+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-05-31 8:14 [PATCH 01/28] ext4: xattr-in-inode support Tahsin Erdogan 2017-05-31 8:14 ` [PATCH 02/28] ext4: fix lockdep warning about recursive inode locking Tahsin Erdogan 2017-05-31 8:14 ` [PATCH 03/28] ext4: lock inode before calling ext4_orphan_add() Tahsin Erdogan 2017-05-31 8:14 ` [PATCH 04/28] ext4: do not set posix acls on xattr inodes Tahsin Erdogan 2017-05-31 8:14 ` [PATCH 05/28] ext4: attach jinode after creation of xattr inode Tahsin Erdogan 2017-05-31 8:14 ` [PATCH 06/28] ext4: ea_inode owner should be the same as the inode owner Tahsin Erdogan 2017-05-31 8:14 ` [PATCH 07/28] ext4: call journal revoke when freeing ea_inode blocks Tahsin Erdogan 2017-05-31 16:12 ` Darrick J. Wong 2017-05-31 16:12 ` [Ocfs2-devel] " Darrick J. Wong 2017-05-31 16:12 ` Darrick J. Wong 2017-05-31 21:01 ` Tahsin Erdogan 2017-06-05 22:08 ` Andreas Dilger 2017-05-31 8:14 ` [PATCH 08/28] ext4: fix ref counting for ea_inode Tahsin Erdogan 2017-05-31 8:14 ` [PATCH 09/28] ext4: extended attribute value size limit is enforced by vfs Tahsin Erdogan 2017-05-31 16:03 ` Darrick J. Wong 2017-05-31 16:03 ` [Ocfs2-devel] " Darrick J. Wong 2017-05-31 16:03 ` Darrick J. Wong 2017-05-31 16:13 ` Tahsin Erdogan 2017-05-31 8:14 ` [PATCH 10/28] ext4: change ext4_xattr_inode_iget() signature Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 11/28] ext4: clean up ext4_xattr_inode_get() Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 12/28] ext4: add missing le32_to_cpu(e_value_inum) conversions Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 13/28] ext4: ext4_xattr_value_same() should return false for external data Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 14/28] ext4: fix ext4_xattr_make_inode_space() value size calculation Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 15/28] ext4: fix ext4_xattr_move_to_block() Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 16/28] ext4: fix ext4_xattr_cmp() Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 17/28] ext4: fix credits calculation for xattr inode Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 18/28] ext4: retry storing value in external inode with xattr block too Tahsin Erdogan 2017-06-20 8:56 ` [PATCH v2 18/31] " Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 19/28] ext4: ext4_xattr_delete_inode() should return accurate errors Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 20/28] ext4: improve journal credit handling in set xattr paths Tahsin Erdogan 2017-06-20 8:59 ` [PATCH v2 20/31] " Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 21/28] ext4: modify ext4_xattr_ino_array to hold struct inode * Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 22/28] ext4: move struct ext4_xattr_inode_array to xattr.h Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 23/28] mbcache: make mbcache more generic Tahsin Erdogan 2017-06-15 7:41 ` Jan Kara 2017-06-15 7:41 ` [Ocfs2-devel] " Jan Kara 2017-06-15 18:25 ` Tahsin Erdogan 2017-06-19 8:50 ` Jan Kara 2017-06-19 8:50 ` [Ocfs2-devel] " Jan Kara 2017-06-20 9:01 ` [PATCH v2 23/31] mbcache: make mbcache naming " Tahsin Erdogan 2017-06-21 17:43 ` Andreas Dilger 2017-06-21 18:33 ` Andreas Dilger 2017-06-21 21:39 ` Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 24/28] ext4: rename mb block cache functions Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 25/28] ext4: add ext4_is_quota_file() Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 26/28] ext4: cleanup transaction restarts during inode deletion Tahsin Erdogan 2017-06-14 14:17 ` [PATCH v2 " Tahsin Erdogan 2017-06-15 0:11 ` Andreas Dilger [this message] 2017-06-15 0:11 ` [Ocfs2-devel] " Andreas Dilger 2017-06-20 9:04 ` [PATCH v3 " Tahsin Erdogan 2017-06-20 9:29 ` Tahsin Erdogan 2017-05-31 8:15 ` [PATCH 27/28] ext4: xattr inode deduplication Tahsin Erdogan 2017-05-31 15:40 ` kbuild test robot 2017-05-31 15:40 ` [Ocfs2-devel] " kbuild test robot 2017-05-31 15:50 ` kbuild test robot 2017-05-31 15:50 ` [Ocfs2-devel] " kbuild test robot 2017-05-31 16:00 ` Darrick J. Wong 2017-05-31 16:00 ` [Ocfs2-devel] " Darrick J. Wong 2017-05-31 16:00 ` Darrick J. Wong 2017-05-31 22:33 ` [PATCH v2 " Tahsin Erdogan 2017-06-02 5:41 ` Darrick J. Wong 2017-06-02 5:41 ` [Ocfs2-devel] " Darrick J. Wong 2017-06-02 5:41 ` Darrick J. Wong 2017-06-02 12:46 ` Tahsin Erdogan 2017-06-02 17:59 ` Darrick J. Wong 2017-06-02 17:59 ` [Ocfs2-devel] " Darrick J. Wong 2017-06-02 17:59 ` Darrick J. Wong 2017-06-02 23:35 ` [PATCH v3 " Tahsin Erdogan 2017-06-14 14:34 ` [PATCH v4 " Tahsin Erdogan 2017-06-14 23:26 ` Andreas Dilger 2017-06-20 9:07 ` [PATCH v5 " Tahsin Erdogan 2017-06-20 9:49 ` Tahsin Erdogan 2017-06-21 17:42 ` Andreas Dilger 2017-06-21 21:14 ` Andreas Dilger 2017-06-21 21:34 ` Tahsin Erdogan 2017-06-21 21:42 ` Andreas Dilger 2017-07-04 18:39 ` Theodore Ts'o 2017-07-05 17:30 ` Tahsin Erdogan 2017-07-06 4:19 ` Theodore Ts'o 2017-05-31 8:15 ` [PATCH 28/28] quota: add extra inode count to dquot transfer functions Tahsin Erdogan 2017-06-15 7:57 ` Jan Kara 2017-06-15 7:57 ` [Ocfs2-devel] " Jan Kara 2017-06-17 1:50 ` Tahsin Erdogan 2017-06-19 9:03 ` Jan Kara 2017-06-19 9:03 ` [Ocfs2-devel] " Jan Kara 2017-06-19 11:46 ` Tahsin Erdogan 2017-06-19 12:36 ` Jan Kara 2017-06-19 12:36 ` [Ocfs2-devel] " Jan Kara 2017-06-20 9:12 ` [PATCH v2 28/31] quota: add get_inode_usage callback to transfer multi-inode charges Tahsin Erdogan 2017-06-20 12:01 ` Tahsin Erdogan 2017-06-20 15:28 ` Jan Kara 2017-06-20 18:08 ` [PATCH v3 " Tahsin Erdogan 2017-06-21 4:48 ` Theodore Ts'o 2017-06-21 11:22 ` Tahsin Erdogan 2017-06-20 9:53 ` [PATCH 28/28] quota: add extra inode count to dquot transfer functions Tahsin Erdogan 2017-05-31 16:42 ` [PATCH 01/28] ext4: xattr-in-inode support Darrick J. Wong 2017-05-31 16:42 ` [Ocfs2-devel] " Darrick J. Wong 2017-05-31 16:42 ` Darrick J. Wong 2017-05-31 19:59 ` Tahsin Erdogan 2017-06-01 15:50 ` [PATCH v2 " Tahsin Erdogan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=A15C59A1-EC1C-4A58-927C-C086CD90A524@dilger.ca \ --to=adilger@dilger.ca \ --cc=axboe@fb.com \ --cc=deepa.kernel@gmail.com \ --cc=fabf@skynet.be \ --cc=jack@suse.com \ --cc=jfs-discussion@lists.sourceforge.net \ --cc=jlbec@evilplan.org \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mchristi@redhat.com \ --cc=mfasheh@versity.com \ --cc=ocfs2-devel@oss.oracle.com \ --cc=reiserfs-devel@vger.kernel.org \ --cc=shaggy@kernel.org \ --cc=tahsin@google.com \ --cc=tytso@mit.edu \ --cc=viro@zeniv.linux.org.uk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.