[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c1af8458-7269-53c3-59f4-b87c5d51c208@redhat.com>
Date: Tue, 4 Feb 2020 11:12:13 -0500
From: Waiman Long <longman@...hat.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Ingo Molnar <mingo@...hat.com>, Will Deacon <will.deacon@....com>,
linux-kernel@...r.kernel.org, Bart Van Assche <bvanassche@....org>
Subject: Re: [PATCH v5 6/7] locking/lockdep: Reuse freed chain_hlocks entries
On 2/4/20 10:42 AM, Peter Zijlstra wrote:
> On Mon, Feb 03, 2020 at 11:41:46AM -0500, Waiman Long wrote:
>> + /*
>> + * We require a minimum of 2 (u16) entries to encode a freelist
>> + * 'pointer'.
>> + */
>> + req = max(req, 2);
>
> Would something simple like the below not avoid that whole 1 entry
> 'chain' nonsense?
>
> It boots and passes the selftests, so it must be perfect :-)
>
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -3163,7 +3163,7 @@ static int validate_chain(struct task_st
> * (If lookup_chain_cache_add() return with 1 it acquires
> * graph_lock for us)
> */
> - if (!hlock->trylock && hlock->check &&
> + if (!chain_head && !hlock->trylock && hlock->check &&
> lookup_chain_cache_add(curr, hlock, chain_key)) {
> /*
> * Check whether last held lock:
>
Well, I think that will eliminate the 1-entry chains for the process
context. However, we can still have 1-entry chain in the irq context, I
think, as long as there are process context locks in front of it.
I think this fix is still worthwhile as it will eliminate some of the
1-entry chains.
Cheers,
Longman
Powered by blists - more mailing lists