]> www.infradead.org Git - users/dwmw2/linux.git/commitdiff
kernfs: Relax constraint in draining guard
authorMichal Koutný <mkoutny@suse.com>
Mon, 5 May 2025 12:12:00 +0000 (14:12 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 21 May 2025 12:23:13 +0000 (14:23 +0200)
The active reference lifecycle provides the break/unbreak mechanism but
the active reference is not truly active after unbreak -- callers don't
use it afterwards but it's important for proper pairing of kn->active
counting. Assuming this mechanism is in place, the WARN check in
kernfs_should_drain_open_files() is too sensitive -- it may transiently
catch those (rightful) callers between
kernfs_unbreak_active_protection() and kernfs_put_active() as found out by Chen
Ridong:

kernfs_remove_by_name_ns kernfs_get_active // active=1
__kernfs_remove   // active=0x80000002
kernfs_drain ...
wait_event
//waiting (active == 0x80000001)
kernfs_break_active_protection
// active = 0x80000001
// continue
kernfs_unbreak_active_protection
// active = 0x80000002
...
kernfs_should_drain_open_files
// warning occurs
kernfs_put_active

To avoid the false positives (mind panic_on_warn) remove the check altogether.
(This is meant as quick fix, I think active reference break/unbreak may be
simplified with larger rework.)

Fixes: bdb2fd7fc56e1 ("kernfs: Skip kernfs_drain_open_files() more aggressively")
Link: https://lore.kernel.org/r/kmmrseckjctb4gxcx2rdminrjnq2b4ipf7562nvfd432ld5v5m@2byj5eedkb2o/
Cc: Chen Ridong <chenridong@huawei.com>
Signed-off-by: Michal Koutný <mkoutny@suse.com>
Acked-by: Tejun Heo <tj@kernel.org>
Link: https://lore.kernel.org/r/20250505121201.879823-1-mkoutny@suse.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
fs/kernfs/dir.c
fs/kernfs/file.c

index d63a96786c9bde8a28aa30c6bf79444de43eba50..a670ba3e565e03e96fac7543b5c3cf2a1eb133f3 100644 (file)
@@ -1582,8 +1582,9 @@ void kernfs_break_active_protection(struct kernfs_node *kn)
  * invoked before finishing the kernfs operation.  Note that while this
  * function restores the active reference, it doesn't and can't actually
  * restore the active protection - @kn may already or be in the process of
- * being removed.  Once kernfs_break_active_protection() is invoked, that
- * protection is irreversibly gone for the kernfs operation instance.
+ * being drained and removed.  Once kernfs_break_active_protection() is
+ * invoked, that protection is irreversibly gone for the kernfs operation
+ * instance.
  *
  * While this function may be called at any point after
  * kernfs_break_active_protection() is invoked, its most useful location
index 66fe8fe41f06056a7517756807e30f4e353f72aa..a6c692cac61659d58dbf14af992510a716eba8b3 100644 (file)
@@ -778,8 +778,9 @@ bool kernfs_should_drain_open_files(struct kernfs_node *kn)
        /*
         * @kn being deactivated guarantees that @kn->attr.open can't change
         * beneath us making the lockless test below safe.
+        * Callers post kernfs_unbreak_active_protection may be counted in
+        * kn->active by now, do not WARN_ON because of them.
         */
-       WARN_ON_ONCE(atomic_read(&kn->active) != KN_DEACTIVATED_BIAS);
 
        rcu_read_lock();
        on = rcu_dereference(kn->attr.open);