batman-adv: remove references for global tt entries

Message ID 1319014945-32281-1-git-send-email-siwu@hrz.tu-chemnitz.de (mailing list archive)
State Accepted, archived
Commit ee9f128534108be4eafd2ecb410ea89e1f063e8b
Headers

Commit Message

Simon Wunderlich Oct. 19, 2011, 9:02 a.m. UTC
  struct tt_global_entry holds a reference to an orig_node which must be
decremented before deallocating the structure.

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
---
 compat.c            |    8 --------
 compat.h            |    1 -
 translation-table.c |   14 +++++++++++++-
 3 files changed, 13 insertions(+), 10 deletions(-)
  

Comments

Marek Lindner Oct. 19, 2011, 9:20 a.m. UTC | #1
On Wednesday, October 19, 2011 11:02:25 Simon Wunderlich wrote:
> struct tt_global_entry holds a reference to an orig_node which must be
> decremented before deallocating the structure.
> 
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
> ---
>  compat.c            |    8 --------
>  compat.h            |    1 -
>  translation-table.c |   14 +++++++++++++-
>  3 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/compat.c b/compat.c
> index 88ceb40..1793904 100644
> --- a/compat.c
> +++ b/compat.c
> @@ -36,12 +36,4 @@ void free_rcu_tt_local_entry(struct rcu_head *rcu)
>         kfree(tt_local_entry);
>  }
>  
> -void free_rcu_tt_global_entry(struct rcu_head *rcu)
> -{
> -       struct tt_global_entry *tt_global_entry;
> -
> -       tt_global_entry = container_of(rcu, struct tt_global_entry, rcu);
> -       kfree(tt_global_entry);
> -}


If you remove the function from compat.c please also remove its declaration in 
compat.h.


Thanks,
Marek
  
Marek Lindner Oct. 19, 2011, 12:08 p.m. UTC | #2
On Wednesday, October 19, 2011 11:20:47 Marek Lindner wrote:
> > -void free_rcu_tt_global_entry(struct rcu_head *rcu)
> > -{
> > -       struct tt_global_entry *tt_global_entry;
> > -
> > -       tt_global_entry = container_of(rcu, struct tt_global_entry, rcu);
> > -       kfree(tt_global_entry);
> > -}
> 
> If you remove the function from compat.c please also remove its declaration
> in  compat.h.

My fault - the compat.h cleanup is there.
Next time I'll read the patch 3 times before commenting.  ;-)

Regards,
Marek
  
Antonio Quartulli Oct. 23, 2011, 9:40 a.m. UTC | #3
Hello,

On Wed, Oct 19, 2011 at 11:02:25AM +0200, Simon Wunderlich wrote:
> struct tt_global_entry holds a reference to an orig_node which must be
> decremented before deallocating the structure.
> 
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
> ---

I got a feedback on IRC from fishor_ who applied the patch while trying to
eliminate the kernel panic problem on module unload. The refcount has not been
printed, therefore we do not know if the patch really corrects it but at least
we know that this patch doesn't crash batman-adv :-)

Cheers,
  
Alexey Fisher Oct. 23, 2011, 9:43 a.m. UTC | #4
On 23.10.2011 11:40, Antonio Quartulli wrote:
> Hello,
> 
> On Wed, Oct 19, 2011 at 11:02:25AM +0200, Simon Wunderlich wrote:
>> struct tt_global_entry holds a reference to an orig_node which must be
>> decremented before deallocating the structure.
>>
>> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
>> ---
> 
> I got a feedback on IRC from fishor_ who applied the patch while trying to
> eliminate the kernel panic problem on module unload. The refcount has not been
> printed, therefore we do not know if the patch really corrects it but at least
> we know that this patch doesn't crash batman-adv :-)
> 
> Cheers,
> 

tested-by: Alexey Fisher <bug-track@fisher-privat.net>
  
Marek Lindner Oct. 24, 2011, 12:05 p.m. UTC | #5
On Sunday, October 23, 2011 11:43:05 Alexey Fisher wrote:
> On 23.10.2011 11:40, Antonio Quartulli wrote:
> > Hello,
> > 
> > On Wed, Oct 19, 2011 at 11:02:25AM +0200, Simon Wunderlich wrote:
> >> struct tt_global_entry holds a reference to an orig_node which must be
> >> decremented before deallocating the structure.
> >> 
> >> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
> >> ---
> > 
> > I got a feedback on IRC from fishor_ who applied the patch while trying
> > to eliminate the kernel panic problem on module unload. The refcount has
> > not been printed, therefore we do not know if the patch really corrects
> > it but at least we know that this patch doesn't crash batman-adv :-)
> > 
> > Cheers,
> 
> tested-by: Alexey Fisher <bug-track@fisher-privat.net>

Patch was applied in revision 5da9ad7.

Thanks,
Marek
  

Patch

diff --git a/compat.c b/compat.c
index 88ceb40..1793904 100644
--- a/compat.c
+++ b/compat.c
@@ -36,12 +36,4 @@  void free_rcu_tt_local_entry(struct rcu_head *rcu)
 	kfree(tt_local_entry);
 }
 
-void free_rcu_tt_global_entry(struct rcu_head *rcu)
-{
-	struct tt_global_entry *tt_global_entry;
-
-	tt_global_entry = container_of(rcu, struct tt_global_entry, rcu);
-	kfree(tt_global_entry);
-}
-
 #endif /* < KERNEL_VERSION(3, 0, 0) */
diff --git a/compat.h b/compat.h
index 43654e8..58c3c6a 100644
--- a/compat.h
+++ b/compat.h
@@ -63,7 +63,6 @@  void free_rcu_gw_node(struct rcu_head *rcu);
 void free_rcu_neigh_node(struct rcu_head *rcu);
 void free_rcu_softif_neigh(struct rcu_head *rcu);
 void free_rcu_tt_local_entry(struct rcu_head *rcu);
-void free_rcu_tt_global_entry(struct rcu_head *rcu);
 
 #endif /* < KERNEL_VERSION(3, 0, 0) */
 
diff --git a/translation-table.c b/translation-table.c
index 873fb3d..abf05cb 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -137,10 +137,22 @@  static void tt_local_entry_free_ref(struct tt_local_entry *tt_local_entry)
 		kfree_rcu(tt_local_entry, rcu);
 }
 
+static void tt_global_entry_free_rcu(struct rcu_head *rcu)
+{
+	struct tt_global_entry *tt_global_entry;
+
+	tt_global_entry = container_of(rcu, struct tt_global_entry, rcu);
+
+	if (tt_global_entry->orig_node)
+		orig_node_free_ref(tt_global_entry->orig_node);
+
+	kfree(tt_global_entry);
+}
+
 static void tt_global_entry_free_ref(struct tt_global_entry *tt_global_entry)
 {
 	if (atomic_dec_and_test(&tt_global_entry->refcount))
-		kfree_rcu(tt_global_entry, rcu);
+		call_rcu(&tt_global_entry->rcu, tt_global_entry_free_rcu);
 }
 
 static void tt_local_event(struct bat_priv *bat_priv, const uint8_t *addr,