From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yx1-f47.google.com (mail-yx1-f47.google.com [74.125.224.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9D5881C695 for ; Sun, 23 Nov 2025 03:43:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.224.47 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763869389; cv=none; b=sRZGkVpzxzleiOPo8uvLgCLZXem+jE9LA4vKftVQ14mqnjodwXLol1jipK50161SeySkW6NPEN3nTLLuZILbVES5I3hwElVcdOky4Rqp3QloM91rgo6rTGGx+l95qJKZB/ZTA99o2QOe80rBRYuEacCK+wC4jfQbJrZWXPWdtPU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763869389; c=relaxed/simple; bh=jq5SwSviumWqzK5n27iNBmkxVFf0Q0caHXk3Cf4lmBY=; h=Date:From:To:cc:Subject:In-Reply-To:Message-ID:References: MIME-Version:Content-Type; b=Iws4iJccvZECM+dvWtfFVKMMckZFJVV2sSIIHqUkt7OUehyE7cF8QC698kJIaShK+VmbM8PZAHHKv3EtEYUcDztuioC6i3oYdtJeDdU7ZPSVFgbErNps+ZaClCorfSUf+2ET2p4Dnl+GQX6bBXaPwMMtaiwwQ6tksEY+vOwDI+A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=JNYdXf/x; arc=none smtp.client-ip=74.125.224.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="JNYdXf/x" Received: by mail-yx1-f47.google.com with SMTP id 956f58d0204a3-640c9c85255so4316135d50.3 for ; Sat, 22 Nov 2025 19:43:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1763869386; x=1764474186; darn=vger.kernel.org; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=atGl1KFG9w/JBAL4HGSSXZEw0hkrZacc8aGE0xS3SqE=; b=JNYdXf/xesKkLDKxSjQF+yC49T++8esJVwB8jZeGJZK9jYvK2PhIkVhzxP2c7hxQtT 5EzRBsCtExsU7NPHINlEcNL3WGolTzPgnwkCkfv+rY9oimbLdcRx5MqPkdQbMFuEJdBg 6Aw+cXnsCz5n1ap95NDlUuCxlxvAD/8HkaT1KhH0Hwn35kcn8QRz3Pypm+mSJX8IwQTg e2mTWbqS2f8ijytmH34va3FLkcCAx8hcbYwNp0yuFia3SJL6Vu5uW/Jqn9XnXaurvj1l 0nc16Nfo0H/lpFXzwUDeZ9SrdXgJQVWfraJc/CdYafqv+j1N+0Y4UYSvQo5PlnylINdV 5CXw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763869386; x=1764474186; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=atGl1KFG9w/JBAL4HGSSXZEw0hkrZacc8aGE0xS3SqE=; b=i6kH81Dwqm1RyhuAKBPaacQoPKTdRKi006Bnd/81SbRAPt8Ht5oleRuzWMEHKhjps+ Qp80X042+JBgihT4FrSsosEr+rE9h3gXB5/1MmU3834qn5G2gNCyYDiVsifOWtkvj4ll sBd98PoSZZyK3stj5kLPDA4NZ1APUNKdt68VhY1VjPubWgC/4MGkngdP2nOi1FW3Nz2/ 8dGH9zBdx06fmsPjDWG/Vt8og1SLz2vka5gZuEZXjsi75I2H1/J1PFTwyD1eB9S5CWCT /CabyD3OoJgyfspZ4EzRVT5rMi4A/m7yyH6stODY6uUdlenu/tlnpa/18P+2s6FWmKMW MN4g== X-Forwarded-Encrypted: i=1; AJvYcCWlTZJGrUMTutuUQeNRP+xxmEn1bhT4SqSkjVw3hrHd9TH4EK0/2wxr+V85ig+wwE9LCz4TLTolUU+WtUc=@vger.kernel.org X-Gm-Message-State: AOJu0YzUQBAoiNq3+1a5r4bC7rqiMZPdJ/6+i/VP1VtvmSfZwzL67+To I31/c+ue4VxHkoRJB/xmLjcIcwCk1sm7TMELde1C5ion8z9p3CmN1BXWIOiadrtplQ== X-Gm-Gg: ASbGncsWvAlLakdxUTEoy15n+jn4qrBWPCkh6FM8h5+dY/r8EXIPbnuG74mec2aJ8ym 9XcMzphSOoXk4N/19KW/7+nu8IG5EwIQ2b8IZU90z0+7N4CLr6xr86c6a/OtyEgwuuhAZe/lN72 e/+I0SOuPWFQpmsUp94pozdbPaSMwQQqnilZNLMotf31AdEl3Pkq2sTOJuqigHgcPbnXiqh3WGd mEyoQPMXGYTI2ZXUeyPqxNdR4MdcN6AtaWGrE20f9bDt8xAZSg3zQPzaPmRD/SeFDdQ5raqleGW o20P8Q8FDLedTNQ0UHcjcBsct68TvA8RGvXqfwUwEDMf00Hc93N9wuagEtstZa29o8B/KhI/Ljm gv0n80co5UmWylQYFoWi8QZ11CLfD2ZqJY4cI6ukw5zrPQiMYxO01tf5aybZH4PpNmYGNgkdQVq 2nSBAwP+gHIJlnqSARs7uFlKXnrPY7R2KJuTXpMUhPHS9vynA+yBWvhCxhOqAwwuF+GAZwNzc= X-Google-Smtp-Source: AGHT+IGxGBlznVg0SJDf4cRndAQ7Zg95tSoW1YUzecdZ7j9X7Q+uJQzQQ08yWuk+41zaU5qKqnuE3g== X-Received: by 2002:a05:690c:4a05:b0:786:57f5:b49a with SMTP id 00721157ae682-78a8b4d4018mr62168257b3.29.1763869386403; Sat, 22 Nov 2025 19:43:06 -0800 (PST) Received: from darker.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id 00721157ae682-78a79953584sm30955797b3.49.2025.11.22.19.43.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 22 Nov 2025 19:43:05 -0800 (PST) Date: Sat, 22 Nov 2025 19:42:51 -0800 (PST) From: Hugh Dickins To: Christoph Hellwig , Vlastimil Babka cc: Andrew Morton , Christoph Lameter , David Rientjes , Roman Gushchin , Harry Yoo , Suren Baghdasaryan , Michal Hocko , Brendan Jackman , Zi Yan , Eric Biggers , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 06/11] mempool: factor out a mempool_alloc_from_pool helper In-Reply-To: <20251113084022.1255121-7-hch@lst.de> Message-ID: References: <20251113084022.1255121-1-hch@lst.de> <20251113084022.1255121-7-hch@lst.de> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII On Thu, 13 Nov 2025, Christoph Hellwig wrote: > Add a helper for the mempool_alloc slowpath to better separate it from the > fast path, and also use it to implement mempool_alloc_preallocated which > shares the same logic. > > Signed-off-by: Christoph Hellwig > --- ... > @@ -413,8 +457,6 @@ void *mempool_alloc_noprof(mempool_t *pool, gfp_t gfp_mask) > { > gfp_t gfp_temp = mempool_adjust_gfp(&gfp_mask); > void *element; > - unsigned long flags; > - wait_queue_entry_t wait; > > VM_WARN_ON_ONCE(gfp_mask & __GFP_ZERO); > might_alloc(gfp_mask); > @@ -428,53 +470,22 @@ void *mempool_alloc_noprof(mempool_t *pool, gfp_t gfp_mask) > element = pool->alloc(gfp_temp, pool->pool_data); > } > > - if (likely(element)) > - return element; > - > - spin_lock_irqsave(&pool->lock, flags); > - if (likely(pool->curr_nr)) { > - element = remove_element(pool); > - spin_unlock_irqrestore(&pool->lock, flags); > - /* paired with rmb in mempool_free(), read comment there */ > - smp_wmb(); > + if (unlikely(!element)) { > /* > - * Update the allocation stack trace as this is more useful > - * for debugging. > + * Try to allocate an element from the pool. > + * > + * The first pass won't have __GFP_DIRECT_RECLAIM and won't > + * sleep in mempool_alloc_from_pool. Retry the allocation > + * with all flags set in that case. > */ > - kmemleak_update_trace(element); > - return element; > - } > - > - /* > - * We use gfp mask w/o direct reclaim or IO for the first round. If > - * alloc failed with that and @pool was empty, retry immediately. > - */ > - if (gfp_temp != gfp_mask) { > - spin_unlock_irqrestore(&pool->lock, flags); > - gfp_temp = gfp_mask; > - goto repeat_alloc; > - } > - > - /* We must not sleep if !__GFP_DIRECT_RECLAIM */ > - if (!(gfp_mask & __GFP_DIRECT_RECLAIM)) { > - spin_unlock_irqrestore(&pool->lock, flags); > - return NULL; > + element = mempool_alloc_from_pool(pool, gfp_mask); > + if (!element && gfp_temp != gfp_mask) { No, that is wrong, it breaks the mempool promise: linux-next oopses in swap_writepage_bdev_async(), which relies on bio_alloc(,,,GFP_NOIO) to return a good bio. The refactoring makes it hard to see, but the old version always used to go back to repeat_alloc at the end, if __GFP_DIRECT_RECLAIM, whereas here it only does so the first time, when gfp_temp != gfp_mask. After bisecting to here, I changed that "gfp_temp != gfp_mask" to "(gfp & __GFP_DIRECT_RECLAIM)", and it worked again. But other patches have come in on top, so below is a patch to the final mm/mempool.c... > + gfp_temp = gfp_mask; > + goto repeat_alloc; > + } > } > > - /* Let's wait for someone else to return an element to @pool */ > - init_wait(&wait); > - prepare_to_wait(&pool->wait, &wait, TASK_UNINTERRUPTIBLE); > - > - spin_unlock_irqrestore(&pool->lock, flags); > - > - /* > - * FIXME: this should be io_schedule(). The timeout is there as a > - * workaround for some DM problems in 2.6.18. > - */ > - io_schedule_timeout(5*HZ); > - > - finish_wait(&pool->wait, &wait); > - goto repeat_alloc; > + return element; > } > EXPORT_SYMBOL(mempool_alloc_noprof); ... [PATCH] mempool: fix NULL from mempool_alloc_noprof() mempool_alloc_noprof() with __GFP_DIRECT_RECLAIM used to loop until it had allocated an element, but recently regressed to returning NULL when pool->alloc and mempool_alloc_from_pool() have both failed twice, causing oops in __swap_writepage() (and presumably others relying on mempool). Fixes: 1d091d2c5bf3 ("mempool: factor out a mempool_alloc_from_pool helper") Signed-off-by: Hugh Dickins --- mm/mempool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/mempool.c b/mm/mempool.c index 1558601600ba..dc9f2a1dc35f 100644 --- a/mm/mempool.c +++ b/mm/mempool.c @@ -576,7 +576,7 @@ void *mempool_alloc_noprof(struct mempool *pool, gfp_t gfp_mask) * with all flags set in that case. */ if (!mempool_alloc_from_pool(pool, &element, 1, 0, gfp_mask) && - gfp_temp != gfp_mask) { + (gfp_mask & __GFP_DIRECT_RECLAIM)) { gfp_temp = gfp_mask; goto repeat_alloc; } -- 2.51.0