From patchwork Fri Apr 17 15:08:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 11495377 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 134811392 for ; Fri, 17 Apr 2020 15:09:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EFF532223D for ; Fri, 17 Apr 2020 15:09:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Oi3ZV8Ri" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728193AbgDQPJI (ORCPT ); Fri, 17 Apr 2020 11:09:08 -0400 Received: from us-smtp-delivery-1.mimecast.com ([205.139.110.120]:54979 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728858AbgDQPJG (ORCPT ); Fri, 17 Apr 2020 11:09:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587136144; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5wdHauiDbb2jHLjxtcDVlN4iLnrrzaREoLO8WLv72kc=; b=Oi3ZV8RirMpZeBbl/04uTj0Y2NPgtGV4kyfVeSad3ic272p6fdwIlWpSTWAVuVJpxoEfC5 8yRAHu/xSvyqjyMjiEhYK1V5ifkj2LHW0PW8lV4zWKhVyvq8GAjp5ybXnsj1J3aRpZ8/Ma 7W/DrBi4kV93gbp8nW/PpzEmbVrolzk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-296-OM_gzf75NWqSJEDzBUpq-g-1; Fri, 17 Apr 2020 11:09:01 -0400 X-MC-Unique: OM_gzf75NWqSJEDzBUpq-g-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8F65C8017FE for ; Fri, 17 Apr 2020 15:09:00 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id 499A360BE0 for ; Fri, 17 Apr 2020 15:09:00 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH 01/12] xfs: refactor failed buffer resubmission into xfsaild Date: Fri, 17 Apr 2020 11:08:48 -0400 Message-Id: <20200417150859.14734-2-bfoster@redhat.com> In-Reply-To: <20200417150859.14734-1-bfoster@redhat.com> References: <20200417150859.14734-1-bfoster@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org Flush locked log items whose underlying buffers fail metadata writeback are tagged with a special flag to indicate that the flush lock is already held. This is currently implemented in the type specific ->iop_push() callback, but the processing required for such items is not type specific because we're only doing basic state management on the underlying buffer. Factor the failed log item handling out of the inode and dquot ->iop_push() callbacks and open code the buffer resubmit helper into a single helper called from xfsaild_push_item(). This provides a generic mechanism for handling failed metadata buffer writeback with a bit less code. Signed-off-by: Brian Foster Reviewed-by: Allison Collins --- fs/xfs/xfs_buf_item.c | 39 --------------------------------------- fs/xfs/xfs_buf_item.h | 2 -- fs/xfs/xfs_dquot_item.c | 15 --------------- fs/xfs/xfs_inode_item.c | 15 --------------- fs/xfs/xfs_trans_ail.c | 41 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 41 insertions(+), 71 deletions(-) diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c index 1545657c3ca0..8796adde2d12 100644 --- a/fs/xfs/xfs_buf_item.c +++ b/fs/xfs/xfs_buf_item.c @@ -1248,42 +1248,3 @@ xfs_buf_iodone( xfs_trans_ail_delete(ailp, lip, SHUTDOWN_CORRUPT_INCORE); xfs_buf_item_free(BUF_ITEM(lip)); } - -/* - * Requeue a failed buffer for writeback. - * - * We clear the log item failed state here as well, but we have to be careful - * about reference counts because the only active reference counts on the buffer - * may be the failed log items. Hence if we clear the log item failed state - * before queuing the buffer for IO we can release all active references to - * the buffer and free it, leading to use after free problems in - * xfs_buf_delwri_queue. It makes no difference to the buffer or log items which - * order we process them in - the buffer is locked, and we own the buffer list - * so nothing on them is going to change while we are performing this action. - * - * Hence we can safely queue the buffer for IO before we clear the failed log - * item state, therefore always having an active reference to the buffer and - * avoiding the transient zero-reference state that leads to use-after-free. - * - * Return true if the buffer was added to the buffer list, false if it was - * already on the buffer list. - */ -bool -xfs_buf_resubmit_failed_buffers( - struct xfs_buf *bp, - struct list_head *buffer_list) -{ - struct xfs_log_item *lip; - bool ret; - - ret = xfs_buf_delwri_queue(bp, buffer_list); - - /* - * XFS_LI_FAILED set/clear is protected by ail_lock, caller of this - * function already have it acquired - */ - list_for_each_entry(lip, &bp->b_li_list, li_bio_list) - xfs_clear_li_failed(lip); - - return ret; -} diff --git a/fs/xfs/xfs_buf_item.h b/fs/xfs/xfs_buf_item.h index 30114b510332..c9c57e2da932 100644 --- a/fs/xfs/xfs_buf_item.h +++ b/fs/xfs/xfs_buf_item.h @@ -59,8 +59,6 @@ void xfs_buf_attach_iodone(struct xfs_buf *, struct xfs_log_item *); void xfs_buf_iodone_callbacks(struct xfs_buf *); void xfs_buf_iodone(struct xfs_buf *, struct xfs_log_item *); -bool xfs_buf_resubmit_failed_buffers(struct xfs_buf *, - struct list_head *); bool xfs_buf_log_check_iovec(struct xfs_log_iovec *iovec); extern kmem_zone_t *xfs_buf_item_zone; diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c index baad1748d0d1..5a7808299a32 100644 --- a/fs/xfs/xfs_dquot_item.c +++ b/fs/xfs/xfs_dquot_item.c @@ -145,21 +145,6 @@ xfs_qm_dquot_logitem_push( if (atomic_read(&dqp->q_pincount) > 0) return XFS_ITEM_PINNED; - /* - * The buffer containing this item failed to be written back - * previously. Resubmit the buffer for IO - */ - if (test_bit(XFS_LI_FAILED, &lip->li_flags)) { - if (!xfs_buf_trylock(bp)) - return XFS_ITEM_LOCKED; - - if (!xfs_buf_resubmit_failed_buffers(bp, buffer_list)) - rval = XFS_ITEM_FLUSHING; - - xfs_buf_unlock(bp); - return rval; - } - if (!xfs_dqlock_nowait(dqp)) return XFS_ITEM_LOCKED; diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c index f779cca2346f..1d4d256a2e96 100644 --- a/fs/xfs/xfs_inode_item.c +++ b/fs/xfs/xfs_inode_item.c @@ -497,21 +497,6 @@ xfs_inode_item_push( if (xfs_ipincount(ip) > 0) return XFS_ITEM_PINNED; - /* - * The buffer containing this item failed to be written back - * previously. Resubmit the buffer for IO. - */ - if (test_bit(XFS_LI_FAILED, &lip->li_flags)) { - if (!xfs_buf_trylock(bp)) - return XFS_ITEM_LOCKED; - - if (!xfs_buf_resubmit_failed_buffers(bp, buffer_list)) - rval = XFS_ITEM_FLUSHING; - - xfs_buf_unlock(bp); - return rval; - } - if (!xfs_ilock_nowait(ip, XFS_ILOCK_SHARED)) return XFS_ITEM_LOCKED; diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c index 564253550b75..0c709651a2c6 100644 --- a/fs/xfs/xfs_trans_ail.c +++ b/fs/xfs/xfs_trans_ail.c @@ -345,6 +345,45 @@ xfs_ail_delete( xfs_trans_ail_cursor_clear(ailp, lip); } +/* + * Requeue a failed buffer for writeback. + * + * We clear the log item failed state here as well, but we have to be careful + * about reference counts because the only active reference counts on the buffer + * may be the failed log items. Hence if we clear the log item failed state + * before queuing the buffer for IO we can release all active references to + * the buffer and free it, leading to use after free problems in + * xfs_buf_delwri_queue. It makes no difference to the buffer or log items which + * order we process them in - the buffer is locked, and we own the buffer list + * so nothing on them is going to change while we are performing this action. + * + * Hence we can safely queue the buffer for IO before we clear the failed log + * item state, therefore always having an active reference to the buffer and + * avoiding the transient zero-reference state that leads to use-after-free. + */ +static inline int +xfsaild_push_failed( + struct xfs_log_item *lip, + struct list_head *buffer_list) +{ + struct xfs_buf *bp = lip->li_buf; + + if (!xfs_buf_trylock(bp)) + return XFS_ITEM_LOCKED; + + if (!xfs_buf_delwri_queue(bp, buffer_list)) { + xfs_buf_unlock(bp); + return XFS_ITEM_FLUSHING; + } + + /* protected by ail_lock */ + list_for_each_entry(lip, &bp->b_li_list, li_bio_list) + xfs_clear_li_failed(lip); + + xfs_buf_unlock(bp); + return XFS_ITEM_SUCCESS; +} + static inline uint xfsaild_push_item( struct xfs_ail *ailp, @@ -365,6 +404,8 @@ xfsaild_push_item( */ if (!lip->li_ops->iop_push) return XFS_ITEM_PINNED; + if (test_bit(XFS_LI_FAILED, &lip->li_flags)) + return xfsaild_push_failed(lip, &ailp->ail_buf_list); return lip->li_ops->iop_push(lip, &ailp->ail_buf_list); } From patchwork Fri Apr 17 15:08:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 11495369 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6C428174A for ; Fri, 17 Apr 2020 15:09:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 544D22223C for ; Fri, 17 Apr 2020 15:09:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="UmLo+r4Z" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728303AbgDQPJG (ORCPT ); Fri, 17 Apr 2020 11:09:06 -0400 Received: from us-smtp-2.mimecast.com ([205.139.110.61]:43900 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728852AbgDQPJF (ORCPT ); Fri, 17 Apr 2020 11:09:05 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587136144; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ekRja/0uHuj4J/sQPDKwWUNvXZRNoP5ryd+VAnwLquc=; b=UmLo+r4Zz7GOq3YWNxPpRIASrsdEXQxZwzYJMolG9/Ekxfu+Oz+OA2X6vbfPo57Qmr5XfP Ie5xSLspvO/8ST2mA7AykFuWDJdsO9zPqLl7kYuvm5IBO5ZOD1pkstPkOpdyMEunDEGHPQ CP2cBpg/VYk7PFhmXeFtBNmgdi9bDiM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-306-GydnFOsFOAaOM8sapZ0ovw-1; Fri, 17 Apr 2020 11:09:01 -0400 X-MC-Unique: GydnFOsFOAaOM8sapZ0ovw-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 0919E18C43C3 for ; Fri, 17 Apr 2020 15:09:01 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id B558D60BE0 for ; Fri, 17 Apr 2020 15:09:00 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH 02/12] xfs: factor out buffer I/O failure simulation code Date: Fri, 17 Apr 2020 11:08:49 -0400 Message-Id: <20200417150859.14734-3-bfoster@redhat.com> In-Reply-To: <20200417150859.14734-1-bfoster@redhat.com> References: <20200417150859.14734-1-bfoster@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org We use the same buffer I/O failure simulation code in a few different places. It's not much code, but it's not necessarily self-explanatory. Factor it into a helper and document it in one place. Signed-off-by: Brian Foster Reviewed-by: Allison Collins --- fs/xfs/xfs_buf.c | 23 +++++++++++++++++++---- fs/xfs/xfs_buf.h | 1 + fs/xfs/xfs_buf_item.c | 22 +++------------------- fs/xfs/xfs_inode.c | 7 +------ 4 files changed, 24 insertions(+), 29 deletions(-) diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index 9ec3eaf1c618..93942d8e35dd 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -1248,6 +1248,24 @@ xfs_buf_ioerror_alert( -bp->b_error); } +/* + * To simulate an I/O failure, the buffer must be locked and held with at least + * three references. The LRU reference is dropped by the stale call. The buf + * item reference is dropped via ioend processing. The third reference is owned + * by the caller and is dropped on I/O completion if the buffer is XBF_ASYNC. + */ +void +xfs_buf_iofail( + struct xfs_buf *bp, + int flags) +{ + bp->b_flags |= flags; + bp->b_flags &= ~XBF_DONE; + xfs_buf_stale(bp); + xfs_buf_ioerror(bp, -EIO); + xfs_buf_ioend(bp); +} + int xfs_bwrite( struct xfs_buf *bp) @@ -1480,10 +1498,7 @@ __xfs_buf_submit( /* on shutdown we stale and complete the buffer immediately */ if (XFS_FORCED_SHUTDOWN(bp->b_mount)) { - xfs_buf_ioerror(bp, -EIO); - bp->b_flags &= ~XBF_DONE; - xfs_buf_stale(bp); - xfs_buf_ioend(bp); + xfs_buf_iofail(bp, 0); return -EIO; } diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h index 9a04c53c2488..a6bce4702b2e 100644 --- a/fs/xfs/xfs_buf.h +++ b/fs/xfs/xfs_buf.h @@ -263,6 +263,7 @@ extern void __xfs_buf_ioerror(struct xfs_buf *bp, int error, xfs_failaddr_t failaddr); #define xfs_buf_ioerror(bp, err) __xfs_buf_ioerror((bp), (err), __this_address) extern void xfs_buf_ioerror_alert(struct xfs_buf *bp, xfs_failaddr_t fa); +void xfs_buf_iofail(struct xfs_buf *, int); extern int __xfs_buf_submit(struct xfs_buf *bp, bool); static inline int xfs_buf_submit(struct xfs_buf *bp) diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c index 8796adde2d12..72d37a4609d8 100644 --- a/fs/xfs/xfs_buf_item.c +++ b/fs/xfs/xfs_buf_item.c @@ -471,28 +471,12 @@ xfs_buf_item_unpin( xfs_buf_relse(bp); } else if (freed && remove) { /* - * There are currently two references to the buffer - the active - * LRU reference and the buf log item. What we are about to do - * here - simulate a failed IO completion - requires 3 - * references. - * - * The LRU reference is removed by the xfs_buf_stale() call. The - * buf item reference is removed by the xfs_buf_iodone() - * callback that is run by xfs_buf_do_callbacks() during ioend - * processing (via the bp->b_iodone callback), and then finally - * the ioend processing will drop the IO reference if the buffer - * is marked XBF_ASYNC. - * - * Hence we need to take an additional reference here so that IO - * completion processing doesn't free the buffer prematurely. + * The buffer must be locked and held by the caller to simulate + * an async I/O failure. */ xfs_buf_lock(bp); xfs_buf_hold(bp); - bp->b_flags |= XBF_ASYNC; - xfs_buf_ioerror(bp, -EIO); - bp->b_flags &= ~XBF_DONE; - xfs_buf_stale(bp); - xfs_buf_ioend(bp); + xfs_buf_iofail(bp, XBF_ASYNC); } } diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index d1772786af29..b539ee221ce5 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -3629,12 +3629,7 @@ xfs_iflush_cluster( * xfs_buf_submit(). */ ASSERT(bp->b_iodone); - bp->b_flags |= XBF_ASYNC; - bp->b_flags &= ~XBF_DONE; - xfs_buf_stale(bp); - xfs_buf_ioerror(bp, -EIO); - xfs_buf_ioend(bp); - + xfs_buf_iofail(bp, XBF_ASYNC); xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE); /* abort the corrupt inode, as it was not attached to the buffer */ From patchwork Fri Apr 17 15:08:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 11495367 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 122AF1392 for ; Fri, 17 Apr 2020 15:09:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EB8E22223C for ; Fri, 17 Apr 2020 15:09:06 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="DXt+iXdG" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728878AbgDQPJG (ORCPT ); Fri, 17 Apr 2020 11:09:06 -0400 Received: from us-smtp-2.mimecast.com ([205.139.110.61]:54296 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728308AbgDQPJF (ORCPT ); Fri, 17 Apr 2020 11:09:05 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587136143; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MBZCa4WsZ0mADsgx0dDOHIgOH2b9EkV8D5sl3i1939Y=; b=DXt+iXdGxS3rD+iY7Lzi+w1h/Jei9Bnt3OvVkwDRuIG0/8Rj+bE8tH1yf0wSpB+B30MDhY xwnboXT6fWerhZifHcAKkMSiJI6genWmXySDHzPst2Dau10h2J/Gm0fdfTOTDr5AWvTExT HaiprmySTd67x17xGyN6d54QRMAROqM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-308-5lN9FIcoOPm-XYS3wH5JSw-1; Fri, 17 Apr 2020 11:09:02 -0400 X-MC-Unique: 5lN9FIcoOPm-XYS3wH5JSw-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 72AC1DBA3 for ; Fri, 17 Apr 2020 15:09:01 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2DAE360BE0 for ; Fri, 17 Apr 2020 15:09:01 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH 03/12] xfs: always attach iflush_done and simplify error handling Date: Fri, 17 Apr 2020 11:08:50 -0400 Message-Id: <20200417150859.14734-4-bfoster@redhat.com> In-Reply-To: <20200417150859.14734-1-bfoster@redhat.com> References: <20200417150859.14734-1-bfoster@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org The inode flush code has several layers of error handling between the inode and cluster flushing code. If the inode flush fails before acquiring the backing buffer, the inode flush is aborted. If the cluster flush fails, the current inode flush is aborted and the cluster buffer is failed to handle the initial inode and any others that might have been attached before the error. Since xfs_iflush() is the only caller of xfs_iflush_cluser(), the error handling between the two can be condensed in the top-level function. If we update xfs_iflush_int() to attach the item completion handler to the buffer first, any errors that occur after the first call to xfs_iflush_int() can be handled with a buffer I/O failure. Lift the error handling from xfs_iflush_cluster() into xfs_iflush() and consolidate with the existing error handling. This also replaces the need to release the buffer because failing the buffer with XBF_ASYNC drops the current reference. Signed-off-by: Brian Foster Reviewed-by: Allison Collins --- fs/xfs/xfs_inode.c | 94 +++++++++++++++------------------------------- 1 file changed, 30 insertions(+), 64 deletions(-) diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index b539ee221ce5..4c9971ec6fa6 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -3496,6 +3496,7 @@ xfs_iflush_cluster( struct xfs_inode **cilist; struct xfs_inode *cip; struct xfs_ino_geometry *igeo = M_IGEO(mp); + int error = 0; int nr_found; int clcount = 0; int i; @@ -3588,11 +3589,10 @@ xfs_iflush_cluster( * re-check that it's dirty before flushing. */ if (!xfs_inode_clean(cip)) { - int error; error = xfs_iflush_int(cip, bp); if (error) { xfs_iunlock(cip, XFS_ILOCK_SHARED); - goto cluster_corrupt_out; + goto out_free; } clcount++; } else { @@ -3611,32 +3611,7 @@ xfs_iflush_cluster( kmem_free(cilist); out_put: xfs_perag_put(pag); - return 0; - - -cluster_corrupt_out: - /* - * Corruption detected in the clustering loop. Invalidate the - * inode buffer and shut down the filesystem. - */ - rcu_read_unlock(); - - /* - * We'll always have an inode attached to the buffer for completion - * process by the time we are called from xfs_iflush(). Hence we have - * always need to do IO completion processing to abort the inodes - * attached to the buffer. handle them just like the shutdown case in - * xfs_buf_submit(). - */ - ASSERT(bp->b_iodone); - xfs_buf_iofail(bp, XBF_ASYNC); - xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE); - - /* abort the corrupt inode, as it was not attached to the buffer */ - xfs_iflush_abort(cip, false); - kmem_free(cilist); - xfs_perag_put(pag); - return -EFSCORRUPTED; + return error; } /* @@ -3692,17 +3667,16 @@ xfs_iflush( */ if (XFS_FORCED_SHUTDOWN(mp)) { error = -EIO; - goto abort_out; + goto abort; } /* * Get the buffer containing the on-disk inode. We are doing a try-lock - * operation here, so we may get an EAGAIN error. In that case, we - * simply want to return with the inode still dirty. + * operation here, so we may get an EAGAIN error. In that case, return + * leaving the inode dirty. * * If we get any other error, we effectively have a corruption situation - * and we cannot flush the inode, so we treat it the same as failing - * xfs_iflush_int(). + * and we cannot flush the inode. Abort the flush and shut down. */ error = xfs_imap_to_bp(mp, NULL, &ip->i_imap, &dip, &bp, XBF_TRYLOCK, 0); @@ -3711,14 +3685,7 @@ xfs_iflush( return error; } if (error) - goto corrupt_out; - - /* - * First flush out the inode that xfs_iflush was called with. - */ - error = xfs_iflush_int(ip, bp); - if (error) - goto corrupt_out; + goto abort; /* * If the buffer is pinned then push on the log now so we won't @@ -3728,28 +3695,28 @@ xfs_iflush( xfs_log_force(mp, 0); /* - * inode clustering: try to gather other inodes into this write + * Flush the provided inode then attempt to gather others from the + * cluster into the write. * - * Note: Any error during clustering will result in the filesystem - * being shut down and completion callbacks run on the cluster buffer. - * As we have already flushed and attached this inode to the buffer, - * it has already been aborted and released by xfs_iflush_cluster() and - * so we have no further error handling to do here. + * Note: Once we attempt to flush an inode, we must run buffer + * completion callbacks on any failure. If this fails, simulate an I/O + * failure on the buffer and shut down. */ - error = xfs_iflush_cluster(ip, bp); - if (error) - return error; + error = xfs_iflush_int(ip, bp); + if (!error) + error = xfs_iflush_cluster(ip, bp); + if (error) { + xfs_buf_iofail(bp, XBF_ASYNC); + goto shutdown; + } *bpp = bp; return 0; -corrupt_out: - if (bp) - xfs_buf_relse(bp); - xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE); -abort_out: - /* abort the corrupt inode, as it was not attached to the buffer */ +abort: xfs_iflush_abort(ip, false); +shutdown: + xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE); return error; } @@ -3798,6 +3765,13 @@ xfs_iflush_int( ip->i_d.di_nextents > XFS_IFORK_MAXEXT(ip, XFS_DATA_FORK)); ASSERT(iip != NULL && iip->ili_fields != 0); + /* + * Attach the inode item callback to the buffer. Whether the flush + * succeeds or not, buffer I/O completion processing is now required to + * remove the inode from the AIL and release the flush lock. + */ + xfs_buf_attach_iodone(bp, xfs_iflush_done, &iip->ili_item); + /* set *dip = inode's place in the buffer */ dip = xfs_buf_offset(bp, ip->i_imap.im_boffset); @@ -3913,14 +3887,6 @@ xfs_iflush_int( xfs_trans_ail_copy_lsn(mp->m_ail, &iip->ili_flush_lsn, &iip->ili_item.li_lsn); - /* - * Attach the function xfs_iflush_done to the inode's - * buffer. This will remove the inode from the AIL - * and unlock the inode's flush lock when the inode is - * completely written to disk. - */ - xfs_buf_attach_iodone(bp, xfs_iflush_done, &iip->ili_item); - /* generate the checksum. */ xfs_dinode_calc_crc(mp, dip); From patchwork Fri Apr 17 15:08:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 11495365 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id C80C01392 for ; Fri, 17 Apr 2020 15:09:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AFB2F21924 for ; Fri, 17 Apr 2020 15:09:05 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="UludvWn6" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728869AbgDQPJF (ORCPT ); Fri, 17 Apr 2020 11:09:05 -0400 Received: from us-smtp-2.mimecast.com ([207.211.31.81]:39708 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728865AbgDQPJE (ORCPT ); Fri, 17 Apr 2020 11:09:04 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587136144; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=gpMBsU/jMI/ZvMgldHYmi+dYL/qC/ZrOvZKXjinlZN8=; b=UludvWn6833R5gdNmmNaU3otOWWWO8fgurC/ENmctgK255wpDf5TBpwhudV3t/9911uPX8 n7doLU9e+STT5aL3zXNIGvxXYl4h7ZsryWeQ5c2EFbpLoLzzrHC3/LV09oJPfOQi088/3f p1g/XLIQ6GlMWbK1bzYBK+HQ2318wAw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-505-4LdlqqVDMB6KADuHYN86tw-1; Fri, 17 Apr 2020 11:09:02 -0400 X-MC-Unique: 4LdlqqVDMB6KADuHYN86tw-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DDE101005513 for ; Fri, 17 Apr 2020 15:09:01 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id 98E5760BE0 for ; Fri, 17 Apr 2020 15:09:01 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH 04/12] xfs: remove unnecessary shutdown check from xfs_iflush() Date: Fri, 17 Apr 2020 11:08:51 -0400 Message-Id: <20200417150859.14734-5-bfoster@redhat.com> In-Reply-To: <20200417150859.14734-1-bfoster@redhat.com> References: <20200417150859.14734-1-bfoster@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org The shutdown check in xfs_iflush() duplicates checks down in the buffer code. If the fs is shut down, xfs_trans_read_buf_map() always returns an error and falls into the same error path. Remove the unnecessary check along with the warning in xfs_imap_to_bp() that generates excessive noise in the log if the fs is shut down. Signed-off-by: Brian Foster Reviewed-by: Allison Collins Reviewed-by: Dave Chinner --- fs/xfs/libxfs/xfs_inode_buf.c | 7 +------ fs/xfs/xfs_inode.c | 13 ------------- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c index 39c5a6e24915..b102e611bf54 100644 --- a/fs/xfs/libxfs/xfs_inode_buf.c +++ b/fs/xfs/libxfs/xfs_inode_buf.c @@ -172,12 +172,7 @@ xfs_imap_to_bp( (int)imap->im_len, buf_flags, &bp, &xfs_inode_buf_ops); if (error) { - if (error == -EAGAIN) { - ASSERT(buf_flags & XBF_TRYLOCK); - return error; - } - xfs_warn(mp, "%s: xfs_trans_read_buf() returned error %d.", - __func__, error); + ASSERT(error != -EAGAIN || (buf_flags & XBF_TRYLOCK)); return error; } diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index 4c9971ec6fa6..98ee1b10d1b0 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -3657,19 +3657,6 @@ xfs_iflush( return 0; } - /* - * This may have been unpinned because the filesystem is shutting - * down forcibly. If that's the case we must not write this inode - * to disk, because the log record didn't make it to disk. - * - * We also have to remove the log item from the AIL in this case, - * as we wait for an empty AIL as part of the unmount process. - */ - if (XFS_FORCED_SHUTDOWN(mp)) { - error = -EIO; - goto abort; - } - /* * Get the buffer containing the on-disk inode. We are doing a try-lock * operation here, so we may get an EAGAIN error. In that case, return From patchwork Fri Apr 17 15:08:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 11495371 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9E0EB13B2 for ; Fri, 17 Apr 2020 15:09:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7B5A021924 for ; Fri, 17 Apr 2020 15:09:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Qwh9emrs" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728308AbgDQPJH (ORCPT ); Fri, 17 Apr 2020 11:09:07 -0400 Received: from us-smtp-delivery-1.mimecast.com ([205.139.110.120]:48218 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728426AbgDQPJG (ORCPT ); Fri, 17 Apr 2020 11:09:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587136144; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=N7w98HjFzM7VvKglY0TQAbw1sk01i17T9bbL71Toa9M=; b=Qwh9emrsOxnC6eSH5aoi+t62WVfvO/BLDdQWIvALUnag482mWRer3jIG92BUdOEXvmoF7D jdfS3Y7nRCMn+sZ8oBTCq8zGegKSETlGqkUUCmYTeY7ei0W8P2XwZTrgfe01LGe4cIaYfl cadcAbGIsm028SzITWjXR+c5paXY9dA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-438-ejGue09oMKaibQ5uWbk3BQ-1; Fri, 17 Apr 2020 11:09:03 -0400 X-MC-Unique: ejGue09oMKaibQ5uWbk3BQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 54806DBA6 for ; Fri, 17 Apr 2020 15:09:02 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1035360BE0 for ; Fri, 17 Apr 2020 15:09:01 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH 05/12] xfs: ratelimit unmount time per-buffer I/O error warning Date: Fri, 17 Apr 2020 11:08:52 -0400 Message-Id: <20200417150859.14734-6-bfoster@redhat.com> In-Reply-To: <20200417150859.14734-1-bfoster@redhat.com> References: <20200417150859.14734-1-bfoster@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org At unmount time, XFS emits a warning for every in-core buffer that might have undergone a write error. In practice this behavior is probably reasonable given that the filesystem is likely short lived once I/O errors begin to occur consistently. Under certain test or otherwise expected error conditions, this can spam the logs and slow down the unmount. Ratelimit the warning to prevent this problem while still informing the user that errors have occurred. Signed-off-by: Brian Foster Reviewed-by: Allison Collins --- fs/xfs/xfs_buf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index 93942d8e35dd..5120fed06075 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -1685,11 +1685,10 @@ xfs_wait_buftarg( bp = list_first_entry(&dispose, struct xfs_buf, b_lru); list_del_init(&bp->b_lru); if (bp->b_flags & XBF_WRITE_FAIL) { - xfs_alert(btp->bt_mount, -"Corruption Alert: Buffer at daddr 0x%llx had permanent write failures!", + xfs_alert_ratelimited(btp->bt_mount, +"Corruption Alert: Buffer at daddr 0x%llx had permanent write failures!\n" +"Please run xfs_repair to determine the extent of the problem.", (long long)bp->b_bn); - xfs_alert(btp->bt_mount, -"Please run xfs_repair to determine the extent of the problem."); } xfs_buf_rele(bp); } From patchwork Fri Apr 17 15:08:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 11495373 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 03E591871 for ; Fri, 17 Apr 2020 15:09:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D6D142223C for ; Fri, 17 Apr 2020 15:09:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="S3HIwM6F" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728883AbgDQPJH (ORCPT ); Fri, 17 Apr 2020 11:09:07 -0400 Received: from us-smtp-2.mimecast.com ([207.211.31.81]:51981 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728865AbgDQPJG (ORCPT ); Fri, 17 Apr 2020 11:09:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587136145; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=pZaGMY1hW/lUitibRHkcNURWUfqtoVcu62RcGDEm1ts=; b=S3HIwM6FgjcTX1rGQpFnJFpBv7GZ3eE5qxxMIcd9bbDAML0T8PdMKL9IiFzt5Xw1TDqLYp ELT3bn47eYaSVRx6R5CKMuhH98xcgGg2HWdIa3ImPdClhvvx+b2eyxuJYHqkJfVsvrGsCs DqK7X4u0V6NigkcTZ2vfR3xnRI+o1XU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-495-k9WzYsn9PMes8cQr2pj4wQ-1; Fri, 17 Apr 2020 11:09:03 -0400 X-MC-Unique: k9WzYsn9PMes8cQr2pj4wQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C2774102C8D3 for ; Fri, 17 Apr 2020 15:09:02 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7C68F60BE0 for ; Fri, 17 Apr 2020 15:09:02 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH 06/12] xfs: remove duplicate verification from xfs_qm_dqflush() Date: Fri, 17 Apr 2020 11:08:53 -0400 Message-Id: <20200417150859.14734-7-bfoster@redhat.com> In-Reply-To: <20200417150859.14734-1-bfoster@redhat.com> References: <20200417150859.14734-1-bfoster@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org The dquot read/write verifier calls xfs_dqblk_verify() on every dquot in the buffer. Remove the duplicate call from xfs_qm_dqflush(). Signed-off-by: Brian Foster --- fs/xfs/xfs_dquot.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c index af2c8e5ceea0..73032c18a94a 100644 --- a/fs/xfs/xfs_dquot.c +++ b/fs/xfs/xfs_dquot.c @@ -1071,7 +1071,6 @@ xfs_qm_dqflush( struct xfs_buf *bp; struct xfs_dqblk *dqb; struct xfs_disk_dquot *ddqp; - xfs_failaddr_t fa; int error; ASSERT(XFS_DQ_IS_LOCKED(dqp)); @@ -1116,19 +1115,6 @@ xfs_qm_dqflush( dqb = bp->b_addr + dqp->q_bufoffset; ddqp = &dqb->dd_diskdq; - /* - * A simple sanity check in case we got a corrupted dquot. - */ - fa = xfs_dqblk_verify(mp, dqb, be32_to_cpu(ddqp->d_id), 0); - if (fa) { - xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS", - be32_to_cpu(ddqp->d_id), fa); - xfs_buf_relse(bp); - xfs_dqfunlock(dqp); - xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE); - return -EFSCORRUPTED; - } - /* This is the only portion of data that needs to persist */ memcpy(ddqp, &dqp->q_core, sizeof(struct xfs_disk_dquot)); From patchwork Fri Apr 17 15:08:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 11495379 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 7111D174A for ; Fri, 17 Apr 2020 15:09:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 59D452223C for ; Fri, 17 Apr 2020 15:09:09 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="F3OEs92c" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728564AbgDQPJI (ORCPT ); Fri, 17 Apr 2020 11:09:08 -0400 Received: from us-smtp-delivery-1.mimecast.com ([205.139.110.120]:49411 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728874AbgDQPJH (ORCPT ); Fri, 17 Apr 2020 11:09:07 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587136145; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=J8NkgnJMXUkfNkR2+KWQu16cSSej33kNjPCb+IUTlGM=; b=F3OEs92cZpviC+Oi/DC3hbyan3f4gyEp9qQiteLo+jNrIvVFsZT3fVJhTEOHHcyXKx5Yej 2z/qZuutUyVdQBQgf5YAVHGREBmnUsN+i9bhCVqfJzOC3feugdV+Qxd0Qk4AbS8S01APYv dhd4fQclyR90n+wxSbRg1NUk+jBN1xY= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-256-ZxSYfBVJO1uBHWRbesypUw-1; Fri, 17 Apr 2020 11:09:04 -0400 X-MC-Unique: ZxSYfBVJO1uBHWRbesypUw-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3ACC21088380 for ; Fri, 17 Apr 2020 15:09:03 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id E7BDF60BE0 for ; Fri, 17 Apr 2020 15:09:02 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH 07/12] xfs: abort consistently on dquot flush failure Date: Fri, 17 Apr 2020 11:08:54 -0400 Message-Id: <20200417150859.14734-8-bfoster@redhat.com> In-Reply-To: <20200417150859.14734-1-bfoster@redhat.com> References: <20200417150859.14734-1-bfoster@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org The dquot flush handler effectively aborts the dquot flush if the filesystem is already shut down, but doesn't actually shut down if the flush fails. Update xfs_qm_dqflush() to consistently abort the dquot flush and shutdown the fs if the flush fails with an unexpected error. Signed-off-by: Brian Foster Reviewed-by: Dave Chinner Reviewed-by: Allison Collins --- fs/xfs/xfs_dquot.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c index 73032c18a94a..41750f797861 100644 --- a/fs/xfs/xfs_dquot.c +++ b/fs/xfs/xfs_dquot.c @@ -1068,6 +1068,7 @@ xfs_qm_dqflush( struct xfs_buf **bpp) { struct xfs_mount *mp = dqp->q_mount; + struct xfs_log_item *lip = &dqp->q_logitem.qli_item; struct xfs_buf *bp; struct xfs_dqblk *dqb; struct xfs_disk_dquot *ddqp; @@ -1082,32 +1083,16 @@ xfs_qm_dqflush( xfs_qm_dqunpin_wait(dqp); - /* - * This may have been unpinned because the filesystem is shutting - * down forcibly. If that's the case we must not write this dquot - * to disk, because the log record didn't make it to disk. - * - * We also have to remove the log item from the AIL in this case, - * as we wait for an emptry AIL as part of the unmount process. - */ - if (XFS_FORCED_SHUTDOWN(mp)) { - struct xfs_log_item *lip = &dqp->q_logitem.qli_item; - dqp->dq_flags &= ~XFS_DQ_DIRTY; - - xfs_trans_ail_remove(lip, SHUTDOWN_CORRUPT_INCORE); - - error = -EIO; - goto out_unlock; - } - /* * Get the buffer containing the on-disk dquot */ error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno, mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK, &bp, &xfs_dquot_buf_ops); - if (error) + if (error == -EAGAIN) goto out_unlock; + if (error) + goto out_abort; /* * Calculate the location of the dquot inside the buffer. @@ -1161,6 +1146,10 @@ xfs_qm_dqflush( *bpp = bp; return 0; +out_abort: + dqp->dq_flags &= ~XFS_DQ_DIRTY; + xfs_trans_ail_remove(lip, SHUTDOWN_CORRUPT_INCORE); + xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE); out_unlock: xfs_dqfunlock(dqp); return error; From patchwork Fri Apr 17 15:08:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 11495381 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1779113B2 for ; Fri, 17 Apr 2020 15:09:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0145B2223D for ; Fri, 17 Apr 2020 15:09:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="aZHbdQ1/" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728288AbgDQPJI (ORCPT ); Fri, 17 Apr 2020 11:09:08 -0400 Received: from us-smtp-delivery-1.mimecast.com ([207.211.31.120]:60498 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728852AbgDQPJH (ORCPT ); Fri, 17 Apr 2020 11:09:07 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587136146; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=r/P9fWlZZlP1VXzxENU8G2/Bi5QqxOKryo3/Ck9Brg8=; b=aZHbdQ1/l5eWkomBj2qfo3iV/uq0DlkX3RZeoAxzAX/5YRpQnMiFIaG6SI4REWr/Fa4ajV umsTgx7fiyc7RapZdTO2IU2kvRAW60rBcUqXFs+nD7qEQ9Rg6vy1LhCcH8RxGwxbIUefYJ 51UGlgid/UXElZl+d/6m4kwh6ftn+h8= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-37-ZGjLdPJVPP6Fbfs3zHXoiA-1; Fri, 17 Apr 2020 11:09:04 -0400 X-MC-Unique: ZGjLdPJVPP6Fbfs3zHXoiA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A8EA380268B for ; Fri, 17 Apr 2020 15:09:03 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5F77760BE0 for ; Fri, 17 Apr 2020 15:09:03 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH 08/12] xfs: remove unnecessary quotaoff intent item push handler Date: Fri, 17 Apr 2020 11:08:55 -0400 Message-Id: <20200417150859.14734-9-bfoster@redhat.com> In-Reply-To: <20200417150859.14734-1-bfoster@redhat.com> References: <20200417150859.14734-1-bfoster@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org The quotaoff intent item push handler unconditionally returns locked status because it remains AIL resident until removed by the quotafoff end intent. xfsaild_push_item() already returns pinned status for items (generally intents) without a push handler. This is effectively the same behavior for the purpose of quotaoff, so remove the unnecessary quotaoff push handler. Signed-off-by: Brian Foster --- fs/xfs/xfs_dquot_item.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c index 5a7808299a32..582b3796a0c9 100644 --- a/fs/xfs/xfs_dquot_item.c +++ b/fs/xfs/xfs_dquot_item.c @@ -274,18 +274,6 @@ xfs_qm_qoff_logitem_format( xlog_finish_iovec(lv, vecp, sizeof(struct xfs_qoff_logitem)); } -/* - * There isn't much you can do to push a quotaoff item. It is simply - * stuck waiting for the log to be flushed to disk. - */ -STATIC uint -xfs_qm_qoff_logitem_push( - struct xfs_log_item *lip, - struct list_head *buffer_list) -{ - return XFS_ITEM_LOCKED; -} - STATIC xfs_lsn_t xfs_qm_qoffend_logitem_committed( struct xfs_log_item *lip, @@ -318,14 +306,12 @@ static const struct xfs_item_ops xfs_qm_qoffend_logitem_ops = { .iop_size = xfs_qm_qoff_logitem_size, .iop_format = xfs_qm_qoff_logitem_format, .iop_committed = xfs_qm_qoffend_logitem_committed, - .iop_push = xfs_qm_qoff_logitem_push, .iop_release = xfs_qm_qoff_logitem_release, }; static const struct xfs_item_ops xfs_qm_qoff_logitem_ops = { .iop_size = xfs_qm_qoff_logitem_size, .iop_format = xfs_qm_qoff_logitem_format, - .iop_push = xfs_qm_qoff_logitem_push, .iop_release = xfs_qm_qoff_logitem_release, }; From patchwork Fri Apr 17 15:08:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 11495383 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5A72F13B2 for ; Fri, 17 Apr 2020 15:09:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3CC6120857 for ; Fri, 17 Apr 2020 15:09:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="aXXV5vpU" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728852AbgDQPJK (ORCPT ); Fri, 17 Apr 2020 11:09:10 -0400 Received: from us-smtp-delivery-1.mimecast.com ([207.211.31.120]:36719 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728867AbgDQPJI (ORCPT ); Fri, 17 Apr 2020 11:09:08 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587136147; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=yXO/dhQQgrEGWlnaDkvWqP+M920YOhxTGASDTDAGpqY=; b=aXXV5vpUmmqRoQGZIrcd8bYJj2s4leumfPkT65+vcpi4EwYGmSzu6efAyZ/M8VVI5hIc7A pOqCRi4vF+mchOmR7SqacffsgV+EsRmPG+U4SmZ4Ztlc4D8YyZhl4NMOZhRYkB0Jynq+GZ i9JTLO21/FOQ/olT3F4s2WuaErtWFjA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-309-RhK-20rANbOiQu4NIwN2dQ-1; Fri, 17 Apr 2020 11:09:04 -0400 X-MC-Unique: RhK-20rANbOiQu4NIwN2dQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 1EAFE1088384 for ; Fri, 17 Apr 2020 15:09:04 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id CB64860BE0 for ; Fri, 17 Apr 2020 15:09:03 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH 09/12] xfs: elide the AIL lock on log item failure tracking Date: Fri, 17 Apr 2020 11:08:56 -0400 Message-Id: <20200417150859.14734-10-bfoster@redhat.com> In-Reply-To: <20200417150859.14734-1-bfoster@redhat.com> References: <20200417150859.14734-1-bfoster@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org The log item failed flag is used to indicate a log item was flushed but the underlying buffer was not successfully written to disk. If the error configuration allows for writeback retries, xfsaild uses the flag to resubmit the underlying buffer without acquiring the flush lock of the item. The flag is currently set and cleared under the AIL lock and buffer lock in the ->iop_error() callback, invoked via ->b_iodone() at I/O completion time. The flag is checked at xfsaild push time under AIL lock and cleared under buffer lock before resubmission. If I/O eventually succeeds, the flag is cleared in the _done() handler for the associated item type, again under AIL lock and buffer lock. As far as I can tell, the only reason for holding the AIL lock across sets/clears is to manage consistency between the log item bitop state and the temporary buffer pointer that is attached to the log item. The bit itself is used to manage consistency of the attached buffer, but is not enough to guarantee the buffer is still attached by the time xfsaild attempts to access it. However since failure state is always set or cleared under buffer lock (either via I/O completion or xfsaild), this particular case can be handled at item push time by verifying failure state once under buffer lock. Remove the AIL lock protection from the various bits of log item failure state management and simplify the surrounding code where applicable. Use the buffer lock in the xfsaild resubmit code to detect failure state changes and temporarily treat the item as locked. Signed-off-by: Brian Foster --- fs/xfs/xfs_buf_item.c | 4 ---- fs/xfs/xfs_dquot.c | 41 +++++++++++++++-------------------------- fs/xfs/xfs_inode_item.c | 11 +++-------- fs/xfs/xfs_trans_ail.c | 12 ++++++++++-- fs/xfs/xfs_trans_priv.h | 5 ----- 5 files changed, 28 insertions(+), 45 deletions(-) diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c index 72d37a4609d8..6b000f855e13 100644 --- a/fs/xfs/xfs_buf_item.c +++ b/fs/xfs/xfs_buf_item.c @@ -1046,7 +1046,6 @@ xfs_buf_do_callbacks_fail( struct xfs_buf *bp) { struct xfs_log_item *lip; - struct xfs_ail *ailp; /* * Buffer log item errors are handled directly by xfs_buf_item_push() @@ -1058,13 +1057,10 @@ xfs_buf_do_callbacks_fail( lip = list_first_entry(&bp->b_li_list, struct xfs_log_item, li_bio_list); - ailp = lip->li_ailp; - spin_lock(&ailp->ail_lock); list_for_each_entry(lip, &bp->b_li_list, li_bio_list) { if (lip->li_ops->iop_error) lip->li_ops->iop_error(lip, bp); } - spin_unlock(&ailp->ail_lock); } static bool diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c index 41750f797861..953059235130 100644 --- a/fs/xfs/xfs_dquot.c +++ b/fs/xfs/xfs_dquot.c @@ -1023,34 +1023,23 @@ xfs_qm_dqflush_done( struct xfs_ail *ailp = lip->li_ailp; /* - * We only want to pull the item from the AIL if its - * location in the log has not changed since we started the flush. - * Thus, we only bother if the dquot's lsn has - * not changed. First we check the lsn outside the lock - * since it's cheaper, and then we recheck while - * holding the lock before removing the dquot from the AIL. + * Only pull the item from the AIL if its location in the log has not + * changed since it was flushed. Do a lockless check first to reduce + * lock traffic. */ - if (test_bit(XFS_LI_IN_AIL, &lip->li_flags) && - ((lip->li_lsn == qip->qli_flush_lsn) || - test_bit(XFS_LI_FAILED, &lip->li_flags))) { - - /* xfs_trans_ail_delete() drops the AIL lock. */ - spin_lock(&ailp->ail_lock); - if (lip->li_lsn == qip->qli_flush_lsn) { - xfs_trans_ail_delete(ailp, lip, SHUTDOWN_CORRUPT_INCORE); - } else { - /* - * Clear the failed state since we are about to drop the - * flush lock - */ - xfs_clear_li_failed(lip); - spin_unlock(&ailp->ail_lock); - } - } + if (!test_bit(XFS_LI_IN_AIL, &lip->li_flags) || + lip->li_lsn != qip->qli_flush_lsn) + goto out; - /* - * Release the dq's flush lock since we're done with it. - */ + spin_lock(&ailp->ail_lock); + if (lip->li_lsn == qip->qli_flush_lsn) + /* xfs_trans_ail_delete() drops the AIL lock */ + xfs_trans_ail_delete(ailp, lip, SHUTDOWN_CORRUPT_INCORE); + else + spin_unlock(&ailp->ail_lock); + +out: + xfs_clear_li_failed(lip); xfs_dqfunlock(dqp); } diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c index 1d4d256a2e96..0ae61844b224 100644 --- a/fs/xfs/xfs_inode_item.c +++ b/fs/xfs/xfs_inode_item.c @@ -682,9 +682,7 @@ xfs_iflush_done( * Scan the buffer IO completions for other inodes being completed and * attach them to the current inode log item. */ - list_add_tail(&lip->li_bio_list, &tmp); - list_for_each_entry_safe(blip, n, &bp->b_li_list, li_bio_list) { if (lip->li_cb != xfs_iflush_done) continue; @@ -695,15 +693,13 @@ xfs_iflush_done( * the AIL lock. */ iip = INODE_ITEM(blip); - if ((iip->ili_logged && blip->li_lsn == iip->ili_flush_lsn) || - test_bit(XFS_LI_FAILED, &blip->li_flags)) + if (iip->ili_logged && blip->li_lsn == iip->ili_flush_lsn) need_ail++; } /* make sure we capture the state of the initial inode. */ iip = INODE_ITEM(lip); - if ((iip->ili_logged && lip->li_lsn == iip->ili_flush_lsn) || - test_bit(XFS_LI_FAILED, &lip->li_flags)) + if (iip->ili_logged && lip->li_lsn == iip->ili_flush_lsn) need_ail++; /* @@ -732,8 +728,6 @@ xfs_iflush_done( xfs_lsn_t lsn = xfs_ail_delete_one(ailp, blip); if (!tail_lsn && lsn) tail_lsn = lsn; - } else { - xfs_clear_li_failed(blip); } } xfs_ail_update_finish(ailp, tail_lsn); @@ -746,6 +740,7 @@ xfs_iflush_done( */ list_for_each_entry_safe(blip, n, &tmp, li_bio_list) { list_del_init(&blip->li_bio_list); + xfs_clear_li_failed(blip); iip = INODE_ITEM(blip); iip->ili_logged = 0; iip->ili_last_fields = 0; diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c index 0c709651a2c6..6af609070143 100644 --- a/fs/xfs/xfs_trans_ail.c +++ b/fs/xfs/xfs_trans_ail.c @@ -368,15 +368,23 @@ xfsaild_push_failed( { struct xfs_buf *bp = lip->li_buf; - if (!xfs_buf_trylock(bp)) + /* + * Log item state bits are racy so we cannot assume the temporary buffer + * pointer is set. Treat the item as locked if the pointer is clear or + * the failure state has changed once we've locked out I/O completion. + */ + if (!bp || !xfs_buf_trylock(bp)) return XFS_ITEM_LOCKED; + if (!test_bit(XFS_LI_FAILED, &lip->li_flags)) { + xfs_buf_unlock(bp); + return XFS_ITEM_LOCKED; + } if (!xfs_buf_delwri_queue(bp, buffer_list)) { xfs_buf_unlock(bp); return XFS_ITEM_FLUSHING; } - /* protected by ail_lock */ list_for_each_entry(lip, &bp->b_li_list, li_bio_list) xfs_clear_li_failed(lip); diff --git a/fs/xfs/xfs_trans_priv.h b/fs/xfs/xfs_trans_priv.h index 35655eac01a6..9135afdcee9d 100644 --- a/fs/xfs/xfs_trans_priv.h +++ b/fs/xfs/xfs_trans_priv.h @@ -158,9 +158,6 @@ xfs_clear_li_failed( { struct xfs_buf *bp = lip->li_buf; - ASSERT(test_bit(XFS_LI_IN_AIL, &lip->li_flags)); - lockdep_assert_held(&lip->li_ailp->ail_lock); - if (test_and_clear_bit(XFS_LI_FAILED, &lip->li_flags)) { lip->li_buf = NULL; xfs_buf_rele(bp); @@ -172,8 +169,6 @@ xfs_set_li_failed( struct xfs_log_item *lip, struct xfs_buf *bp) { - lockdep_assert_held(&lip->li_ailp->ail_lock); - if (!test_and_set_bit(XFS_LI_FAILED, &lip->li_flags)) { xfs_buf_hold(bp); lip->li_buf = bp; From patchwork Fri Apr 17 15:08:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 11495385 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 16B331392 for ; Fri, 17 Apr 2020 15:09:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EFC2E2223C for ; Fri, 17 Apr 2020 15:09:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="HmOeYaN+" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728137AbgDQPJL (ORCPT ); Fri, 17 Apr 2020 11:09:11 -0400 Received: from us-smtp-2.mimecast.com ([207.211.31.81]:49504 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728865AbgDQPJJ (ORCPT ); Fri, 17 Apr 2020 11:09:09 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587136147; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=LY7lZ1PcLq5bv52aF1mtRhEy/2hE+VeaEJJZbGXaUaM=; b=HmOeYaN+Z9D02ZktM1cPYCrZhuELCK64T6zcpnm0RctRS/XQPi3rANO6qzH5uw+HWW69GZ O2bDnyDx2fPNug5T18yr351UWWAlOzHtP5gAbbEvvP4/dAF9yTyOo7IZsvL3wgfJa/yV8b rn8r7CbjyNlYRzc1M0omg3csPtxV8oc= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-465-R8b8JFTIPvKJ7gEBAQ7f1A-1; Fri, 17 Apr 2020 11:09:05 -0400 X-MC-Unique: R8b8JFTIPvKJ7gEBAQ7f1A-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8B09D802681 for ; Fri, 17 Apr 2020 15:09:04 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4393F60BE0 for ; Fri, 17 Apr 2020 15:09:04 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH 10/12] xfs: clean up AIL log item removal functions Date: Fri, 17 Apr 2020 11:08:57 -0400 Message-Id: <20200417150859.14734-11-bfoster@redhat.com> In-Reply-To: <20200417150859.14734-1-bfoster@redhat.com> References: <20200417150859.14734-1-bfoster@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org We have two AIL removal functions with slightly different semantics. xfs_trans_ail_delete() expects the caller to have the AIL lock and for the associated item to be AIL resident. If not, the filesystem is shut down. xfs_trans_ail_remove() acquires the AIL lock, checks that the item is AIL resident and calls the former if so. These semantics lead to confused usage between the two. For example, the _remove() variant takes a shutdown parameter to pass to the _delete() variant, but immediately returns if the AIL bit is not set. This means that _remove() would never shut down if an item is not AIL resident, even though it appears that many callers would expect it to. Make the following changes to clean up both of these functions: - Most callers of xfs_trans_ail_delete() acquire the AIL lock just before the call. Update _delete() to acquire the lock and open code the couple of callers that make additional checks under AIL lock. - Drop the unnecessary ailp parameter from _delete(). - Drop the unused shutdown parameter from _remove() and open code the implementation. In summary, this leaves a _delete() variant that expects an AIL resident item and a _remove() helper that checks the AIL bit. Audit the existing callsites for use of the appropriate function and update as necessary. Signed-off-by: Brian Foster --- fs/xfs/xfs_bmap_item.c | 2 +- fs/xfs/xfs_buf_item.c | 21 ++++++++------------- fs/xfs/xfs_dquot.c | 12 +++++++----- fs/xfs/xfs_dquot_item.c | 2 +- fs/xfs/xfs_extfree_item.c | 2 +- fs/xfs/xfs_inode_item.c | 6 +----- fs/xfs/xfs_refcount_item.c | 2 +- fs/xfs/xfs_rmap_item.c | 2 +- fs/xfs/xfs_trans_ail.c | 3 ++- fs/xfs/xfs_trans_priv.h | 17 +++++++++-------- 10 files changed, 32 insertions(+), 37 deletions(-) diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c index ee6f4229cebc..909221a4a8ab 100644 --- a/fs/xfs/xfs_bmap_item.c +++ b/fs/xfs/xfs_bmap_item.c @@ -51,7 +51,7 @@ xfs_bui_release( { ASSERT(atomic_read(&buip->bui_refcount) > 0); if (atomic_dec_and_test(&buip->bui_refcount)) { - xfs_trans_ail_remove(&buip->bui_item, SHUTDOWN_LOG_IO_ERROR); + xfs_trans_ail_delete(&buip->bui_item, SHUTDOWN_LOG_IO_ERROR); xfs_bui_item_free(buip); } } diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c index 6b000f855e13..1bf1c14d4ebb 100644 --- a/fs/xfs/xfs_buf_item.c +++ b/fs/xfs/xfs_buf_item.c @@ -410,7 +410,6 @@ xfs_buf_item_unpin( { struct xfs_buf_log_item *bip = BUF_ITEM(lip); xfs_buf_t *bp = bip->bli_buf; - struct xfs_ail *ailp = lip->li_ailp; int stale = bip->bli_flags & XFS_BLI_STALE; int freed; @@ -463,8 +462,7 @@ xfs_buf_item_unpin( list_del_init(&bp->b_li_list); bp->b_iodone = NULL; } else { - spin_lock(&ailp->ail_lock); - xfs_trans_ail_delete(ailp, lip, SHUTDOWN_LOG_IO_ERROR); + xfs_trans_ail_delete(lip, SHUTDOWN_LOG_IO_ERROR); xfs_buf_item_relse(bp); ASSERT(bp->b_log_item == NULL); } @@ -568,7 +566,7 @@ xfs_buf_item_put( * state. */ if (aborted) - xfs_trans_ail_remove(lip, SHUTDOWN_LOG_IO_ERROR); + xfs_trans_ail_delete(lip, SHUTDOWN_LOG_IO_ERROR); xfs_buf_item_relse(bip->bli_buf); return true; } @@ -1209,22 +1207,19 @@ xfs_buf_iodone( struct xfs_buf *bp, struct xfs_log_item *lip) { - struct xfs_ail *ailp = lip->li_ailp; - ASSERT(BUF_ITEM(lip)->bli_buf == bp); xfs_buf_rele(bp); /* - * If we are forcibly shutting down, this may well be - * off the AIL already. That's because we simulate the - * log-committed callbacks to unpin these buffers. Or we may never - * have put this item on AIL because of the transaction was - * aborted forcibly. xfs_trans_ail_delete() takes care of these. + * If we are forcibly shutting down, this may well be off the AIL + * already. That's because we simulate the log-committed callbacks to + * unpin these buffers. Or we may never have put this item on AIL + * because of the transaction was aborted forcibly. + * xfs_trans_ail_delete() takes care of these. * * Either way, AIL is useless if we're forcing a shutdown. */ - spin_lock(&ailp->ail_lock); - xfs_trans_ail_delete(ailp, lip, SHUTDOWN_CORRUPT_INCORE); + xfs_trans_ail_delete(lip, SHUTDOWN_CORRUPT_INCORE); xfs_buf_item_free(BUF_ITEM(lip)); } diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c index 953059235130..996751dd6302 100644 --- a/fs/xfs/xfs_dquot.c +++ b/fs/xfs/xfs_dquot.c @@ -1021,6 +1021,7 @@ xfs_qm_dqflush_done( struct xfs_dq_logitem *qip = (struct xfs_dq_logitem *)lip; struct xfs_dquot *dqp = qip->qli_dquot; struct xfs_ail *ailp = lip->li_ailp; + xfs_lsn_t tail_lsn; /* * Only pull the item from the AIL if its location in the log has not @@ -1032,10 +1033,11 @@ xfs_qm_dqflush_done( goto out; spin_lock(&ailp->ail_lock); - if (lip->li_lsn == qip->qli_flush_lsn) - /* xfs_trans_ail_delete() drops the AIL lock */ - xfs_trans_ail_delete(ailp, lip, SHUTDOWN_CORRUPT_INCORE); - else + if (lip->li_lsn == qip->qli_flush_lsn) { + /* xfs_ail_update_finish() drops the AIL lock */ + tail_lsn = xfs_ail_delete_one(ailp, lip); + xfs_ail_update_finish(ailp, tail_lsn); + } else spin_unlock(&ailp->ail_lock); out: @@ -1137,7 +1139,7 @@ xfs_qm_dqflush( out_abort: dqp->dq_flags &= ~XFS_DQ_DIRTY; - xfs_trans_ail_remove(lip, SHUTDOWN_CORRUPT_INCORE); + xfs_trans_ail_remove(lip); xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE); out_unlock: xfs_dqfunlock(dqp); diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c index 582b3796a0c9..f129cfcc36be 100644 --- a/fs/xfs/xfs_dquot_item.c +++ b/fs/xfs/xfs_dquot_item.c @@ -329,7 +329,7 @@ xfs_qm_qoff_logitem_relse( ASSERT(test_bit(XFS_LI_IN_AIL, &lip->li_flags) || test_bit(XFS_LI_ABORTED, &lip->li_flags) || XFS_FORCED_SHUTDOWN(lip->li_mountp)); - xfs_trans_ail_remove(lip, SHUTDOWN_LOG_IO_ERROR); + xfs_trans_ail_remove(lip); kmem_free(lip->li_lv_shadow); kmem_free(qoff); } diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c index 6ea847f6e298..cd98eba24884 100644 --- a/fs/xfs/xfs_extfree_item.c +++ b/fs/xfs/xfs_extfree_item.c @@ -55,7 +55,7 @@ xfs_efi_release( { ASSERT(atomic_read(&efip->efi_refcount) > 0); if (atomic_dec_and_test(&efip->efi_refcount)) { - xfs_trans_ail_remove(&efip->efi_item, SHUTDOWN_LOG_IO_ERROR); + xfs_trans_ail_delete(&efip->efi_item, SHUTDOWN_LOG_IO_ERROR); xfs_efi_item_free(efip); } } diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c index 0ae61844b224..f8dd9bb8c851 100644 --- a/fs/xfs/xfs_inode_item.c +++ b/fs/xfs/xfs_inode_item.c @@ -763,11 +763,7 @@ xfs_iflush_abort( xfs_inode_log_item_t *iip = ip->i_itemp; if (iip) { - if (test_bit(XFS_LI_IN_AIL, &iip->ili_item.li_flags)) { - xfs_trans_ail_remove(&iip->ili_item, - stale ? SHUTDOWN_LOG_IO_ERROR : - SHUTDOWN_CORRUPT_INCORE); - } + xfs_trans_ail_remove(&iip->ili_item); iip->ili_logged = 0; /* * Clear the ili_last_fields bits now that we know that the diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c index 8eeed73928cd..712939a015a9 100644 --- a/fs/xfs/xfs_refcount_item.c +++ b/fs/xfs/xfs_refcount_item.c @@ -50,7 +50,7 @@ xfs_cui_release( { ASSERT(atomic_read(&cuip->cui_refcount) > 0); if (atomic_dec_and_test(&cuip->cui_refcount)) { - xfs_trans_ail_remove(&cuip->cui_item, SHUTDOWN_LOG_IO_ERROR); + xfs_trans_ail_delete(&cuip->cui_item, SHUTDOWN_LOG_IO_ERROR); xfs_cui_item_free(cuip); } } diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c index 4911b68f95dd..ff949b32c051 100644 --- a/fs/xfs/xfs_rmap_item.c +++ b/fs/xfs/xfs_rmap_item.c @@ -50,7 +50,7 @@ xfs_rui_release( { ASSERT(atomic_read(&ruip->rui_refcount) > 0); if (atomic_dec_and_test(&ruip->rui_refcount)) { - xfs_trans_ail_remove(&ruip->rui_item, SHUTDOWN_LOG_IO_ERROR); + xfs_trans_ail_delete(&ruip->rui_item, SHUTDOWN_LOG_IO_ERROR); xfs_rui_item_free(ruip); } } diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c index 6af609070143..80acdb89bd6e 100644 --- a/fs/xfs/xfs_trans_ail.c +++ b/fs/xfs/xfs_trans_ail.c @@ -872,13 +872,14 @@ xfs_ail_delete_one( */ void xfs_trans_ail_delete( - struct xfs_ail *ailp, struct xfs_log_item *lip, int shutdown_type) { + struct xfs_ail *ailp = lip->li_ailp; struct xfs_mount *mp = ailp->ail_mount; xfs_lsn_t tail_lsn; + spin_lock(&ailp->ail_lock); if (!test_bit(XFS_LI_IN_AIL, &lip->li_flags)) { spin_unlock(&ailp->ail_lock); if (!XFS_FORCED_SHUTDOWN(mp)) { diff --git a/fs/xfs/xfs_trans_priv.h b/fs/xfs/xfs_trans_priv.h index 9135afdcee9d..7563c78e2997 100644 --- a/fs/xfs/xfs_trans_priv.h +++ b/fs/xfs/xfs_trans_priv.h @@ -94,22 +94,23 @@ xfs_trans_ail_update( xfs_lsn_t xfs_ail_delete_one(struct xfs_ail *ailp, struct xfs_log_item *lip); void xfs_ail_update_finish(struct xfs_ail *ailp, xfs_lsn_t old_lsn) __releases(ailp->ail_lock); -void xfs_trans_ail_delete(struct xfs_ail *ailp, struct xfs_log_item *lip, - int shutdown_type); +void xfs_trans_ail_delete(struct xfs_log_item *lip, int shutdown_type); static inline void xfs_trans_ail_remove( - struct xfs_log_item *lip, - int shutdown_type) + struct xfs_log_item *lip) { struct xfs_ail *ailp = lip->li_ailp; + xfs_lsn_t tail_lsn; spin_lock(&ailp->ail_lock); - /* xfs_trans_ail_delete() drops the AIL lock */ - if (test_bit(XFS_LI_IN_AIL, &lip->li_flags)) - xfs_trans_ail_delete(ailp, lip, shutdown_type); - else + /* xfs_ail_update_finish() drops the AIL lock */ + if (test_bit(XFS_LI_IN_AIL, &lip->li_flags)) { + tail_lsn = xfs_ail_delete_one(ailp, lip); + xfs_ail_update_finish(ailp, tail_lsn); + } else { spin_unlock(&ailp->ail_lock); + } } void xfs_ail_push(struct xfs_ail *, xfs_lsn_t); From patchwork Fri Apr 17 15:08:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 11495387 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3B14D174A for ; Fri, 17 Apr 2020 15:09:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 21A4C21924 for ; Fri, 17 Apr 2020 15:09:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Z4xRvt6f" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728373AbgDQPJM (ORCPT ); Fri, 17 Apr 2020 11:09:12 -0400 Received: from us-smtp-delivery-1.mimecast.com ([205.139.110.120]:48078 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728885AbgDQPJI (ORCPT ); Fri, 17 Apr 2020 11:09:08 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587136147; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zAatR+aXNdcCh0+s5b7P+nuFcv4mgHM0z+tUrhJ5TGo=; b=Z4xRvt6fb42FQ8G4K7UeS0v3vCmSd2tz1IK9gleEYxmPDFXCO1e1upzjGmuF74PfQtRbQ9 GsVEIRjM+dg9qqr0lqk5FvJk0w1u/JIqfGDS+D2v3SyfyAbgzw6aacl3T2cKVCk9ZXI36s w/V6qsXKxTmIWZWjjGZC2xg4r3qopLc= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-104-jIsWE1mUOKaPSv2AsDRI9Q-1; Fri, 17 Apr 2020 11:09:05 -0400 X-MC-Unique: jIsWE1mUOKaPSv2AsDRI9Q-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 03014DBA7 for ; Fri, 17 Apr 2020 15:09:05 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id B185F60BE0 for ; Fri, 17 Apr 2020 15:09:04 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH 11/12] xfs: remove unused iflush stale parameter Date: Fri, 17 Apr 2020 11:08:58 -0400 Message-Id: <20200417150859.14734-12-bfoster@redhat.com> In-Reply-To: <20200417150859.14734-1-bfoster@redhat.com> References: <20200417150859.14734-1-bfoster@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org The stale parameter was used to control the now unused shutdown parameter of xfs_trans_ail_remove(). Signed-off-by: Brian Foster Reviewed-by: Dave Chinner Reviewed-by: Allison Collins --- fs/xfs/xfs_icache.c | 2 +- fs/xfs/xfs_inode.c | 2 +- fs/xfs/xfs_inode_item.c | 7 +++---- fs/xfs/xfs_inode_item.h | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c index a7be7a9e5c1a..9088716465e7 100644 --- a/fs/xfs/xfs_icache.c +++ b/fs/xfs/xfs_icache.c @@ -1118,7 +1118,7 @@ xfs_reclaim_inode( if (XFS_FORCED_SHUTDOWN(ip->i_mount)) { xfs_iunpin_wait(ip); /* xfs_iflush_abort() drops the flush lock */ - xfs_iflush_abort(ip, false); + xfs_iflush_abort(ip); goto reclaim; } if (xfs_ipincount(ip)) { diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index 98ee1b10d1b0..502ffe857b12 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -3701,7 +3701,7 @@ xfs_iflush( return 0; abort: - xfs_iflush_abort(ip, false); + xfs_iflush_abort(ip); shutdown: xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE); return error; diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c index f8dd9bb8c851..b8cbd0c61ce0 100644 --- a/fs/xfs/xfs_inode_item.c +++ b/fs/xfs/xfs_inode_item.c @@ -757,10 +757,9 @@ xfs_iflush_done( */ void xfs_iflush_abort( - xfs_inode_t *ip, - bool stale) + struct xfs_inode *ip) { - xfs_inode_log_item_t *iip = ip->i_itemp; + struct xfs_inode_log_item *iip = ip->i_itemp; if (iip) { xfs_trans_ail_remove(&iip->ili_item); @@ -788,7 +787,7 @@ xfs_istale_done( struct xfs_buf *bp, struct xfs_log_item *lip) { - xfs_iflush_abort(INODE_ITEM(lip)->ili_inode, true); + xfs_iflush_abort(INODE_ITEM(lip)->ili_inode); } /* diff --git a/fs/xfs/xfs_inode_item.h b/fs/xfs/xfs_inode_item.h index 07a60e74c39c..a68c114b79a0 100644 --- a/fs/xfs/xfs_inode_item.h +++ b/fs/xfs/xfs_inode_item.h @@ -34,7 +34,7 @@ extern void xfs_inode_item_init(struct xfs_inode *, struct xfs_mount *); extern void xfs_inode_item_destroy(struct xfs_inode *); extern void xfs_iflush_done(struct xfs_buf *, struct xfs_log_item *); extern void xfs_istale_done(struct xfs_buf *, struct xfs_log_item *); -extern void xfs_iflush_abort(struct xfs_inode *, bool); +extern void xfs_iflush_abort(struct xfs_inode *); extern int xfs_inode_item_format_convert(xfs_log_iovec_t *, struct xfs_inode_log_format *); From patchwork Fri Apr 17 15:08:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 11495389 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8DF401871 for ; Fri, 17 Apr 2020 15:09:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7326321924 for ; Fri, 17 Apr 2020 15:09:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Oen9T4x/" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728446AbgDQPJM (ORCPT ); Fri, 17 Apr 2020 11:09:12 -0400 Received: from us-smtp-delivery-1.mimecast.com ([205.139.110.120]:48362 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728567AbgDQPJK (ORCPT ); Fri, 17 Apr 2020 11:09:10 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587136148; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=sNVK9eLDDV1r6IH4OGj/VF/51UCqF7N6jSNhxVZlvOM=; b=Oen9T4x/nu4Zajn9kajwPBbcPj8aXOzS3nibjlnOOO9Z85bP4+VZY476Wi3KQiNVTYB2KZ UBTS0l45IrPnAzi3PAsIpoXmVNnhEg5focca3ZAtIiuvWzy0HBn539MvjxcL+/xdhpRinV uVUEu+MWt+QUfo3eHYGnWtjPPwsUH04= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-214-GNHGWts2OnCnFeLEFklpLw-1; Fri, 17 Apr 2020 11:09:06 -0400 X-MC-Unique: GNHGWts2OnCnFeLEFklpLw-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6FD128017F3 for ; Fri, 17 Apr 2020 15:09:05 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2998060BE0 for ; Fri, 17 Apr 2020 15:09:05 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH 12/12] xfs: random buffer write failure errortag Date: Fri, 17 Apr 2020 11:08:59 -0400 Message-Id: <20200417150859.14734-13-bfoster@redhat.com> In-Reply-To: <20200417150859.14734-1-bfoster@redhat.com> References: <20200417150859.14734-1-bfoster@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org Introduce an error tag to randomly fail async buffer writes. This is primarily to facilitate testing of the XFS error configuration mechanism. Signed-off-by: Brian Foster Reviewed-by: Allison Collins --- fs/xfs/libxfs/xfs_errortag.h | 4 +++- fs/xfs/xfs_buf.c | 6 ++++++ fs/xfs/xfs_error.c | 3 +++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h index 79e6c4fb1d8a..2486dab19023 100644 --- a/fs/xfs/libxfs/xfs_errortag.h +++ b/fs/xfs/libxfs/xfs_errortag.h @@ -55,7 +55,8 @@ #define XFS_ERRTAG_FORCE_SCRUB_REPAIR 32 #define XFS_ERRTAG_FORCE_SUMMARY_RECALC 33 #define XFS_ERRTAG_IUNLINK_FALLBACK 34 -#define XFS_ERRTAG_MAX 35 +#define XFS_ERRTAG_BUF_IOERROR 35 +#define XFS_ERRTAG_MAX 36 /* * Random factors for above tags, 1 means always, 2 means 1/2 time, etc. @@ -95,5 +96,6 @@ #define XFS_RANDOM_FORCE_SCRUB_REPAIR 1 #define XFS_RANDOM_FORCE_SUMMARY_RECALC 1 #define XFS_RANDOM_IUNLINK_FALLBACK (XFS_RANDOM_DEFAULT/10) +#define XFS_RANDOM_BUF_IOERROR XFS_RANDOM_DEFAULT #endif /* __XFS_ERRORTAG_H_ */ diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index 5120fed06075..a305db779156 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -1289,6 +1289,12 @@ xfs_buf_bio_end_io( struct bio *bio) { struct xfs_buf *bp = (struct xfs_buf *)bio->bi_private; + struct xfs_mount *mp = bp->b_mount; + + if (!bio->bi_status && + (bp->b_flags & XBF_WRITE) && (bp->b_flags & XBF_ASYNC) && + XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BUF_IOERROR)) + bio->bi_status = errno_to_blk_status(-EIO); /* * don't overwrite existing errors - otherwise we can lose errors on diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c index a21e9cc6516a..7f6e20899473 100644 --- a/fs/xfs/xfs_error.c +++ b/fs/xfs/xfs_error.c @@ -53,6 +53,7 @@ static unsigned int xfs_errortag_random_default[] = { XFS_RANDOM_FORCE_SCRUB_REPAIR, XFS_RANDOM_FORCE_SUMMARY_RECALC, XFS_RANDOM_IUNLINK_FALLBACK, + XFS_RANDOM_BUF_IOERROR, }; struct xfs_errortag_attr { @@ -162,6 +163,7 @@ XFS_ERRORTAG_ATTR_RW(buf_lru_ref, XFS_ERRTAG_BUF_LRU_REF); XFS_ERRORTAG_ATTR_RW(force_repair, XFS_ERRTAG_FORCE_SCRUB_REPAIR); XFS_ERRORTAG_ATTR_RW(bad_summary, XFS_ERRTAG_FORCE_SUMMARY_RECALC); XFS_ERRORTAG_ATTR_RW(iunlink_fallback, XFS_ERRTAG_IUNLINK_FALLBACK); +XFS_ERRORTAG_ATTR_RW(buf_ioerror, XFS_ERRTAG_BUF_IOERROR); static struct attribute *xfs_errortag_attrs[] = { XFS_ERRORTAG_ATTR_LIST(noerror), @@ -199,6 +201,7 @@ static struct attribute *xfs_errortag_attrs[] = { XFS_ERRORTAG_ATTR_LIST(force_repair), XFS_ERRORTAG_ATTR_LIST(bad_summary), XFS_ERRORTAG_ATTR_LIST(iunlink_fallback), + XFS_ERRORTAG_ATTR_LIST(buf_ioerror), NULL, };