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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id CF85DC433F5 for ; Fri, 25 Mar 2022 09:29:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5C91A6B0071; Fri, 25 Mar 2022 05:29:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 578D76B0073; Fri, 25 Mar 2022 05:29:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 440A66B0074; Fri, 25 Mar 2022 05:29:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.28]) by kanga.kvack.org (Postfix) with ESMTP id 356296B0071 for ; Fri, 25 Mar 2022 05:29:17 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id E555F21132 for ; Fri, 25 Mar 2022 09:29:16 +0000 (UTC) X-FDA: 79282385112.12.F7FA54E Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf09.hostedemail.com (Postfix) with ESMTP id 3057F140028 for ; Fri, 25 Mar 2022 09:29:16 +0000 (UTC) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 074E6210DD; Fri, 25 Mar 2022 09:29:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1648200555; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=1v1XIVByLQ34phodUlsoo1UGythbajf2AygU4JMpU8Q=; b=Sklm7/D56iGlC3XMsgPJyP2d2O9N9vDUC2PbSZrK6qdK76HLzaksWp4l+6DM0BtT1ry5OA vXPVpQQEZ1yxgQr9dA5ppZMc/eiHLpdd1d9mbhvnargW+E+TqA7HavnxHa3PWXOtgzdTLG phd3ap9sP7d5Beau6Me024fWBwRKtxw= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1648200555; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=1v1XIVByLQ34phodUlsoo1UGythbajf2AygU4JMpU8Q=; b=OPZfoFocy0vcoPOSn/AIyId4zzrcpNq9VQA0zPtlQR51tJ/hIxE3BEiFdS4yEzr2xxlkZM hwpZO01LFwJVKrCw== Received: from quack3.suse.cz (unknown [10.100.200.198]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id E800FA3B82; Fri, 25 Mar 2022 09:29:14 +0000 (UTC) Received: by quack3.suse.cz (Postfix, from userid 1000) id 922AEA0610; Fri, 25 Mar 2022 10:29:11 +0100 (CET) Date: Fri, 25 Mar 2022 10:29:11 +0100 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , "khazhy@google.com" , "linux-mm@kvack.org" , linux-fsdevel Subject: Re: [PATCH RFC] nfsd: avoid recursive locking through fsnotify Message-ID: <20220325092911.fnttlyrvw7qzggc7@quack3.lan> References: <20220321145111.qz3bngofoi5r5cmh@quack3.lan> <20220323104129.k4djfxtjwdgoz3ci@quack3.lan> <20220323134851.px6s4i6iiaj4zlju@quack3.lan> <20220323142835.epitipiq7zc55vgb@quack3.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: 8mgrdyjsbpwj3oodbaws7b1qb8ju7o81 Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b="Sklm7/D5"; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=OPZfoFoc; dmarc=none; spf=pass (imf09.hostedemail.com: domain of jack@suse.cz designates 195.135.220.28 as permitted sender) smtp.mailfrom=jack@suse.cz X-Rspamd-Queue-Id: 3057F140028 X-HE-Tag: 1648200556-273839 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 Thu 24-03-22 21:17:09, Amir Goldstein wrote: > On Wed, Mar 23, 2022 at 5:46 PM Amir Goldstein wrote: > > > > On Wed, Mar 23, 2022 at 4:28 PM Jan Kara wrote: > > > > > > On Wed 23-03-22 16:00:30, Amir Goldstein wrote: > > > > > Well, but reclaim from kswapd is always the main and preferred source of > > > > > memory reclaim. And we will kick kswapd to do work if we are running out of > > > > > memory. Doing direct filesystem slab reclaim from mark allocation is useful > > > > > only to throttle possibly aggressive mark allocations to the speed of > > > > > reclaim (instead of getting ENOMEM). So I'm still not convinced this is a > > > > > big issue but I certainly won't stop you from implementing more fine > > > > > grained GFP mode selection and lockdep annotations if you want to go that > > > > > way :). > > > > > > > > Well it was just two lines of code to annotate the fanotify mutex as its own > > > > class, so I just did that: > > > > > > > > https://github.com/amir73il/linux/commit/7b4b6e2c0bd1942cd130e9202c4b187a8fb468c6 > > > > > > But this implicitely assumes there isn't any allocation under mark_mutex > > > anywhere else where it is held. Which is likely true (I didn't check) but > > > it is kind of fragile. So I was rather imagining we would have per-group > > > "NOFS" flag and fsnotify_group_lock/unlock() would call > > > memalloc_nofs_save() based on the flag. And we would use > > > fsnotify_group_lock/unlock() uniformly across the whole fsnotify codebase. > > > > > > > I see what you mean, but looking at the code it seems quite a bit of churn to go > > over all the old backends and convert the locks to use wrappers where we "know" > > those backends are fs reclaim safe (because we did not get reports of deadlocks > > over the decades they existed). > > > > I think I will sleep better with a conversion to three flavors: > > > > 1. pflags = fsnotify_group_nofs_lock(fanotify_group); > > 2. fsnotify_group_lock(dnotify_group) => > > WARN_ON_ONCE(group->flags & FSNOTIFY_NOFS) > > mutex_lock(&group->mark_mutex) > > 3. fsnotify_group_lock_nested(group) => > > mutex_lock_nested(&group->mark_mutex, SINGLE_DEPTH_NESTING) > > > > I think I might have misunderstood you and you meant that the > SINGLE_DEPTH_NESTING subcalls should be eliminated and then > we are left with two lock classes. > Correct? Yeah, at least at this point I don't see a good reason for using SINGLE_DEPTH_NESTING lockdep annotation. In my opinion it has just a potential of silencing reports of real locking problems. So removing it and seeing what complains would be IMO a way to go. Honza -- Jan Kara SUSE Labs, CR