From patchwork Wed Jun 21 21:21:36 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tahsin Erdogan X-Patchwork-Id: 779145 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3wtHmX0btZz9s2s for ; Thu, 22 Jun 2017 07:25:00 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="mA+nW9RV"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752332AbdFUVYI (ORCPT ); Wed, 21 Jun 2017 17:24:08 -0400 Received: from mail-pf0-f181.google.com ([209.85.192.181]:36685 "EHLO mail-pf0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752253AbdFUVWh (ORCPT ); Wed, 21 Jun 2017 17:22:37 -0400 Received: by mail-pf0-f181.google.com with SMTP id q86so6621973pfl.3 for ; Wed, 21 Jun 2017 14:22:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=ueUaRYv4A4ob6YDIYVbjr4gF5mabewUly+QJZ1r5kvo=; b=mA+nW9RVCJMNe84+pw3ZtkJWviWB8/v97gnEQfWKPOqb+4iESfRjgOPwxlCg/UKSud 7vxx89QSKidW8k5jtARaka1ne7BUgOHF4KPOQLftS3oJlRoums0aGDhPFj7b+ODAz4QU fnsUm5FW6UfhA1fGnC/cAc32SxZEh9AUoVKN8p7h/fBDzsAiMkMekxcFvYFDnJ0ARIkY w4e/Vp233rRJZg7BSvsYdfFEfKHs7azoxs9+cNjo8WlXqYjpb69gEfufu9EBLtdOxKzF JDYPKioUNIm/eHlYng88FyClfJdkLio0wMHqARVPuetJRX3PvS/mTOae/TW8zsCUcmM1 kq3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=ueUaRYv4A4ob6YDIYVbjr4gF5mabewUly+QJZ1r5kvo=; b=tisk/ISkeW+AxDIuFm05ebE4enAMyclf4to95MMZt1j9vbS3P0U2QgST501ZmJ3xD1 NrZ+VNcPpq/TwQuY8PBx4alLsHiwzzHheuPHLf5ekswXUyE/iNhZESxpd5TRN1YHar7k HCMYteJ2SY/gYSJI2bPYvU7FuU0kgGEYXMxk8feKPqy+tO72pkRmZTTkhESW4CXLYWIn 7/IMLM8y+qzgSu5iyK2GBY4crJCUKNliIv0XGSebLlNvtftIuPOL+QRQmX/CiU4aOgu7 mGsLYZIKpTgdkddhdHSgX40QlShFia2SmBRfDTCOMZOOygGe9WvdNBf9bSlCFYCOUGYx Nz8A== X-Gm-Message-State: AKS2vOwpXhfpfz4vae2kSPI0KfngKl7Un7SDKplvU/b0XMlO6zZ1jb/r msn39TBj9ZXKF0ZC X-Received: by 10.84.173.67 with SMTP id o61mr35174788plb.236.1498080146260; Wed, 21 Jun 2017 14:22:26 -0700 (PDT) Received: from tahsin1.svl.corp.google.com ([100.123.230.167]) by smtp.gmail.com with ESMTPSA id s64sm1829980pfd.77.2017.06.21.14.22.25 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 21 Jun 2017 14:22:25 -0700 (PDT) From: Tahsin Erdogan To: Andreas Dilger , "Darrick J . Wong" , Jan Kara , Theodore Ts'o , linux-ext4@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Tahsin Erdogan Subject: [PATCH 26/32] ext4: cleanup transaction restarts during inode deletion Date: Wed, 21 Jun 2017 14:21:36 -0700 Message-Id: <20170621212142.16581-26-tahsin@google.com> X-Mailer: git-send-email 2.13.1.611.g7e3b11ae1-goog In-Reply-To: <20170621212142.16581-1-tahsin@google.com> References: <20170621212142.16581-1-tahsin@google.com> Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org 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. Signed-off-by: Tahsin Erdogan --- v3: fixed checkpatch.pl warnings about long lines and indented label v2: made ext4_xattr_ensure_credits() static fs/ext4/inode.c | 66 ++++----------- fs/ext4/xattr.c | 258 ++++++++++++++++++++++++++++++++++++-------------------- fs/ext4/xattr.h | 3 +- 3 files changed, 184 insertions(+), 143 deletions(-) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index cf91532765a4..cd007f9757d1 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 5f36121c5a00..353a323f30b1 100644 --- a/fs/ext4/xattr.c +++ b/fs/ext4/xattr.c @@ -108,6 +108,10 @@ const struct xattr_handler *ext4_xattr_handlers[] = { #define MB_BLOCK_CACHE(inode) (((struct ext4_sb_info *) \ inode->i_sb->s_fs_info)->s_mb_block_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) { @@ -652,6 +656,128 @@ 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. @@ -1984,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() * @@ -2032,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; @@ -2049,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); @@ -2091,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,