Return-Path: <linux-kernel-owner+w=401wt.eu-S1754689AbZGSQCa@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1754689AbZGSQCa (ORCPT <rfc822;w@1wt.eu>);
	Sun, 19 Jul 2009 12:02:30 -0400
Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754535AbZGSQC3
	(ORCPT <rfc822;linux-kernel-outgoing>);
	Sun, 19 Jul 2009 12:02:29 -0400
Received: from cantor.suse.de ([195.135.220.2]:41503 "EHLO mx1.suse.de"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754533AbZGSQC2 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Sun, 19 Jul 2009 12:02:28 -0400
Date: Sun, 19 Jul 2009 18:02:27 +0200
Message-ID: <s5h1voclkz0.wl%tiwai@suse.de>
From: Takashi Iwai <tiwai@suse.de>
To: Jaswinder Singh Rajput <jaswinder@kernel.org>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>,
       Catalin Marinas <catalin.marinas@arm.com>,
       LKML <linux-kernel@vger.kernel.org>, Sam Ravnborg <sam@ravnborg.org>
Subject: Re: kmemleak issues in alsa
In-Reply-To: <1248017194.2548.3.camel@ht.satnam>
References: <1247861644.4513.15.camel@ht.satnam>
	<s5h4ot8op0b.wl%tiwai@suse.de>
	<1248017194.2548.3.camel@ht.satnam>
User-Agent: Wanderlust/2.12.0 (Your Wildest Dreams) SEMI/1.14.6 (Maruoka)
 FLIM/1.14.7 (=?ISO-8859-4?Q?Sanj=F2?=) APEL/10.6 Emacs/22.3
 (x86_64-suse-linux-gnu) MULE/5.0 (SAKAKI)
MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 5646
Lines: 143

At Sun, 19 Jul 2009 20:56:34 +0530,
Jaswinder Singh Rajput wrote:
> 
> Hello Takashi,
> 
> On Sun, 2009-07-19 at 14:07 +0200, Takashi Iwai wrote:
> > At Sat, 18 Jul 2009 01:44:04 +0530,
> > Jaswinder Singh Rajput wrote:
> > > 
> > > I was getting these kmemleak reports when I build SND_SEQUENCER in
> > > kernel :
> > > 
> > > unreferenced object 0xf6b0ac00 (size 512):
> > >   comm "swapper", pid 1, jiffies 4294671240
> > >   hex dump (first 32 bytes):
> > >     02 00 00 00 03 4f 53 53 20 73 65 71 75 65 6e 63  .....OSS sequenc
> > >     65 72 00 00 00 00 00 00 00 00 00 00 00 00 00 00  er..............
> > >   backtrace:
> > >     [<c12d7d83>] kmemleak_alloc+0x25/0x49
> > >     [<c109350d>] kmem_cache_alloc+0x95/0x124
> > >     [<c12568f0>] seq_create_client1+0x1d/0x165
> > >     [<c1257b01>] snd_seq_create_kernel_client+0x68/0xe7
> > >     [<c14c966f>] snd_seq_oss_create_client+0x86/0x143
> > >     [<c14c958a>] alsa_seq_oss_init+0xf8/0x157
> > >     [<c100104f>] do_one_initcall+0x4a/0x111
> > >     [<c14a8608>] kernel_init+0x16b/0x1bc
> > >     [<c100352b>] kernel_thread_helper+0x7/0x1a
> > >     [<ffffffff>] 0xffffffff
> > > unreferenced object 0xf6b08700 (size 128):
> > >   comm "swapper", pid 1, jiffies 4294671240
> > >   hex dump (first 32 bytes):
> > >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > >   backtrace:
> > >     [<c12d7d83>] kmemleak_alloc+0x25/0x49
> > >     [<c109350d>] kmem_cache_alloc+0x95/0x124
> > >     [<c12591a3>] snd_seq_pool_new+0x17/0xbe
> > >     [<c1256955>] seq_create_client1+0x82/0x165
> > >     [<c1257b01>] snd_seq_create_kernel_client+0x68/0xe7
> > >     [<c14c966f>] snd_seq_oss_create_client+0x86/0x143
> > >     [<c14c958a>] alsa_seq_oss_init+0xf8/0x157
> > >     [<c100104f>] do_one_initcall+0x4a/0x111
> > >     [<c14a8608>] kernel_init+0x16b/0x1bc
> > >     [<c100352b>] kernel_thread_helper+0x7/0x1a
> > >     [<ffffffff>] 0xffffffff
> > > unreferenced object 0xf6b0aa00 (size 512):
> > >   comm "swapper", pid 1, jiffies 4294671246
> > >   hex dump (first 32 bytes):
> > >     0f 00 00 00 00 00 00 00 52 65 63 65 69 76 65 72  ........Receiver
> > >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > >   backtrace:
> > >     [<c12d7d83>] kmemleak_alloc+0x25/0x49
> > >     [<c109350d>] kmem_cache_alloc+0x95/0x124
> > >     [<c125bdf9>] snd_seq_create_port+0x4c/0x197
> > >     [<c1257589>] snd_seq_ioctl_create_port+0x52/0x141
> > >     [<c1256bb3>] snd_seq_do_ioctl+0x59/0x78
> > >     [<c1256c11>] snd_seq_kernel_client_ctl+0x2f/0x48
> > >     [<c14c96e1>] snd_seq_oss_create_client+0xf8/0x143
> > >     [<c14c958a>] alsa_seq_oss_init+0xf8/0x157
> > >     [<c100104f>] do_one_initcall+0x4a/0x111
> > >     [<c14a8608>] kernel_init+0x16b/0x1bc
> > >     [<c100352b>] kernel_thread_helper+0x7/0x1a
> > >     [<ffffffff>] 0xffffffff
> > > 
> > > But when I build SND_SEQUENCER as module these kmemleak issues
> > > disappear.
> > > 
> > > Then I noticed that issue was in ordering. 
> > > 
> > > When build in kernel flow is like this :
> > > 
> > > OSS sequencer(alsa_seq_oss_init) -> System (snd_seq_system_client_init)
> > > 
> > > When build as modules flow is like this :
> > > 
> > > System(snd_seq_system_client_init) -> OSS sequencer(alsa_seq_oss_init)
> > > 
> > > So this fixes above kmemleak issues in my case, I hope it will be
> > > helpful :
> > > 
> > > diff --git a/sound/core/seq/oss/seq_oss.c b/sound/core/seq/oss/seq_oss.c
> > > index f25e3cc..3ddf2c2 100644
> > > --- a/sound/core/seq/oss/seq_oss.c
> > > +++ b/sound/core/seq/oss/seq_oss.c
> > > @@ -116,7 +116,7 @@ static void __exit alsa_seq_oss_exit(void)
> > >  	unregister_device();
> > >  }
> > >  
> > > -module_init(alsa_seq_oss_init)
> > > +late_initcall(alsa_seq_oss_init);
> > >  module_exit(alsa_seq_oss_exit)
> > 
> > Thanks for checking this.
> > 
> > Another option would be to fix Makefile.  I guess simply putting the
> > line including sound/core/seq/oss/Makefile after the definitions of
> > other seq modules would do the same thing...
> 
> Yes, this also fixes the ordering and memory leak when build
> SND_SEQUENCER in kernel and is equivalent to :
> 
> 1. insmod sound/core/seq/snd-seq-device.ko
> 2. insmod sound/core/seq/snd-seq.ko
> 3. insmod sound/core/seq/snd-seq-midi-event.ko
> 4. insmod sound/core/seq/oss/snd-seq-oss.ko
> 
> diff --git a/sound/core/seq/Makefile b/sound/core/seq/Makefile
> index 1bcb360..941f64a 100644
> --- a/sound/core/seq/Makefile
> +++ b/sound/core/seq/Makefile
> @@ -3,10 +3,6 @@
>  # Copyright (c) 1999 by Jaroslav Kysela <perex@perex.cz>
>  #
>  
> -ifeq ($(CONFIG_SND_SEQUENCER_OSS),y)
> -  obj-$(CONFIG_SND_SEQUENCER) += oss/
> -endif
> -
>  snd-seq-device-objs := seq_device.o
>  snd-seq-objs := seq.o seq_lock.o seq_clientmgr.o seq_memory.o seq_queue.o \
>                  seq_fifo.o seq_prioq.o seq_timer.o \
> @@ -19,7 +15,8 @@ snd-seq-virmidi-objs := seq_virmidi.o
>  
>  obj-$(CONFIG_SND_SEQUENCER) += snd-seq.o snd-seq-device.o
>  ifeq ($(CONFIG_SND_SEQUENCER_OSS),y)
> -obj-$(CONFIG_SND_SEQUENCER) += snd-seq-midi-event.o
> +  obj-$(CONFIG_SND_SEQUENCER) += snd-seq-midi-event.o
> +  obj-$(CONFIG_SND_SEQUENCER) += oss/
>  endif
>  obj-$(CONFIG_SND_SEQ_DUMMY) += snd-seq-dummy.o
>  

Looks good.  Could you give a changelog and your sign-off to merge?


thanks,

Takashi
--
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/