From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DB7453BB665 for ; Tue, 7 Jul 2026 09:53:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783418017; cv=none; b=B24FV3poldPtJXGf0NCE/IUzhKmh21zhDClFgMB0UvIMdJGt2QVIh4zcjJiZxpJ9gE7kaDMz8zeTZ/NwWJWt7myDAsuW7hx0/jtr5BTcHOt0G37gRPxYiPEBfEKr24lGQ0tCUr8XX/qKrkaSEe3F5ki5r/tLmOr6qUieLMr2eu8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783418017; c=relaxed/simple; bh=29aEOrnU2ORtuyaY8j0gfYWt24xpSiXIbPKYfgrotaE=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=EpGQlqTMrVq1bir8HVeYn+AIUL+tH8aKB6ZDv8DgFeZjjcXEER62d8czHlz9nvKUJHLkzn4OPyrtDItCi/0DFtqiPLyWBGJ/Vn/wz9gmO2nnYAOCDhYOEbK5Jimdo6pzQQRxbvw/BU/50C1yx3LB3tNeSps97+o1B5Zan3ZgmRg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=agA2/Igz; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="agA2/Igz" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6E01A1F000E9; Tue, 7 Jul 2026 09:53:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1783418015; bh=heO7BCatoIHW6vgfHyqZvrqbsWWERDnfXAzWaFZqjPM=; h=Date:Cc:Subject:To:References:From:In-Reply-To; b=agA2/Igz9nEg7NQodp9V04Ggr7vSozVqNWzY+JQtZKPeEZGNmzk7hYyKno+zHIX/m axejXpnzRvoCWPADGb8dWA1Kk1VpwyzYH06SkuYYWnLT7tOKGmbjR++e5BHNo+dNg2 N8cbHl53XDtuijXO3Jq6kSFkG7WrJiEY7QpE7GlAEK7kqE7n9cG3o19JC7lWfq6651 rTmWFB6FUhvz5BQ9m3cg1oZFL2og7AZSmzi92kvqxgoS/i1X8enm/e6bGTQ0eB/bhu Ce5TIVtbt5FueY0E1dWcdU9O/V8m9Bdl59MBS+IZAarUjPGL9lEn9a+dMwsjQxvtoT 1sEqO14UnOLsA== Message-ID: Date: Tue, 7 Jul 2026 17:53:32 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: chao@kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] f2fs: quota: use memalloc_nofs_{save,restore} instead of FGP_NOFS To: Matthew Wilcox , jaegeuk@kernel.org References: <20260706095943.2560208-1-chao@kernel.org> <20260706095943.2560208-2-chao@kernel.org> Content-Language: en-US From: Chao Yu In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 7/7/26 17:44, Chao Yu wrote: > On 7/6/26 22:18, Matthew Wilcox wrote: >> On Mon, Jul 06, 2026 at 05:59:43PM +0800, Chao Yu wrote: >>> FGP_NOFS could be removed later, let's use memalloc_nofs_{save,restore} >>> instead, which is recommended to be used to avoid potential deadlock >>> when memory allocation in f2fs_quota_write() will call into filesystem >>> interface again, e.g. .writepages, evict_inode, shrinker due to >>> complicated lock race condition. >> >> I think we need to be clear on why we need the memalloc_nofs_save() >> call here. What problem would it cause if we did call into the >> filesystem to reclaim memory? > > Hmm, after taking another look at the code, I suspect below deadlock may > occur w/o NOFS flag? > > User Sync (Thread 1) > - f2fs_do_quota_sync > - f2fs_lock_op > : down_read(cp_rwsem) [HELD] (1st Read Lock) > - f2fs_quota_sync_file > - dquot_writeback_dquots > - ... Oh, seems commit 537e11cdc7a6 ("quota: Prevent memory allocation recursion while holding dq_lock") covers write path (f2fs_quota_write) as well, please ignore this. Thanks, > - f2fs_quota_write > - f2fs_write_begin > - f2fs_filemap_get_folio > - __filemap_get_folio > - ... (GFP_FS allocation triggers reclaim) > - try_to_free_pages() > - shrink_node() > - shrink_lruvec() > - shrink_page_list() > - pageout() <-- Reclaim decides to write back a dirty F2FS data page > - f2fs_writepages() > - f2fs_write_cache_pages() > - f2fs_write_single_data_page(allow_balance = true) > - f2fs_balance_fs(need = true) > - f2fs_gc() > - f2fs_write_checkpoint() > - block_operations() > - f2fs_lock_all() > : down_write(cp_rwsem) [BLOCKED] (Self-deadlock) > > To Jaegeuk, please help to double check this. :) > >> >> I suspect this is the wrong place to insert this call and it should be >> near the lock that causes the problem. I've attempted a rewrite of the >> memalloc_nofs_save documentation; let me know what you think: >> >> /** >> * memalloc_nofs_save - Prevent recursion into the filesystem. >> * >> * All memory allocations between calling this function and calling >> * memalloc_nofs_restore() will be prevented from calling into filesystems >> * to reclaim memory. Clean page cache memory can still be reclaimed, >> * but (for example) inodes will not be. >> * >> * The primary reason to do this is that the caller has taken a lock >> * which would be needed by FS reclaim. While we could theoretically >> * call into a different filesystem in this case, it can be a deep call >> * stack so it is better to avoid all filesystems. >> * >> * Filesystems often choose to incorporate a call to this function as part >> * of starting a journal transaction. While not a lock in the normal >> * sense, it has much the same effect as nested journal transactions >> * are either prohibited or expensive. >> * >> * Also call this function if you need to allocate memory while holding >> * a file folio locked. High order allocations (such as those requested >> * by slab) can trigger compaction which will attempt to lock the folio. >> * >> * Context: This function is safe to be used from any context. >> * Return: The saved flags to be passed to memalloc_nofs_restore. >> */ > > It makes sense to me, thanks for updating. > > Thanks,