Regression for duplicate (?) console parameters on next-20220630

Regression for duplicate (?) console parameters on next-20220630

[Michael Walle]

Hi,

I'm not sure it these are the correct recipients, feel free to CC 
others.

Since next-20220630 (or maybe also since next-20220629) I'm getting the

[    3.707900] WARNING: CPU: 0 PID: 38 at fs/kernfs/dir.c:531 
kernfs_put.part.0+0x1a0/0x1d4
[    3.716313] kernfs_put: console/active: released with incorrect 
active_ref 0

on both arm and arm64 boards. See for example:
https://linux.kernelci.org/test/case/id/62bd840b330c4851eaa39c16/

I have the console set in the device tree as well as on the commandline.
Up until recently that wasn't a problem and I guess that should be a 
valid
configuration. That being said, the warn() will go away if I remove the
console= parameter on the commandline.

I haven't had time to do a bisect yet. That will probably my next step;
or maybe kernelci will already do that for me, Guillaume? Unless someone
has some more insights/ideas.

-michael

Re: Regression for duplicate (?) console parameters on next-20220630

[Nathan Chancellor]

On Thu, Jun 30, 2022 at 03:11:58PM +0200, Michael Walle wrote:
> Hi,
> 
> I'm not sure it these are the correct recipients, feel free to CC others.
> 
> Since next-20220630 (or maybe also since next-20220629) I'm getting the
> 
> [    3.707900] WARNING: CPU: 0 PID: 38 at fs/kernfs/dir.c:531
> kernfs_put.part.0+0x1a0/0x1d4
> [    3.716313] kernfs_put: console/active: released with incorrect
> active_ref 0
> 
> on both arm and arm64 boards. See for example:
> https://linux.kernelci.org/test/case/id/62bd840b330c4851eaa39c16/
> 
> I have the console set in the device tree as well as on the commandline.
> Up until recently that wasn't a problem and I guess that should be a valid
> configuration. That being said, the warn() will go away if I remove the
> console= parameter on the commandline.
> 
> I haven't had time to do a bisect yet. That will probably my next step;
> or maybe kernelci will already do that for me, Guillaume? Unless someone
> has some more insights/ideas.

I noticed this as well when booting ARCH=um defconfig.

