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 D5F15C47404 for ; Fri, 11 Oct 2019 23:38:32 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 8D2A62084C for ; Fri, 11 Oct 2019 23:38:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8D2A62084C 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 35FA66B0003; Fri, 11 Oct 2019 19:38:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 310386B0005; Fri, 11 Oct 2019 19:38:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 225568E0005; Fri, 11 Oct 2019 19:38:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0234.hostedemail.com [216.40.44.234]) by kanga.kvack.org (Postfix) with ESMTP id EF2306B0003 for ; Fri, 11 Oct 2019 19:38:31 -0400 (EDT) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with SMTP id 962216132 for ; Fri, 11 Oct 2019 23:38:31 +0000 (UTC) X-FDA: 76033120422.17.owner98_1023f6dc56b0b X-HE-Tag: owner98_1023f6dc56b0b X-Filterd-Recvd-Size: 4681 Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by imf02.hostedemail.com (Postfix) with ESMTP for ; Fri, 11 Oct 2019 23:38:30 +0000 (UTC) Received: from dread.disaster.area (pa49-181-198-88.pa.nsw.optusnet.com.au [49.181.198.88]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 11FAC3639EB; Sat, 12 Oct 2019 10:38:29 +1100 (AEDT) Received: from dave by dread.disaster.area with local (Exim 4.92.2) (envelope-from ) id 1iJ4UO-0007v4-0D; Sat, 12 Oct 2019 10:38:28 +1100 Date: Sat, 12 Oct 2019 10:38:27 +1100 From: Dave Chinner To: Christoph Hellwig Cc: linux-xfs@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, Peter Zijlstra , Ingo Molnar , Will Deacon , "Paul E. McKenney" Subject: Re: [PATCH 25/26] xfs: rework unreferenced inode lookups Message-ID: <20191011233827.GP16973@dread.disaster.area> References: <20191009032124.10541-1-david@fromorbit.com> <20191009032124.10541-26-david@fromorbit.com> <20191011125522.GA13167@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20191011125522.GA13167@infradead.org> User-Agent: Mutt/1.10.1 (2018-07-13) X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=P6RKvmIu c=1 sm=1 tr=0 a=ocld+OpnWJCUTqzFQA3oTA==:117 a=ocld+OpnWJCUTqzFQA3oTA==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=IkcTkHD0fZMA:10 a=XobE76Q3jBoA:10 a=7-415B0cAAAA:8 a=9fGYlBlKfAZRPwy0aZMA:9 a=QEXdDO2ut3YA:10 a=biEYGPWJfzWAr4FL6Ov7:22 Content-Transfer-Encoding: quoted-printable 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, Oct 11, 2019 at 05:55:22AM -0700, Christoph Hellwig wrote: > On Wed, Oct 09, 2019 at 02:21:23PM +1100, Dave Chinner wrote: > > 4. it xfs_ilock_nowait() fails until the rcu grace period >=20 > Should this be: >=20 > > 4. if xfs_ilock_nowait() fails before the rcu grace period >=20 > ? >=20 > > + xfs_iunlock(ip, XFS_ILOCK_EXCL); > > kmem_zone_free(xfs_inode_zone, ip); > > } > > =20 > > @@ -131,6 +132,7 @@ xfs_inode_free( > > * free state. The ip->i_flags_lock provides the barrier against lo= okup > > * races. > > */ > > + xfs_ilock(ip, XFS_ILOCK_EXCL); >=20 > This introduce=D1=95 a non-owner unlock of an exclusively held rwsem. = As-is > this will make lockdep very unhappy. We have a non-owner unlock versio= n > of up_read, but not of up_write currently. I'm also not sure if those > are allowed from RCU callback, which IIRC can run from softirq context. >=20 > That being said this scheme of only unlocking the inode in the rcu free > callback makes totaly sense to me, so I wish we can accomodate it > somehow. AFAICT it is safe to do this. Lockdep just needs to be bashed about the head a bit to make it shut up. > > @@ -312,7 +327,8 @@ xfs_iget_cache_hit( > > rcu_read_lock(); > > spin_lock(&ip->i_flags_lock); > > wake =3D !!__xfs_iflags_test(ip, XFS_INEW); > > - ip->i_flags &=3D ~(XFS_INEW | XFS_IRECLAIM); > > + ip->i_flags &=3D ~XFS_INEW | XFS_IRECLAIM; >=20 > This change looks wrong to me, or did I miss something? We now > clear all bits that are not XFS_I_NEW and XFS_IRECLAIM, which > already is set in ~XFS_INEW. So if that was the intent just: >=20 > ip->i_flags &=3D ~XFS_INEW; Nah, I screwed up backing out a change. This line should not be modified at all. >=20 > > + * This requires code that requires such pins to do the following un= der a single >=20 > This adds an > 80 char line. (there are a few more below. >=20 > > + /* push the AIL to clean dirty reclaimable inodes */ > > + xfs_ail_push_all(mp->m_ail); > > + > > + /* push the AIL to clean dirty reclaimable inodes */ > > + xfs_ail_push_all(mp->m_ail); > > + >=20 > This looks spurious vs the rest of the patch. Looks like rebase failure fallout. I must have missed it on cleanup. I'll sort that out. > > + if (__xfs_iflags_test(ip, XFS_ISTALE)) { > > + spin_unlock(&ip->i_flags_lock); > > + if (ip !=3D free_ip) > > xfs_iunlock(ip, XFS_ILOCK_EXCL); > > - } > > + rcu_read_unlock(); > > + continue; >=20 > This unlock out of order. Should be harmless, but also pointless. >=20 > I think this code would be a lot easier to understand if we fatored > this inner loop into a new helper. Untested patch that does, and > also removes a no incorrect comment below: *nod* I'll put a refacting patch at the start of the series to split this into separate code movement and algorithm modification patches.... Cheers, Dave. --=20 Dave Chinner david@fromorbit.com