From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6B48CCD13D1 for ; Mon, 18 Sep 2023 03:46:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233256AbjIRDpk (ORCPT ); Sun, 17 Sep 2023 23:45:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55354 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236717AbjIRDpG (ORCPT ); Sun, 17 Sep 2023 23:45:06 -0400 Received: from mxhk.zte.com.cn (mxhk.zte.com.cn [63.216.63.40]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 388BE10A; Sun, 17 Sep 2023 20:44:59 -0700 (PDT) Received: from mse-fl2.zte.com.cn (unknown [10.5.228.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mxhk.zte.com.cn (FangMail) with ESMTPS id 4RprK34GWKz8XrRF; Mon, 18 Sep 2023 11:44:55 +0800 (CST) Received: from szxlzmapp01.zte.com.cn ([10.5.231.85]) by mse-fl2.zte.com.cn with SMTP id 38I3iqSr028467; Mon, 18 Sep 2023 11:44:52 +0800 (+08) (envelope-from cheng.lin130@zte.com.cn) Received: from mapi (szxlzmapp06[null]) by mapi (Zmail) with MAPI id mid14; Mon, 18 Sep 2023 11:44:53 +0800 (CST) Date: Mon, 18 Sep 2023 11:44:53 +0800 (CST) X-Zmail-TransId: 2b086507c7b56cf-2fc22 X-Mailer: Zmail v1.0 Message-ID: <202309181144537682244@zte.com.cn> In-Reply-To: References: ZQJIyx419cw24ppF@dread.disaster.area,202309151750563356840@zte.com.cn,ZQeBY3kmww8qAjfP@dread.disaster.area Mime-Version: 1.0 From: To: Cc: , , , , , Subject: =?UTF-8?B?UmU6IFtQQVRDSCB2M10geGZzOiBpbnRyb2R1Y2UgcHJvdGVjdGlvbiBmb3IgZHJvcCBubGluaw==?= Content-Type: text/plain; charset="UTF-8" X-MAIL: mse-fl2.zte.com.cn 38I3iqSr028467 X-Fangmail-Gw-Spam-Type: 0 X-Fangmail-Anti-Spam-Filtered: true X-Fangmail-MID-QID: 6507C7B7.001/4RprK34GWKz8XrRF Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On Fri, Sep 15, 2023 at 05:50:56PM +0800, cheng.lin130@zte.com.cn wrote: > > > On Wed, Sep 13, 2023 at 05:44:45PM +0800, cheng.lin130@zte.com.cn wrote: > > > > From: Cheng Lin > > > > > > > > When abnormal drop_nlink are detected on the inode, > > > > shutdown filesystem, to avoid corruption propagation. > > > > > > > > Signed-off-by: Cheng Lin > > > > --- > > > > fs/xfs/xfs_inode.c | 9 +++++++++ > > > > 1 file changed, 9 insertions(+) > > > > > > > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > > > > index 9e62cc500..40cc106ae 100644 > > > > --- a/fs/xfs/xfs_inode.c > > > > +++ b/fs/xfs/xfs_inode.c > > > > @@ -919,6 +919,15 @@ xfs_droplink( > > > > xfs_trans_t *tp, > > > > xfs_inode_t *ip) > > > > { > > > > + > > > > + if (VFS_I(ip)->i_nlink == 0) { > > > > + xfs_alert(ip->i_mount, > > > > + "%s: Deleting inode %llu with no links.", > > > > + __func__, ip->i_ino); > > > > + tp->t_flags |= XFS_TRANS_DIRTY; > > > Marking the transaction dirty is not necessary. > > > Otherwise this seems fine. > > Another strategy: > > Set nlink to an invalid value(like XFS_NLINK_PINNED), and > > Complete this transaction before shutdown fs. To make sure > > nlink not be zero. If the nlink of a directory are zero, it may > > be cleaned up. > > Is that appropriate? > No, all I'm asking you to do is drop dirtying of the transaction > from this patch because it is a) unnecessary and b) a layering > violation. > It is unnecessary because the transaction will almost always be > dirty before we get to xfs_droplink(). In the cases where it isn't > dirty (e.g. xfs_remove() on a directory) we explicitly check that > nlink == 2 before proceeding to call xfs_droplink(). Hence we can't > actually get to xfs_droplink() with a clean transaction, and so If the corrupted inode is a parent directory, when remove its subdirectory, the parent's nlink will be decreased to 0. But the transaction of subdirectory removing is not dirty (There are not check about the parent directory). In this situation, the transaction will be failed and the filesystem will be alive. > marking it dirty here on underrun is unnecessary as returning an > error from xfs_droplink() will result in shutting down the > filesystem when the transaction is cancelled. > -Dave. > -- > Dave Chinner > david@fromorbit.com