Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <0aa365ca-a9f0-8d15-b515-adb8823f5d28@gmx.de>
Date:   Tue, 19 Jul 2022 18:32:04 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: WARNING: CPU: 1 PID: 14735 at fs/dcache.c:365
 dentry_free+0x100/0x128
Content-Language: en-US
To:     Hillf Danton <hdanton@sina.com>
Cc:     linux-kernel@vger.kernel.org, linux-parisc@vger.kernel.org
References: <20220709090756.2384-1-hdanton@sina.com>
 <20220715133300.1297-1-hdanton@sina.com>
 <cff76e00-3561-4069-f5c7-26d3de4da3c4@gmx.de>
 <20220717113634.1552-1-hdanton@sina.com>
From:   Helge Deller <deller@gmx.de>
In-Reply-To: <20220717113634.1552-1-hdanton@sina.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hello Hillf,

On 7/17/22 13:36, Hillf Danton wrote:
> On Sun, 17 Jul 2022 11:42:48 +0200
>> I used WARN_ON() instead of BUG_ON().
>> With that, both triggered, first the first one, then the second one.
>> Full log is here:
>> http://dellerweb.de/testcases/minicom.dcache.crash.6-warn
>
> Given the first BUG_ON triggered, and dentry at the moment is supposed t=
o
> not be alias, see if it is still in lookup with d_lock held. That is the
> step before de-unioning d_alias with d_in_lookup_hash.
>
> On the other hand if only the second one triggered, we should track
> DCACHE_DENTRY_KILLED instead in assumption that killed dentry was
> used again after releasing d_lock surrounding the firt one.

The machine has now been up for 2 days without any issues, while it had pr=
etty
much the same load as when it was crashing earlier.
So, in summary I'd assume that your patch below fixes the issue.

I'm now rebooting the machine with a new kernel, where I just changed
	if (unlikely(d_in_lookup(dentry)))
to
	if (WARN_ON_ONCE(d_in_lookup(dentry)))
in order to see if this really triggered.

Anyway, I think your patch is good so far.
Would that be the final patch, or should I test some others?

Thanks!
Helge

>
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -605,8 +605,12 @@ static void __dentry_kill(struct dentry
>  		spin_unlock(&parent->d_lock);
>  	if (dentry->d_inode)
>  		dentry_unlink_inode(dentry);
> -	else
> +	else {
> +		if (unlikely(d_in_lookup(dentry))) {
> +			__d_lookup_done(dentry);
> +		}
>  		spin_unlock(&dentry->d_lock);
> +	}
>  	this_cpu_dec(nr_dentry);
>  	if (dentry->d_op && dentry->d_op->d_release)
>  		dentry->d_op->d_release(dentry);