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 X-Spam-Level: X-Spam-Status: No, score=-2.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 67055C432C0 for ; Mon, 18 Nov 2019 01:00:52 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 2A3B220730 for ; Mon, 18 Nov 2019 01:00:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2A3B220730 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=fromorbit.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id A74FA6B0007; Sun, 17 Nov 2019 20:00:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A25426B0008; Sun, 17 Nov 2019 20:00:51 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 93B316B000A; Sun, 17 Nov 2019 20:00:51 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0211.hostedemail.com [216.40.44.211]) by kanga.kvack.org (Postfix) with ESMTP id 7E0BE6B0007 for ; Sun, 17 Nov 2019 20:00:51 -0500 (EST) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with SMTP id 440258249980 for ; Mon, 18 Nov 2019 01:00:51 +0000 (UTC) X-FDA: 76167593502.08.men40_7bcbbc866e50a X-HE-Tag: men40_7bcbbc866e50a X-Filterd-Recvd-Size: 3543 Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by imf23.hostedemail.com (Postfix) with ESMTP for ; Mon, 18 Nov 2019 01:00:50 +0000 (UTC) Received: from dread.disaster.area (pa49-181-255-80.pa.nsw.optusnet.com.au [49.181.255.80]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 333503A1F83; Mon, 18 Nov 2019 12:00:49 +1100 (AEDT) Received: from dave by dread.disaster.area with local (Exim 4.92.3) (envelope-from ) id 1iWVPL-0004U1-Vm; Mon, 18 Nov 2019 12:00:47 +1100 Date: Mon, 18 Nov 2019 12:00:47 +1100 From: Dave Chinner To: Brian Foster Cc: linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 28/28] xfs: rework unreferenced inode lookups Message-ID: <20191118010047.GS4614@dread.disaster.area> References: <20191031234618.15403-1-david@fromorbit.com> <20191031234618.15403-29-david@fromorbit.com> <20191106221846.GE37080@bfoster> <20191114221602.GJ4614@dread.disaster.area> <20191115172600.GC55854@bfoster> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191115172600.GC55854@bfoster> User-Agent: Mutt/1.10.1 (2018-07-13) X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=G6BsK5s5 c=1 sm=1 tr=0 a=XqaD5fcB6dAc7xyKljs8OA==:117 a=XqaD5fcB6dAc7xyKljs8OA==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=kj9zAlcOel0A:10 a=MeAgGD-zjQ4A:10 a=7-415B0cAAAA:8 a=ETrtCZpq2FqVuZsvotgA:9 a=CjuIK1q_8ugA:10 a=biEYGPWJfzWAr4FL6Ov7:22 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Nov 15, 2019 at 12:26:00PM -0500, Brian Foster wrote: > On Fri, Nov 15, 2019 at 09:16:02AM +1100, Dave Chinner wrote: > > On Wed, Nov 06, 2019 at 05:18:46PM -0500, Brian Foster wrote: > > If so, most of this patch will go away.... > > > > > > + * attached to the buffer so we don't need to do anything more here. > > > > */ > > > > - if (ip != free_ip) { > > > > - if (!xfs_ilock_nowait(ip, XFS_ILOCK_EXCL)) { > > > > - rcu_read_unlock(); > > > > - delay(1); > > > > - goto retry; > > > > - } > > > > - > > > > - /* > > > > - * Check the inode number again in case we're racing with > > > > - * freeing in xfs_reclaim_inode(). See the comments in that > > > > - * function for more information as to why the initial check is > > > > - * not sufficient. > > > > - */ > > > > - if (ip->i_ino != inum) { > > > > + if (__xfs_iflags_test(ip, XFS_ISTALE)) { > > > > > > Is there a correctness reason for why we move the stale check to under > > > ilock (in both iflush/ifree)? > > > > It's under the i_flags_lock, and so I moved it up under the lookup > > hold of the i_flags_lock so we don't need to cycle it again. > > > > Yeah, but in both cases it looks like it moved to under the ilock as > well, which comes after i_flags_lock. IOW, why grab ilock for stale > inodes when we're just going to skip them? Because I was worrying about serialising against reclaim before changing the state of the inode. i.e. if the inode has already been isolated by not yet disposed of, we shouldn't touch the inode state at all. Serialisation against reclaim in this patch is via the ILOCK, hence we need to do that before setting ISTALE.... IOWs, ISTALE is not protected by ILOCK, we just can't modify the inode state until after we've gained the ILOCK to protect against reclaim.... Cheers, Dave. -- Dave Chinner david@fromorbit.com