Skip to contentSkip to conversation

gigaset: fix module reference counting

PATCH In review Andreas Herrmann 2 participants 4 messages Jun 6, 2008 – Jun 23, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 gigaset: fix module reference counting

bas_gigaset can't be unloaded due to wrong module reference counting. Ensure that module_put is called whenever a channel is freed.

---
 drivers/isdn/gigaset/common.c  |    5 +----
 drivers/isdn/gigaset/gigaset.h |    1 -
 2 files changed, 1 insertions(+), 5 deletions(-)
drivers/isdn/gigaset/common.c +1 -4
index 827c32c..8498317 100644
@@ -184,7 +184,6 @@ int gigaset_get_channel(struct bc_state *bcs)
 		return 0;
 	}
 	++bcs->use_count;
-	bcs->busy = 1;
 	gig_dbg(DEBUG_ANY, "allocated channel %d", bcs->channel);
 	spin_unlock_irqrestore(&bcs->cs->lock, flags);
 	return 1;
@@ -195,13 +194,12 @@ void gigaset_free_channel(struct bc_state *bcs)
 	unsigned long flags;
 
 	spin_lock_irqsave(&bcs->cs->lock, flags);
-	if (!bcs->busy) {
+	if (!bcs->use_count) {
 		gig_dbg(DEBUG_ANY, "could not free channel %d", bcs->channel);
 		spin_unlock_irqrestore(&bcs->cs->lock, flags);
 		return;
 	}
 	--bcs->use_count;
-	bcs->busy = 0;
 	module_put(bcs->cs->driver->owner);
 	gig_dbg(DEBUG_ANY, "freed channel %d", bcs->channel);
 	spin_unlock_irqrestore(&bcs->cs->lock, flags);
@@ -589,7 +587,6 @@ static struct bc_state *gigaset_initbcs(struct bc_state *bcs,
 
 	bcs->chstate = 0;
 	bcs->use_count = 1;
-	bcs->busy = 0;
 	bcs->ignore = cs->ignoreframes;
 
 	for (i = 0; i < AT_NUM; ++i)
drivers/isdn/gigaset/gigaset.h +0 -1
index f365993..ef1bc17 100644
@@ -419,7 +419,6 @@ struct bc_state {
 #ifdef CONFIG_GIGASET_DEBUG
 	int emptycount;
 #endif
-	int busy;
 	int use_count;
 
 	/* private data of hardware drivers */
Signature · 1 line
1.5.3.7
#1 lore raw

Hi Andreas,

Quoted 2 lines · Andreas Herrmann

bas_gigaset can't be unloaded due to wrong module reference counting. Ensure that module_put is called whenever a channel is freed.

I'm not quite happy with that patch. Specifically, I see a risk of module_put being called too often.

Could you explain in some more detail what bug you are trying to fix? Do you have a sequence of events which leads to a situation where the bas_gigaset module cannot be unloaded but should be?

Thanks,
Tilman

#2 lore raw
Quoted 10 lines · Tilman Schmidt
Quoted 2 lines · Andreas Herrmann

bas_gigaset can't be unloaded due to wrong module reference counting. Ensure that module_put is called whenever a channel is freed.

I'm not quite happy with that patch. Specifically, I see a risk of module_put being called too often.

Could you explain in some more detail what bug you are trying to fix? Do you have a sequence of events which leads to a situation where the bas_gigaset module cannot be unloaded but should be?

I've tried to reproduce the problem doing various tests -- but without success.

Then i've checked the code and there was already this fix:

commit e468c04894f36045cf93d1384183a461014b6840 (Gigaset: permit module unload)

This was commited after 2.6.24 was released.

And (shit happens!) on the machine where I've previously tested the stuff I had also an 2.6.24 kernel installed.

So it seems that I have mixed up different kernel versions when I've tested my patch. Because with current git-tree bas_gigaset can be unloaded without any problems.

Sorry for the noise.

Regards,

Andreas

#3 lore raw

Hi,

am Date: Mon, 23 Jun 2008 09:32:27 +0200 schrieb Andreas Herrmann:

Quoted 5 lines

So it seems that I have mixed up different kernel versions when I've tested my patch. Because with current git-tree bas_gigaset can be unloaded without any problems.

Sorry for the noise.

No problem. Thanks for the feedback.

Take care,
Tilman

Signature · 4 lines
Tilman Schmidt                    E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)