31 Oct
2010
31 Oct
'10
3:32 p.m.
sysfs_del_hardif invokes kobject_put, which might sleep. However, we
are not allowed to sleep during a call_rcu. There is also no need to
do the removal with an atomic call_rcu, as kobject_put only frees the
kobject when there is no more reference to it anyway.
This commit basically revokes 7f32f2e8d97150ba5b80410dda86b01b0879fe8d,
despite not reintroducing the synchronize_rcu, our rcu_barrier should
handle this.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
---
hard-interface.c | 14 +++-----------
1 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/hard-interface.c b/hard-interface.c
index 37f0f8b..5c6ce3f 100644
--- a/hard-interface.c
+++ b/hard-interface.c
@@ -36,16 +36,6 @@
/* protect update critical side of if_list - but not the content */
static DEFINE_SPINLOCK(if_list_lock);
-static void hardif_free_rcu(struct rcu_head *rcu)
-{
- struct batman_if *batman_if;
-
- batman_if = container_of(rcu, struct batman_if, rcu);
- sysfs_del_hardif(&batman_if->hardif_obj);
- dev_put(batman_if->net_dev);
- kref_put(&batman_if->refcount, hardif_free_ref);
-}
-
struct batman_if *get_batman_if_by_netdev(struct net_device *net_dev)
{
struct batman_if *batman_if;
@@ -470,7 +460,9 @@ static void hardif_remove_interface(struct batman_if *batman_if)
/* caller must take if_list_lock */
list_del_rcu(&batman_if->list);
- call_rcu(&batman_if->rcu, hardif_free_rcu);
+ sysfs_del_hardif(&batman_if->hardif_obj);
+ dev_put(batman_if->net_dev);
+ kref_put(&batman_if->refcount, hardif_free_ref);
}
void hardif_remove_interfaces(void)
--
1.7.1
31 Oct
31 Oct
4:04 p.m.
New subject: [B.A.T.M.A.N.] [PATCH] batman-adv: Don't call sysfs_del_hardif() in atomic context
Linus Lüssing wrote:
sysfs_del_hardif invokes kobject_put, which might
sleep. However, we
are not allowed to sleep during a call_rcu. There is also no need to
do the removal with an atomic call_rcu, as kobject_put only frees the
kobject when there is no more reference to it anyway.
This commit basically revokes 7f32f2e8d97150ba5b80410dda86b01b0879fe8d,
despite not reintroducing the synchronize_rcu, our rcu_barrier should
handle this.
This is an extreme bad idea as we would free the object before the rcu grace
period is over. This would mean that any parallel run through the list would
probably access memory which is invalid. So this is a good way to crash your
machine.
What makes you think that kobject_put sleeps? There is no code which proves
it. The only reason would be that kobject_put -> kobject_release ->
kobject_cleanup -> ... sleeps. Please complete that chain to show were the
problem is. If it really sleeps then please only do the kobject related
cleanup outside of call_rcu.
Best regards,
Sven
4:12 p.m.
New subject: [B.A.T.M.A.N.] [PATCH] batman-adv: Don't call sysfs_del_hardif() in atomic context
Sven Eckelmann wrote:
Linus Lüssing wrote:
Found documentation about it in Documentation/kobject.txt
If you need to do a two-stage delete of the kobject (say you are not
allowed to sleep when you need to destroy the object), then call
kobject_del() which will unregister the kobject from sysfs. This makes the
kobject "invisible", but it is not cleaned up, and the reference count of
the object is still the same. At a later time call kobject_put() to finish
the cleanup of the memory associated with the kobject.
Please find another way to fix it - reverting
7f32f2e8d97150ba5b80410dda86b01b0879fe8d is no option (especially not when
removing rcu synchronization).
Best regards,
Sven
sysfs_del_hardif invokes kobject_put, which might
sleep. However, we
are not allowed to sleep during a call_rcu. There is also no need to
do the removal with an atomic call_rcu, as kobject_put only frees the
kobject when there is no more reference to it anyway.
This commit basically revokes 7f32f2e8d97150ba5b80410dda86b01b0879fe8d,
despite not reintroducing the synchronize_rcu, our rcu_barrier should
handle this.
This is an extreme bad idea as we would free the object before the rcu
grace period is over. This would mean that any parallel run through the
list would probably access memory which is invalid. So this is a good way
to crash your machine.
What makes you think that kobject_put sleeps? There is no code which proves
it. The only reason would be that kobject_put -> kobject_release ->
kobject_cleanup -> ... sleeps. Please complete that chain to show were the
problem is. If it really sleeps then please only do the kobject related
cleanup outside of call_rcu.
9:07 p.m.
New subject: [B.A.T.M.A.N.] [PATCHv2] batman-adv: Don't call sysfs_del_hardif() in atomic context
sysfs_del_hardif invokes kobject_put, which might sleep. However, we
are not allowed to sleep during a call_rcu. There is also no need to
do the removal with an atomic call_rcu, as kobject_put only frees the
kobject when there is no more reference to it anyway.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
---
hard-interface.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hard-interface.c b/hard-interface.c
index 37f0f8b..a9b8367 100644
--- a/hard-interface.c
+++ b/hard-interface.c
@@ -41,7 +41,6 @@ static void hardif_free_rcu(struct rcu_head *rcu)
struct batman_if *batman_if;
batman_if = container_of(rcu, struct batman_if, rcu);
- sysfs_del_hardif(&batman_if->hardif_obj);
dev_put(batman_if->net_dev);
kref_put(&batman_if->refcount, hardif_free_ref);
}
@@ -470,6 +469,7 @@ static void hardif_remove_interface(struct batman_if *batman_if)
/* caller must take if_list_lock */
list_del_rcu(&batman_if->list);
+ sysfs_del_hardif(&batman_if->hardif_obj);
call_rcu(&batman_if->rcu, hardif_free_rcu);
}
--
1.7.1
4 Nov
4 Nov
1:31 p.m.
New subject: [B.A.T.M.A.N.] [PATCHv2] batman-adv: Don't call sysfs_del_hardif() in atomic context
On Sunday 31 October 2010 22:07:37 Linus Lüssing wrote:
sysfs_del_hardif invokes kobject_put, which might
sleep. However, we
are not allowed to sleep during a call_rcu. There is also no need to
do the removal with an atomic call_rcu, as kobject_put only frees the
kobject when there is no more reference to it anyway.
Applied in revision 1856.
Thanks,
Marek
4591
days inactive
4595
days old
b.a.t.m.a.n@lists.open-mesh.org
4 comments
3 participants
participants (3)
-
Linus Lüssing -
Marek Lindner -
Sven Eckelmann