I ended up doing a bisect against next-20220630 and I landed on the
driver-core merge (5732b42edfd18ee888e127fa13d425ed3ed1bef3). I did two
more bisects to figure out that there is some sort of contextual
contlict between commit 5831788afb17 ("Revert "printk: add kthread
console printers"") and commit b8f35fa1188b ("kernfs: Change
kernfs_notify_list to llist."), as it is only when those two changes are
present that this issue occurs. I am happy to provide more information
if necessary.

Cheers,
Nathan

Re: Regression for duplicate (?) console parameters on next-20220630

[Michael Walle]

Am 2022-06-30 21:35, schrieb Nathan Chancellor:
> On Thu, Jun 30, 2022 at 03:11:58PM +0200, Michael Walle wrote:
>> Hi,
>> 
>> I'm not sure it these are the correct recipients, feel free to CC 
>> others.
>> 
>> Since next-20220630 (or maybe also since next-20220629) I'm getting 
>> the
>> 
>> [    3.707900] WARNING: CPU: 0 PID: 38 at fs/kernfs/dir.c:531
>> kernfs_put.part.0+0x1a0/0x1d4
>> [    3.716313] kernfs_put: console/active: released with incorrect
>> active_ref 0
>> 
>> on both arm and arm64 boards. See for example:
>> https://linux.kernelci.org/test/case/id/62bd840b330c4851eaa39c16/
>> 
>> I have the console set in the device tree as well as on the 
>> commandline.
>> Up until recently that wasn't a problem and I guess that should be a 
>> valid
>> configuration. That being said, the warn() will go away if I remove 
>> the
>> console= parameter on the commandline.
>> 
>> I haven't had time to do a bisect yet. That will probably my next 
>> step;
>> or maybe kernelci will already do that for me, Guillaume? Unless 
>> someone
>> has some more insights/ideas.
> 
> I noticed this as well when booting ARCH=um defconfig.
> 
> I ended up doing a bisect against next-20220630 and I landed on the
> driver-core merge (5732b42edfd18ee888e127fa13d425ed3ed1bef3). I did two
> more bisects to figure out that there is some sort of contextual
> contlict between commit 5831788afb17 ("Revert "printk: add kthread
> console printers"") and commit b8f35fa1188b ("kernfs: Change
> kernfs_notify_list to llist."), as it is only when those two changes 
> are
> present that this issue occurs. I am happy to provide more information
> if necessary.

Thanks for the info.

I can confirm that reverting b8f35fa1188b will fix the error. I couldn't
revert 5831788afb17 though, didn't apply cleanly.

-michael

Re: Regression for duplicate (?) console parameters on next-20220630

[Imran Khan]

Hello Michael, Hello Nathan,

Thanks for sharing the test result.

On 1/7/22 7:18 am, Michael Walle wrote:
> Am 2022-06-30 21:35, schrieb Nathan Chancellor:
>> On Thu, Jun 30, 2022 at 03:11:58PM +0200, Michael Walle wrote:
>>> Hi,
>>>
>>> I'm not sure it these are the correct recipients, feel free to CC others.
>>>
>>> Since next-20220630 (or maybe also since next-20220629) I'm getting the
>>>
>>> [    3.707900] WARNING: CPU: 0 PID: 38 at fs/kernfs/dir.c:531
>>> kernfs_put.part.0+0x1a0/0x1d4
>>> [    3.716313] kernfs_put: console/active: released with incorrect
>>> active_ref 0
>>>
>>> on both arm and arm64 boards. See for example:
>>> https://urldefense.com/v3/__https://linux.kernelci.org/test/case/id/62bd840b330c4851eaa39c16/__;!!ACWV5N9M2RV99hQ!LGrYRXVZCYKcB-xmJ52QGhidx1F_PJNpJPqzVxrbFnprXCDPx0p8f7uqnH-_pc0KrO3alOkTJuvOm2q2qzA$
>>>
>>> I have the console set in the device tree as well as on the commandline.
>>> Up until recently that wasn't a problem and I guess that should be a valid
>>> configuration. That being said, the warn() will go away if I remove the
>>> console= parameter on the commandline.
>>>
>>> I haven't had time to do a bisect yet. That will probably my next step;
>>> or maybe kernelci will already do that for me, Guillaume? Unless someone
>>> has some more insights/ideas.
>>
>> I noticed this as well when booting ARCH=um defconfig.
>>
>> I ended up doing a bisect against next-20220630 and I landed on the
>> driver-core merge (5732b42edfd18ee888e127fa13d425ed3ed1bef3). I did two
>> more bisects to figure out that there is some sort of contextual
>> contlict between commit 5831788afb17 ("Revert "printk: add kthread
>> console printers"") and commit b8f35fa1188b ("kernfs: Change
>> kernfs_notify_list to llist."), as it is only when those two changes are
>> present that this issue occurs. I am happy to provide more information
>> if necessary.
> 
> Thanks for the info.
> 
> I can confirm that reverting b8f35fa1188b will fix the error. I couldn't
> revert 5831788afb17 though, didn't apply cleanly.
> 

I tried to reproduce this issue on a qemu ARM instance (-m vexpress-a15, rootfs
based on buildroot) but could not reproduce this issue.
If you could share some more details about test steps/setup it would be great.
In the meanwhile based on the stack trace seen in WARN message and reverted
change, I think following change should fix this issue:


From 6bf7f1adc4b091dc6d6c60e0dd0f16247f61f374 Mon Sep 17 00:00:00 2001
From: Imran Khan <imran.f.khan@oracle.com>
Date: Fri, 1 Jul 2022 14:27:52 +1000
Subject: [PATCH] kernfs: Avoid re-adding kernfs_node into kernfs_notify_list.

---
 fs/kernfs/file.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index bb933221b4bae..e8ec054e11c63 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -917,6 +917,7 @@ static void kernfs_notify_workfn(struct work_struct *work)
        if (free == NULL)
                return;

+       free->next = NULL;
        attr = llist_entry(free, struct kernfs_elem_attr, notify_next);
        kn = attribute_to_node(attr, struct kernfs_node, attr);
        root = kernfs_root(kn);
@@ -992,9 +993,11 @@ void kernfs_notify(struct kernfs_node *kn)
        rcu_read_unlock();

        /* schedule work to kick fsnotify */
-       kernfs_get(kn);
-       llist_add(&kn->attr.notify_next, &kernfs_notify_list);
-       schedule_work(&kernfs_notify_work);
+       if (kn->attr.notify_next.next != NULL) {
+               kernfs_get(kn);
+               llist_add(&kn->attr.notify_next, &kernfs_notify_list);
+               schedule_work(&kernfs_notify_work);
+       }
 }
 EXPORT_SYMBOL_GPL(kernfs_notify);


base-commit: 6cc11d2a1759275b856e464265823d94aabd5eaf
-- 
2.30.2


Could you please verify this in your setup? If I can reproduce this issue
locally I will try myself as well.

Thanks,
 -- Imran