Summary:	DAHLIN-00009: Function dahdi_register should get a Linux 2.6 module and dahdi_unregister put
Reporter:	matti (matti)	Labels:
Date Opened:	2007-08-30 00:40:31	Date Closed:	2009-11-09 13:51:44.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	NewFeature
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) patch ( 1) patchv2
Description:	Function zt_register should get a and function zt_unregister put a Linux 2.6 module. Module zaptel can call functions of registered modules and it would be cleaner if zaptel itself took care of the modules not unloading and leaving stale function pointers.
Comments:	By: matti (matti) 2007-08-31 22:59:10 If a module registered with zaptel does not get itself, Asterisk processes can crash at least in Asterisk 1.2 if the module is removed. "Golden Rule: If you are calling though a function pointer into a (different) module, you must hold a reference to that module." http://ftp.kernel.org/pub/linux/kernel/people/rusty/modules/FAQ By: Tzafrir Cohen (tzafrir) 2007-12-13 09:58:50.000-0600 Matti: what are those patches? By: matti (matti) 2007-12-13 12:24:50.000-0600 Those patches delay the unloading of a module until all open channels are closed. patchv2 works better. By: Tzafrir Cohen (tzafrir) 2007-12-13 13:05:52.000-0600 I don't think you can afford waiting indefinetly. A user might have forgotten an application with the channel open. We can't just hang in kernel mode forever. By: Kevin P. Fleming (kpfleming) 2008-01-25 18:26:30.000-0600 I'm a bit confused. Modules already increment and decrement their own reference counts when channels are opened and closed. What do you mean by 'leaving stale function pointers'? How could a module get unloaded and not call zt_unregister() to remove the pointers that point to it? How are you unloading a module that has a non-zero reference count? By: matti (matti) 2008-01-26 02:17:12.000-0600 I just thought zaptel could take care of the module reference counting instead of duplicating the task in each driver. By: Jason Parker (jparker) 2008-03-28 16:25:49 What do we want to do here then? By: Tzafrir Cohen (tzafrir) 2008-04-01 10:10:26 First-off, I don't understand how the patches are related to the topic of the bug report. I also believe that they allow hanging rmmod forever under certain circumstances. Maybe open a separate bug for those if that is a valid issue? The topic of the bug report is a valid issue. xpp currently handles it explicitly by maintaining open counts for channels. Before that was done, incorrect removal caused Asterisk to crash in nice ways. I'm not sure exactly how other modules handle this, if at all. By: matti (matti) 2008-04-01 10:31:23 The patches are a way to avoid Asterisk crashing if some special channels are open. They do allow hanging rmmod forever under those circumstances. I think wct4xxp handles removal by try_module_get. My idea was just to move the try_module_get from all drivers to one location in the zaptel module. By: Tzafrir Cohen (tzafrir) 2008-07-07 07:03:46 While I don't think that the included patches fix the issue, the issue is a valid one. Setting this one to "new" as I have no specific plan right now how to fix it. By: Digium Subversion (svnbot) 2009-06-29 15:56:09 Repository: dahdi Revision: 6794 U   linux/trunk/drivers/dahdi/dahdi-base.c U   linux/trunk/drivers/dahdi/dahdi_dummy.c U   linux/trunk/drivers/dahdi/dahdi_dynamic.c U   linux/trunk/drivers/dahdi/pciradio.c U   linux/trunk/drivers/dahdi/tor2.c U   linux/trunk/drivers/dahdi/wcb4xxp/base.c U   linux/trunk/drivers/dahdi/wcfxo.c U   linux/trunk/drivers/dahdi/wct1xxp.c U   linux/trunk/drivers/dahdi/wct4xxp/base.c U   linux/trunk/drivers/dahdi/wctdm.c U   linux/trunk/drivers/dahdi/wctdm24xxp/base.c U   linux/trunk/drivers/dahdi/wcte11xp.c U   linux/trunk/drivers/dahdi/wcte12xp/base.c U   linux/trunk/drivers/dahdi/xpp/card_bri.c U   linux/trunk/drivers/dahdi/xpp/card_fxo.c U   linux/trunk/drivers/dahdi/xpp/card_fxs.c U   linux/trunk/drivers/dahdi/xpp/card_pri.c U   linux/trunk/drivers/dahdi/xpp/xpp_dahdi.c U   linux/trunk/include/dahdi/kernel.h ------------------------------------------------------------------------ r6794 | sruffell | 2009-06-29 15:56:08 -0500 (Mon, 29 Jun 2009) | 9 lines dahdi-base: Enable DAHDI to manage the reference counts for the board drivers. Adds a struct module 'owner' member to the dahdi_span structure and updates all the board drivers to set this member before registering the span.  This allows the core of dahdi to maintain the reference counts on the channels itself. (closes issue DAHLIN-9) Reported by: Matti ------------------------------------------------------------------------ http://svn.digium.com/view/dahdi?view=rev&revision=6794 By: Digium Subversion (svnbot) 2009-06-29 17:44:54 Repository: dahdi Revision: 6797 U   linux/trunk/drivers/dahdi/wcte11xp.c ------------------------------------------------------------------------ r6797 | sruffell | 2009-06-29 17:44:54 -0500 (Mon, 29 Jun 2009) | 3 lines wcte11xp: Set the owner field of the dahdi_span before registration. (related to issue DAHLIN-9) ------------------------------------------------------------------------ http://svn.digium.com/view/dahdi?view=rev&revision=6797 By: Digium Subversion (svnbot) 2009-11-09 13:51:43.000-0600 Repository: dahdi Revision: 7541 _U  linux/branches/2.2/ ------------------------------------------------------------------------ r7541 | sruffell | 2009-11-09 13:51:41 -0600 (Mon, 09 Nov 2009) | 48 lines Blocked revisions 6791,6794,6797,6926,6967 via svnmerge ........  r6791 | kpfleming | 2009-06-29 15:05:43 -0500 (Mon, 29 Jun 2009) | 12 lines    Improve MMX safety for DAHDI echo cancellers on 32-bit x86 systems.    Replaces the standard kernel FPU save/restore operations with custom written  versions for 32-bit x86 CPUs, which have been tested to be reliable and safe  to use.    (closes issue DAHLIN-39)  Reported by: tzafrir  Patches:        dahdi_mmx_fix.diff uploaded by tzafrir (license 46) ........  r6794 | sruffell | 2009-06-29 15:56:07 -0500 (Mon, 29 Jun 2009) | 9 lines    dahdi-base: Enable DAHDI to manage the reference counts for the board drivers.    Adds a struct module 'owner' member to the dahdi_span structure and updates  all the board drivers to set this member before registering the span.  This  allows the core of dahdi to maintain the reference counts on the channels  itself.    (closes issue DAHLIN-9)  Reported by: Matti ........  r6797 | sruffell | 2009-06-29 17:44:54 -0500 (Mon, 29 Jun 2009) | 3 lines    wcte11xp: Set the owner field of the dahdi_span before registration.    (related to issue DAHLIN-9) ........  r6926 | sruffell | 2009-08-04 11:22:17 -0500 (Tue, 04 Aug 2009) | 1 line    wctdm24xxp: We no longer need to keep our own reference count. ........  r6967 | tzafrir | 2009-08-11 23:49:47 -0500 (Tue, 11 Aug 2009) | 7 lines    xpp: Don't check for the 'owner' field too soon    The owner field should only tested after the low-level driver  pre-registration method is called.    xpp rev: 7287 ........ ------------------------------------------------------------------------ http://svn.digium.com/view/dahdi?view=rev&revision=7541