Re: [PATCH] ALSA: remove magic cast (0/6)

Re: [PATCH] ALSA: remove magic cast (0/6)

[Jaroslav Kysela]

On Mon, 14 Jun 2004, Takashi Iwai wrote:

> > This patch remove snd_magic_cast from the kernel.  It's on top of the
> > kcalloc() patch and was compile-tested with allyesconfig.
> 
> Thanks for the patch.
> 
> Did you already make a patch to remove hidden_kmalloc and
> hidden_vmalloc wrappers, or do I miss it?  They are also suprefluous.
> 
> Jaroslav, what do you think about removing kmalloc/vmalloc wrappers
> and magic stuffs from the kernel tree?
> 
> For me, it's ok to remove them from the linux kernel tree
> (ie. alsa-kernel tree).  I agree that they are not needed for the
> matured drivers.  For the experimental drivers, We can provide another
> type check mechanism in alsa-driver tree locally.

I think that we need to separate the patch:

1) removal of own memory pool (check for memory leaks)
2) magic stuff

The structure mistakes usually ends with an oops, but the memory leaks 
cannot be detected easy inside the kernel.

I don't like too much the idea to remove the own memory pool in the debug
mode. It helped me in past numerous times and I think that ALSA is one
from the clean subsystems in this area. I don't understand the kernel
developers - there is no replacement. Yes, it's good only for debugging,
but why not make debugging easier for other developers as well with
proposing a general solution?

Regarding the magic stuffs - I do not care so much. It can be removed now.

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SUSE Labs


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: [PATCH] ALSA: remove magic cast (0/6)

[Takashi Iwai]

At Mon, 14 Jun 2004 20:18:31 +0200 (CEST),
Jaroslav wrote:
> 
> On Mon, 14 Jun 2004, Takashi Iwai wrote:
> 
> > > This patch remove snd_magic_cast from the kernel.  It's on top of the
> > > kcalloc() patch and was compile-tested with allyesconfig.
> > 
> > Thanks for the patch.
> > 
> > Did you already make a patch to remove hidden_kmalloc and
> > hidden_vmalloc wrappers, or do I miss it?  They are also suprefluous.
> > 
> > Jaroslav, what do you think about removing kmalloc/vmalloc wrappers
> > and magic stuffs from the kernel tree?
> > 
> > For me, it's ok to remove them from the linux kernel tree
> > (ie. alsa-kernel tree).  I agree that they are not needed for the
> > matured drivers.  For the experimental drivers, We can provide another
> > type check mechanism in alsa-driver tree locally.
> 
> I think that we need to separate the patch:
> 
> 1) removal of own memory pool (check for memory leaks)
> 2) magic stuff
> 
> The structure mistakes usually ends with an oops, but the memory leaks 
> cannot be detected easy inside the kernel.

IMO, the memory leak check should be better in slab rather than in
ALSA wrapper.  I understand that has annoyed people.

Having ALSA-specific isn't always nice, for example, we have to have
kfree() and kfree_nocheck(), and this has triggered some bugs.

About vmalloc(), we need to reconsider the memory check, too.  Many
routines assume that the vmalloc data is aligned to the page, but the
hidden header breaks this.


> I don't like too much the idea to remove the own memory pool in the debug
> mode. It helped me in past numerous times and I think that ALSA is one
> from the clean subsystems in this area. I don't understand the kernel
> developers - there is no replacement. Yes, it's good only for debugging,
> but why not make debugging easier for other developers as well with
> proposing a general solution?

Agreed.  The memory leak check is useful.
A general solution would be better...


Takashi


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: ALSA: remove magic cast (0/6)

[Takashi Iwai]

At Tue, 15 Jun 2004 09:38:45 +0300,
Pekka J Enberg wrote:
> 
> Hi, 
> 
> Jaroslav Kysela writes:
> > I think that we need to separate the patch: 
> > 
> > 1) removal of own memory pool (check for memory leaks)
> > 2) magic stuff
> 
> I only replaced snd_kcalloc() and snd_magic_kcalloc() with the new generic 
> kcalloc() and removed snd_magic_cast so the kmalloc wrappers are still in 
> place. 

I agree with Jaroslav.   To make the things easy, let's split the
patch.

1. addition of kcalloc()

2. replacement of snd_kcalloc with kcalloc

3. removal of magic stuff
   - replace snd_magic_kmalloc/kcalloc to kmalloc/kcalloc.
   - replace snd_magic_kfree to kfree.
   - replace snd_magic_cast to normal cast.
   - remove the rest of magic-related codes/files.


Then only the memory tracking (called memory pool in above) will remain.

> Jaroslav Kysela writes:
> > I don't like too much the idea to remove the own memory pool in the debug
> > mode. It helped me in past numerous times and I think that ALSA is one
> > from the clean subsystems in this area. I don't understand the kernel
> > developers - there is no replacement. Yes, it's good only for debugging,
> > but why not make debugging easier for other developers as well with
> > proposing a general solution?
> 
> What memory pool? To me, it looks like sound/core/memory.c just maintains a 
> list of kmallocs and vmallocs we check when snd_memory_done() is called. 

Jaroslav mentioned this, too.

> Perhaps we could make it somehow automagically per-module for debug builds 
> to make it generic? Memory leak check could be done at module_exit. 

I thought of a similar thing.  It will be more useful to make it
generic.

For now, until the generic debug code like the above is implmented in
the generic slab/kmalloc layer, we can keep it inside the ALSA.
We can safely clean up the other stuff beforehand.


How about this?


Takashi


-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - 
digital self defense, top technical experts, no vendor pitches, 
unmatched networking opportunities. Visit www.blackhat.com

Re: ALSA: remove magic cast (0/6)

[Takashi Iwai]

At Tue, 22 Jun 2004 08:51:01 +0300,
Pekka J Enberg wrote:
> 
> Takashi Iwai writes:
> > I agree with Jaroslav.   To make the things easy, let's split the
> > patch. 
> > 
> > 1. addition of kcalloc() 
> > 
> > 2. replacement of snd_kcalloc with kcalloc 
> > 
> > 3. removal of magic stuff
> >    - replace snd_magic_kmalloc/kcalloc to kmalloc/kcalloc.
> >    - replace snd_magic_kfree to kfree.
> >    - replace snd_magic_cast to normal cast.
> >    - remove the rest of magic-related codes/files. 
> > 
> > Then only the memory tracking (called memory pool in above) will remain.
> 
> Sounds good. It's almost like this already. I just need to split the 
> snd_kcalloc and snd_magic_kcalloc sweep. 

Or, could you give your magic script, so that we can apply on our
tree?  There have been already many changes since 2.6.7...


Takashi


-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - 
digital self defense, top technical experts, no vendor pitches, 
unmatched networking opportunities. Visit www.blackhat.com