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=-3.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS 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 CE7CAC433DF for ; Mon, 15 Jun 2020 15:09:30 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 73EF42063A for ; Mon, 15 Jun 2020 15:09:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="ZdV7+Hej" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 73EF42063A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id BFE7B6B000D; Mon, 15 Jun 2020 11:09:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BAF5B6B000E; Mon, 15 Jun 2020 11:09:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AC4676B0010; Mon, 15 Jun 2020 11:09:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0225.hostedemail.com [216.40.44.225]) by kanga.kvack.org (Postfix) with ESMTP id 9256E6B000D for ; Mon, 15 Jun 2020 11:09:29 -0400 (EDT) Received: from smtpin14.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 4F5B7184D4221 for ; Mon, 15 Jun 2020 15:09:29 +0000 (UTC) X-FDA: 76931780058.14.blade25_4508be626df7 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin14.hostedemail.com (Postfix) with ESMTP id D9CE91814963A for ; Mon, 15 Jun 2020 15:07:47 +0000 (UTC) X-HE-Tag: blade25_4508be626df7 X-Filterd-Recvd-Size: 4655 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by imf42.hostedemail.com (Postfix) with ESMTP for ; Mon, 15 Jun 2020 15:07:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Transfer-Encoding :Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=D9f5Mi6L2QV4D+1O3yqVfrh3RLhrtNFUT6np/JE/KSY=; b=ZdV7+HejkWrYxGvOxYruUhoeUu oxY5bTcqQ3cKMz5uGbWWbBUqjD+vFfUMpFhw6r3k4PqjLvJkQ/t9Nswq4pybAJOLQnGWvZJLcjLRu pd2wJHkzxgSBbbH9zCzPZ5AWeFITpIA6CpCEZNYBdffHotj+ReUvVg+gLJcTL4McaB1cQqoKhlJ3t qv+SvrjtNRooNnkVU6azlaotQh+IOhJMvF0RAk/TAnPIpdnlJILXZyHXO58geBTnoWNcu7ZJtA44w hGXk2ZbzhVXcrQu0M+DGDJEPTLd+JQs6wSvwxojTxnKLXdQgBddd+1Y/KUJqYUejFmFAOLnXHaDiX CMIGrjfQ==; Received: from willy by bombadil.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1jkqi0-0003AR-Ae; Mon, 15 Jun 2020 15:07:36 +0000 Date: Mon, 15 Jun 2020 08:07:36 -0700 From: Matthew Wilcox To: Michal Hocko Cc: Holger =?iso-8859-1?Q?Hoffst=E4tte?= , Yafang Shao , linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v3] xfs: avoid deadlock when trigger memory reclaim in ->writepages Message-ID: <20200615150736.GU8681@bombadil.infradead.org> References: <1592222181-9832-1-git-send-email-laoar.shao@gmail.com> <20200615145331.GK25296@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20200615145331.GK25296@dhcp22.suse.cz> X-Rspamd-Queue-Id: D9CE91814963A X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam05 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 Mon, Jun 15, 2020 at 04:53:31PM +0200, Michal Hocko wrote: > On Mon 15-06-20 16:25:52, Holger Hoffst=E4tte wrote: > > On 2020-06-15 13:56, Yafang Shao wrote: > [...] > > > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > > > index b356118..1ccfbf2 100644 > > > --- a/fs/xfs/xfs_aops.c > > > +++ b/fs/xfs/xfs_aops.c > > > @@ -573,9 +573,21 @@ static inline bool xfs_ioend_needs_workqueue(s= truct iomap_ioend *ioend) > > > struct writeback_control *wbc) > > > { > > > struct xfs_writepage_ctx wpc =3D { }; > > > + unsigned int nofs_flag; > > > + int ret; > > > xfs_iflags_clear(XFS_I(mapping->host), XFS_ITRUNCATED); > > > - return iomap_writepages(mapping, wbc, &wpc.ctx, &xfs_writeback_op= s); > > > + > > > + /* > > > + * We can allocate memory here while doing writeback on behalf of > > > + * memory reclaim. To avoid memory allocation deadlocks set the > > > + * task-wide nofs context for the following operations. > > > + */ > > > + nofs_flag =3D memalloc_nofs_save(); > > > + ret =3D iomap_writepages(mapping, wbc, &wpc.ctx, &xfs_writeback_o= ps); > > > + memalloc_nofs_restore(nofs_flag); > > > + > > > + return ret; > > > } > > > STATIC int > > >=20 > >=20 > > Not sure if I did something wrong, but while the previous version of = this patch > > worked fine, this one gave me (with v2 removed obviously): > >=20 > > [ +0.000004] WARNING: CPU: 0 PID: 2811 at fs/iomap/buffered-io.c:154= 4 iomap_do_writepage+0x6b4/0x780 >=20 > This corresponds to > /* > * Given that we do not allow direct reclaim to call us, we sho= uld > * never be called in a recursive filesystem reclaim context. > */ > if (WARN_ON_ONCE(current->flags & PF_MEMALLOC_NOFS)) > goto redirty; >=20 > which effectivelly says that memalloc_nofs_save/restore cannot be used > for that code path. Your stack trace doesn't point to a reclaim path > which shows that this path is shared and also underlines that this is > not really an intended use of the api. Please refer to > Documentation/core-api/gfp_mask-from-fs-io.rst for more details but > shortly the API should be used at the layer which defines a context > which shouldn't allow to recurse. E.g. a lock which would be problemati= c > in the reclaim recursion path. I'm concerned that this might not be the correct approach. Generally we have the rule that freeing memory should not require allocating memory to succeed. Since XFS obviously does need to allocate memory to start a transaction, this allocation should normally be protected by a mempool or similar. XFS is subtle and has its own rules around memory allocation and writeback, so I don't want to say this is definitely wrong. I'm just saying that I have concerns it might not be right.