linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Waiman Long <longman@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>, Will Deacon <will.deacon@arm.com>,
	linux-kernel@vger.kernel.org,
	Bart Van Assche <bvanassche@acm.org>
Subject: Re: [PATCH v2 4/6] locking/lockdep: Reuse freed chain_hlocks entries
Date: Wed, 15 Jan 2020 14:26:38 -0500	[thread overview]
Message-ID: <6b3f5617-7c91-f013-039c-ed9c0ed13c68@redhat.com> (raw)
In-Reply-To: <20200115104446.GK2827@hirez.programming.kicks-ass.net>

On 1/15/20 5:44 AM, Peter Zijlstra wrote:
> On Tue, Jan 14, 2020 at 02:16:58PM -0500, Waiman Long wrote:
>> On 1/14/20 4:46 AM, Peter Zijlstra wrote:
>>> I'm thinking worst-fit might work well for our use-case. Best-fit would
>>> result in a heap of tiny fragments and we don't have really large
>>> allocations, which is the Achilles-heel of worst-fit.
>> I am going to add a patch to split chain block as a last resort in case
>> we run out of the main buffer.
> It will be the common path; you'll start with a single huge fragment.
>
> Remember, 1 allocator is better than 2.

One reason why I keep the array allocation code is because it is simple
and fast. Using a free block in the bucket will be a bit slower. Still
we are just dealing with the first block in the list as long as the size
isn't bigger than MAX_CHAIN_BUCKETS. Now if we need to deal with block
splitting (or even merging), it will be even more slow. Given the fact
that we are holding the graph lock in doing all these. It could have a
visible impact on performance.

That is the primary reason why I intend to allow block splitting only as
the last resort when the chain_hlocks array is running out.

>>> Also, since you put in a minimal allocation size of 2, but did not
>>> mandate size is a multiple of 2, there is a weird corner case of size-1
>>> fragments. The simplest case is to leak those, but put in a counter so
>>> we can see if they're a problem -- there is a fairly trivial way to
>>> recover them without going full merge.
>> There is no size-1 fragment. Are you referring to the those blocks with
>> a size of 2, but with only one entry used? There are some wasted space
>> there. I can add a counter to track that.
> There will be; imagine you have a size-6 fragment and request a size-5,
> then we'll have to split off one. But one is too short to encode on the
> free lists.
>
> Suppose you tag them with -2, then on free of the size-5, we can check
> if curr+size+1 is -2 and reunite.
>
> First-fit or best-fit would result in lots of that, hence my suggestion
> to use worst-fit if you can't find an exact match.
>
What I am thinking is to allow block splitting only if it has a size
that is at least 2 larger than the desire value. In that way, we will
not produce a fragment that is less than 2 in size. The down side is
that we have fewer viable candidates for splitting.

Cheers,
Longman


  reply	other threads:[~2020-01-15 19:26 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-16 15:15 [PATCH v2 0/6] locking/lockdep: Reuse zapped chain_hlocks entries Waiman Long
2019-12-16 15:15 ` [PATCH v2 1/6] locking/lockdep: Track number of zapped classes Waiman Long
2020-01-13 14:55   ` Peter Zijlstra
2020-01-13 14:58     ` Waiman Long
2020-01-13 16:02       ` Bart Van Assche
2019-12-16 15:15 ` [PATCH v2 2/6] locking/lockdep: Throw away all lock chains with zapped class Waiman Long
2020-01-13 15:18   ` Peter Zijlstra
2020-01-13 15:44     ` Waiman Long
2020-01-13 16:05     ` Bart Van Assche
2020-01-13 16:15       ` Waiman Long
2019-12-16 15:15 ` [PATCH v2 3/6] locking/lockdep: Track number of zapped lock chains Waiman Long
2019-12-16 15:15 ` [PATCH v2 4/6] locking/lockdep: Reuse freed chain_hlocks entries Waiman Long
2020-01-13 15:51   ` Peter Zijlstra
2020-01-13 16:04     ` Waiman Long
2020-01-13 15:58   ` Peter Zijlstra
2020-01-13 16:24     ` Waiman Long
2020-01-14  9:46       ` Peter Zijlstra
2020-01-14 19:16         ` Waiman Long
2020-01-15 10:44           ` Peter Zijlstra
2020-01-15 19:26             ` Waiman Long [this message]
2020-01-13 15:59   ` Peter Zijlstra
2020-01-13 16:03     ` Waiman Long
2019-12-16 15:15 ` [PATCH v2 5/6] locking/lockdep: Decrement irq context counters when removing lock chain Waiman Long
2020-01-14  9:53   ` Peter Zijlstra
2020-01-14 15:04     ` Waiman Long
2019-12-16 15:15 ` [PATCH v2 6/6] locking/lockdep: Display irq_context names in /proc/lockdep_chains Waiman Long
2020-01-06 15:54 ` [PATCH v2 0/6] locking/lockdep: Reuse zapped chain_hlocks entries Waiman Long
2020-01-06 16:50   ` Peter Zijlstra
2020-01-06 16:52     ` Waiman Long

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=6b3f5617-7c91-f013-039c-ed9c0ed13c68@redhat.com \
    --to=longman@redhat.com \
    --cc=bvanassche@acm.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=will.deacon@arm.com \
    /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).