From: David Hildenbrand <david@redhat.com>
To: Liam Howlett <liam.howlett@oracle.com>, Mike Rapoport <rppt@kernel.org>
Cc: "maple-tree@lists.infradead.org" <maple-tree@lists.infradead.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Jirka Hladky <jhladky@redhat.com>,
Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH] maple_tree: Remove GFP_ZERO from kmem_cache_alloc() and kmem_cache_alloc_bulk()
Date: Tue, 24 Jan 2023 14:17:59 +0100 [thread overview]
Message-ID: <de1ed6a6-6f74-2664-dad4-3467efd83483@redhat.com> (raw)
In-Reply-To: <20230106183559.mpdywid4szzpghlk@revolver>
On 06.01.23 19:36, Liam Howlett wrote:
> * Mike Rapoport <rppt@kernel.org> [230106 02:28]:
>> On Thu, Jan 05, 2023 at 04:05:34PM +0000, Liam Howlett wrote:
>>> Preallocations are common in the VMA code to avoid allocating under
>>> certain locking conditions. The preallocations must also cover the
>>> worst-case scenario. Removing the GFP_ZERO flag from the
>>> kmem_cache_alloc() (and bulk variant) calls will reduce the amount of
>>> time spent zeroing memory that may not be used. Only zero out the
>>> necessary area to keep track of the allocations in the maple state.
>>> Zero the entire node prior to using it in the tree.
>>>
>>> This required internal changes to node counting on allocation, so the
>>> test code is also updated.
>>>
>>> This restores some micro-benchmark performance:
>>> up to +9% in mmtests mmap1 by my testing
>>> +10% to +20% in mmap, mmapaddr, mmapmany tests reported by Red Hat
>>>
>>> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2149636
>>> Reported-by: Jirka Hladky <jhladky@redhat.com>
>>> Suggested-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>>> Signed-off-by: Liam Howlett <Liam.Howlett@oracle.com>
>>> ---
>>> lib/maple_tree.c | 80 +++++++++++++++++---------------
>>> tools/testing/radix-tree/maple.c | 18 +++----
>>> 2 files changed, 52 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
>>> index 26e2045d3cda..82a8121fe49b 100644
>>> --- a/lib/maple_tree.c
>>> +++ b/lib/maple_tree.c
>>> @@ -149,13 +149,12 @@ struct maple_subtree_state {
>>> /* Functions */
>>> static inline struct maple_node *mt_alloc_one(gfp_t gfp)
>>> {
>>> - return kmem_cache_alloc(maple_node_cache, gfp | __GFP_ZERO);
>>> + return kmem_cache_alloc(maple_node_cache, gfp);
>>> }
>>>
>>> static inline int mt_alloc_bulk(gfp_t gfp, size_t size, void **nodes)
>>> {
>>> - return kmem_cache_alloc_bulk(maple_node_cache, gfp | __GFP_ZERO, size,
>>> - nodes);
>>> + return kmem_cache_alloc_bulk(maple_node_cache, gfp, size, nodes);
>>> }
>>>
>>> static inline void mt_free_bulk(size_t size, void __rcu **nodes)
>>> @@ -1127,9 +1126,10 @@ static inline struct maple_node *mas_pop_node(struct ma_state *mas)
>>> {
>>> struct maple_alloc *ret, *node = mas->alloc;
>>> unsigned long total = mas_allocated(mas);
>>> + unsigned int req = mas_alloc_req(mas);
>>>
>>> /* nothing or a request pending. */
>>> - if (unlikely(!total))
>>> + if (WARN_ON(!total))
>>
>> Hmm, isn't WARN_ON() here too much?
>
> I don't think so. If we get to the point of asking for a node while we
> don't have any to give, then it's too late. It means we (I) have
> calculated the necessary nodes incorrectly and we won't have enough
> memory to fit things into the tree. It should never happen.
Either way, the suggestion is to use WARN_ON_ONCE() instead of WARN_ON().
... now documented in Documentation/process/coding-style.rst
--
Thanks,
David / dhildenb
next prev parent reply other threads:[~2023-01-24 13:20 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-05 16:05 [PATCH] maple_tree: Remove GFP_ZERO from kmem_cache_alloc() and kmem_cache_alloc_bulk() Liam Howlett
2023-01-06 7:28 ` Mike Rapoport
2023-01-06 18:36 ` Liam Howlett
2023-01-24 13:17 ` David Hildenbrand [this message]
2023-01-24 15:23 ` Liam R. Howlett
2023-01-24 13:02 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=de1ed6a6-6f74-2664-dad4-3467efd83483@redhat.com \
--to=david@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=jhladky@redhat.com \
--cc=liam.howlett@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=maple-tree@lists.infradead.org \
--cc=rppt@kernel.org \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).