| Message ID | 1454697361.28847.39.camel@tiscali.nl |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Feb 5, 2016 at 7:36 PM, Paul Bolle <pebolle@tiscali.nl> wrote: > On vr, 2016-02-05 at 17:06 +0100, Paul Bolle wrote: >> If that would happen, then cs can be reused while the previous >> > cs->hw.ser is not freed yet. Just a guess. >> >> I'll have to ponder on that a bit, sorry. > > This is from the hit-the-code-until-it-confesses department: > --- a/drivers/isdn/gigaset/ser-gigaset.c > +++ b/drivers/isdn/gigaset/ser-gigaset.c > @@ -373,13 +373,9 @@ static void gigaset_freecshw(struct cardstate *cs) > > static void gigaset_device_release(struct device *dev) > { > - struct cardstate *cs = dev_get_drvdata(dev); > - > - if (!cs) > - return; > + struct ser_cardstate *scs = dev_get_drvdata(dev); > dev_set_drvdata(dev, NULL); > - kfree(cs->hw.ser); > - cs->hw.ser = NULL; > + kfree(scs); > } > > /* > @@ -408,7 +404,7 @@ static int gigaset_initcshw(struct cardstate *cs) > cs->hw.ser = NULL; > return rc; > } > - dev_set_drvdata(&cs->hw.ser->dev.dev, cs); > + dev_set_drvdata(&cs->hw.ser->dev.dev, scs); > > tasklet_init(&cs->write_tasklet, > gigaset_modem_fill, (unsigned long) cs); > > Does that make any difference? Nope. Almost 500 objects leaked in less than 10 seconds: # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 1992 2015 2520 13 8 : tunables 0 0 0 : slabdata 155 155 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2024 2041 2520 13 8 : tunables 0 0 0 : slabdata 157 157 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2061 2080 2520 13 8 : tunables 0 0 0 : slabdata 160 160 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2091 2119 2520 13 8 : tunables 0 0 0 : slabdata 163 163 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2147 2171 2520 13 8 : tunables 0 0 0 : slabdata 167 167 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2228 2236 2520 13 8 : tunables 0 0 0 : slabdata 172 172 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2261 2288 2520 13 8 : tunables 0 0 0 : slabdata 176 176 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2289 2301 2520 13 8 : tunables 0 0 0 : slabdata 177 177 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2316 2340 2520 13 8 : tunables 0 0 0 : slabdata 180 180 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2324 2366 2520 13 8 : tunables 0 0 0 : slabdata 182 182 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2356 2379 2520 13 8 : tunables 0 0 0 : slabdata 183 183 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2450 2509 2520 13 8 : tunables 0 0 0 : slabdata 193 193 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2450 2509 2520 13 8 : tunables 0 0 0 : slabdata 193 193 0 # cat /proc/slabinfo | egrep "^kmalloc-2048" kmalloc-2048 2450 2509 2520 13 8 : tunables 0 0 0 : slabdata 193 193 0
On vr, 2016-02-05 at 22:25 +0100, Dmitry Vyukov wrote: > On Fri, Feb 5, 2016 at 7:36 PM, Paul Bolle <pebolle@tiscali.nl> wrote: > > Does that make any difference? > Nope. > Almost 500 objects leaked in less than 10 seconds: Too bad. Still a nice (potential) clean up though. Thanks, Paul Bolle
--- a/drivers/isdn/gigaset/ser-gigaset.c +++ b/drivers/isdn/gigaset/ser-gigaset.c @@ -373,13 +373,9 @@ static void gigaset_freecshw(struct cardstate *cs) static void gigaset_device_release(struct device *dev) { - struct cardstate *cs = dev_get_drvdata(dev); - - if (!cs) - return; + struct ser_cardstate *scs = dev_get_drvdata(dev); dev_set_drvdata(dev, NULL); - kfree(cs->hw.ser); - cs->hw.ser = NULL; + kfree(scs); } /* @@ -408,7 +404,7 @@ static int gigaset_initcshw(struct cardstate *cs) cs->hw.ser = NULL; return rc; } - dev_set_drvdata(&cs->hw.ser->dev.dev, cs); + dev_set_drvdata(&cs->hw.ser->dev.dev, scs); tasklet_init(&cs->write_tasklet, gigaset_modem_fill, (unsigned long) cs);