Messages in this thread Patch in this message | ![/](/images/icornerl.gif) | | Date | Wed, 23 Apr 2003 16:26:40 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [BK ] new module infrastructure for net_proto_family |
| |
Em Wed, Apr 23, 2003 at 12:13:37PM -0700, Max Krasnyansky escreveu: > Hi Folks, > > Can somebody (DaveM perhaps) please explain to me what the hell the following > changset is doing in 2.5.68 > http://linux.bkbits.net:8080/linux-2.5/cset@1.1118.1.1?nav=index.html|ChangeSet@-7d > ?? > > I've spent quite a bit of time looking into what's needed to fix socket module refcounting > issues and convicting DaveM and Alex that we need to fix them. > Here is the original thread > http://marc.theaimsgroup.com/?l=linux-kernel&m=104308300808557&w=2
I was not aware of this thread, will read it
> My patch was rejected without giving any technical explanation. I was waiting for Rusty's > __module_get() patch to get in, to release a new patch which addresses some issues that > came up during discussion. > Next thing you know, incomplete patch (changeset mentioned above) shows up in the BK and > 2.5.68. Without even a simple note on the lkm. And completely ignoring discussion that > we had about this very issue just a few weeks ago.
This is just the first part, DaveM already merged the second part, that deals with struct sock
> I don't mind of course if some other (better) patch is accepted instead of mine. But patch > that went in is incomplete and buggy. It doesn't handle accept case properly and doesn't > address the issue of the 'struct sock' ownership (read original thread for more details).
This is dealt with the following patch, and these patches were discussed on the netdev mailing list. Considerations? I'll be sending today patches converting the net protocols that I maintain and then for some others that are ummaintained, etc.
- Arnaldo
You can import this changeset into BK by piping this whole message to: '| bk receive [path to repository]' or apply the patch as usual.
===================================================================
ChangeSet@1.1180, 2003-04-23 02:53:59-03:00, acme@conectiva.com.br o net: module refcounting for sk_alloc/sk_free I had to move the rtnetlink_init and init_netlink calls to af_netlink init time, so that the sk_alloc called down the rtnetlink_init callchain is done after the PF_NETLINK net_proto_family is sock_registered, and because of that the af_netlink init function call had to be moved to earlier by means of subsys_initcall (DaveM's suggestion).
include/linux/net.h | 3 +++ net/core/sock.c | 13 +++++++++---- net/netlink/af_netlink.c | 8 +++++++- net/socket.c | 42 +++++++++++++++++++++++++++--------------- 4 files changed, 46 insertions(+), 20 deletions(-)
diff -Nru a/include/linux/net.h b/include/linux/net.h --- a/include/linux/net.h Wed Apr 23 02:57:58 2003 +++ b/include/linux/net.h Wed Apr 23 02:57:58 2003 @@ -140,6 +140,9 @@ struct module *owner; }; +extern int net_family_get(int family); +extern void net_family_put(int family); + struct iovec; extern int sock_wake_async(struct socket *sk, int how, int band); diff -Nru a/net/core/sock.c b/net/core/sock.c --- a/net/core/sock.c Wed Apr 23 02:57:58 2003 +++ b/net/core/sock.c Wed Apr 23 02:57:58 2003 @@ -589,8 +589,10 @@ */ struct sock *sk_alloc(int family, int priority, int zero_it, kmem_cache_t *slab) { - struct sock *sk; - + struct sock *sk = NULL; + + if (!net_family_get(family)) + goto out; if (!slab) slab = sk_cachep; sk = kmem_cache_alloc(slab, priority); @@ -602,14 +604,16 @@ sock_lock_init(sk); } sk->slab = slab; - } - + } else + net_family_put(family); +out: return sk; } void sk_free(struct sock *sk) { struct sk_filter *filter; + const int family = sk->family; if (sk->destruct) sk->destruct(sk); @@ -624,6 +628,7 @@ printk(KERN_DEBUG "sk_free: optmem leakage (%d bytes) detected.\n", atomic_read(&sk->omem_alloc)); kmem_cache_free(sk->slab, sk); + net_family_put(family); } void __init sk_init(void) diff -Nru a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c --- a/net/netlink/af_netlink.c Wed Apr 23 02:57:58 2003 +++ b/net/netlink/af_netlink.c Wed Apr 23 02:57:58 2003 @@ -1052,6 +1052,7 @@ struct net_proto_family netlink_family_ops = { .family = PF_NETLINK, .create = netlink_create, + .owner = THIS_MODULE, /* for consistency 8) */ }; static int __init netlink_proto_init(void) @@ -1066,6 +1067,11 @@ #ifdef CONFIG_PROC_FS create_proc_read_entry("net/netlink", 0, 0, netlink_read_proc, NULL); #endif + /* The netlink device handler may be needed early. */ + rtnetlink_init(); +#ifdef CONFIG_NETLINK_DEV + init_netlink(); +#endif return 0; } @@ -1075,7 +1081,7 @@ remove_proc_entry("net/netlink", NULL); } -module_init(netlink_proto_init); +subsys_initcall(netlink_proto_init); module_exit(netlink_proto_exit); MODULE_LICENSE("GPL"); diff -Nru a/net/socket.c b/net/socket.c --- a/net/socket.c Wed Apr 23 02:57:58 2003 +++ b/net/socket.c Wed Apr 23 02:57:58 2003 @@ -69,8 +69,6 @@ #include <linux/proc_fs.h> #include <linux/seq_file.h> #include <linux/wanrouter.h> -#include <linux/netlink.h> -#include <linux/rtnetlink.h> #include <linux/if_bridge.h> #include <linux/init.h> #include <linux/poll.h> @@ -143,6 +141,31 @@ static struct net_proto_family *net_families[NPROTO]; +static __inline__ void net_family_bug(int family) +{ + printk(KERN_ERR "%d is not yet sock_registered!\n", family); + BUG(); +} + +int net_family_get(int family) +{ + int rc = 1; + + if (likely(net_families[family] != NULL)) + rc = try_module_get(net_families[family]->owner); + else + net_family_bug(family); + return rc; +} + +void net_family_put(int family) +{ + if (likely(net_families[family] != NULL)) + module_put(net_families[family]->owner); + else + net_family_bug(family); +} + #if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT) static atomic_t net_family_lockct = ATOMIC_INIT(0); static spinlock_t net_family_lock = SPIN_LOCK_UNLOCKED; @@ -511,7 +534,7 @@ sock->ops->release(sock); sock->ops = NULL; - module_put(net_families[family]->owner); + net_family_put(family); } if (sock->fasync_list) @@ -1064,7 +1087,7 @@ sock->type = type; i = -EBUSY; - if (!try_module_get(net_families[family]->owner)) + if (!net_family_get(family)) goto out_release; if ((i = net_families[family]->create(sock, protocol)) < 0) @@ -1953,17 +1976,6 @@ * do_initcalls is run. */ - - /* - * The netlink device handler may be needed early. - */ - -#ifdef CONFIG_NET - rtnetlink_init(); -#endif -#ifdef CONFIG_NETLINK_DEV - init_netlink(); -#endif #ifdef CONFIG_NETFILTER netfilter_init(); #endif ===================================================================
This BitKeeper patch contains the following changesets: 1.1180 ## Wrapped with gzip_uu ##
M'XL( &8KICX ^U8;7/:1A#^K/L5FV0ZQ:F!.[V+C#-N@I,P=AR/$_=+T]&( MTPDT",DCG4AHR7_OGF0P5L OM-]L8$;2:7?UW-X^C_9X 1>%R'M:P*>"O( / M62%[&L]2P64\"SH\FW:&.=XXSS*\T1UG4]%]<]Q-A6SK'8O@G;- \C',1%[T M--8Q5B-R?BEZVOG1^XN3W\\).3B M^,@'8G/0L+! 9%9/@N2L#@,Y#C)TH[, M@[28"ED]<[$R7>B4ZOBUF&-0RUXPFYK.@K.0L<!D(J2ZZ=HF22;3Y+ 8BZDH MQIUA_/?- 8U=9W9AHFNS+8=C_2!=1AS*5"C2\VN;@#5>Y;1L[PV-7J4@LK' M83,/\)L);4K>P/\+_BWAD &FM ?3+"P3 ;F(>%:F,DY'$&4Y%!,_2)*,=_$D MRH5 !_P-8!R$B 6]9@+D&/TD1DGB=.+':2PA2$-0)_[5*'",4BB/(%J-598R MGHI]*#*,$D@,K8(M'UIYB1#"[%NZZ2GJ-A\'<0IQ@4:IP.A2Y)7IV3O_].C+ MR>#T&(.BEW^99S+SHV :)W-E7V1\XN=B%!?H(L+]"O-0\* L!&11A:>*U$0< ME2DN39:2&M\R%4-19:,Z%T&>Q AD.(>IP 52\8IR6,R+"GCEUNH',_'Q5P12 MCD:B4!'W.N08F.W8#CF[KEG2?N"'$!I0\OJ.6HE3GI2AZ.+$RN^*5IWQ>M5X M)EWH'GX7MNE15<+,#6A@"[&Y0K?%4PS $C1=RUW8AFW<#0Q=NVIQ, *_@<AR M$(OC6 M.(T>W@\BRG&BH>WP+HF:@=2C,9 :[%Y2KM>]>ET$3EK=@&,Q:6)QQ M(S+,D(O 9=R^!=:VH.L0==<US'M!Y%DNJIDVD57S])#X^M!E@2N&'G<,9MP" MK!'J1LKPPJSD=,-2WRVL.]<;>6B],4\W=,:<A66YAETKKGE#;TVW9[FWZZWQ MI+>/0V]K4?H$[?Q;]4/]/-M4X3O(\("9.AA$?,<98\Y2J8'ZJ.S4>?%'0K9P M'.K+O5=+VUD6AS\97Y8-XZ\5&QNTO9N)NT@&"3%[T\.TE$4GQ:4).AAEFV(P M:ND>M1>4,48K_BG:/8Q_'K3-)_X] O[5KY4&_QJ%M0/W^I:G@TX&EF> 2;1" MYB6751+@93&! SB].#E1%-+B"%K/&IR\HM@>T;01IA&R4KXB?9M:*J1-;62U M]@-$4@BT:%!T14]TZJ$UTX$1M;,I)%S3%Q$4D_;K^N(5FNFV,ML6:TGT38W# M_1B_>Q^SY05\1Q]##8:['M-"/;&86:N _E 5<*#-GE3@$:A W>QN4(%-%;;3 MJQB[6$6P#F95Y-H!?/DP^.Q__-2_.#G:U[HOJW57)%692?D<W#UXV55^M@L6 M419?,"W+G(1B%G.!64C#!*<\#>8J%:D0(>9")6+>07<@VLW%:R&57\11*")X M^^GTW>#]<KG\_M$?*$5KQ5.9BC2,(])GU'$1_.#JV$AJ:_F$>K'5\)IB+'= M]U.)AVV\;E&&QL9KU9/;U+#KGMQZL!KH2@ZL)SUX!'I0[\\WZ,&RKG9I"9RJ M(\#W$>@6*60@8PX^8L%9"=^OFN[U?GM8CM;[;?(/T2YS')BTCH_.3_VC\W-X M_DNHLIAF$N9"-K/Y[&OZ?/^Z7=?>7+Q7K/Z!78>*NWT?H!ZE+G-D+;!5EY+$ M$Y',6RN_6!1_UAY_P;.ZHZE:ELI-YG._+N<J^B:G]NM*#!6TGUL9-?MKZ+F0 M)>Y+<E[#;Z:JL36I\#\ \!5.%>6_X538^MAN*+&L#]O;,U15VZE5M3K>W@CV ?F6=AA\:N_WCF8\$G13D]$'KHF=2FY%]2!X$WTQ8
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |