Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-13 Thread Badari Pulavarty
On Wed, 2008-02-13 at 14:09 +0900, Yasunori Goto wrote: > Thanks Badari-san. > > I understand what was occured. :-) > > > On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: > > > > > + /* > > > > > +* Its ugly, but this is the best I can do - HELP !! > > > > > +* We don't know

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-13 Thread Badari Pulavarty
On Wed, 2008-02-13 at 14:09 +0900, Yasunori Goto wrote: Thanks Badari-san. I understand what was occured. :-) On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: + /* +* Its ugly, but this is the best I can do - HELP !! +* We don't know where the allocations

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Yasunori Goto
Thanks Badari-san. I understand what was occured. :-) > On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: > > > > + /* > > > > +* Its ugly, but this is the best I can do - HELP !! > > > > +* We don't know where the allocations for section memmap and usemap > > > > +* came

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Dave Hansen
On Tue, 2008-02-12 at 15:03 -0800, Badari Pulavarty wrote: > Here is the version with your suggestion. Do you like this better ? I do like how it looks, better, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Badari Pulavarty
On Tue, 2008-02-12 at 14:15 -0800, Dave Hansen wrote: > On Tue, 2008-02-12 at 14:07 -0800, Badari Pulavarty wrote: > > On Tue, 2008-02-12 at 13:57 -0800, Dave Hansen wrote: > > > On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: > > > > > > > > +static void __remove_section(struct zone

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Badari Pulavarty
On Tue, 2008-02-12 at 14:15 -0800, Dave Hansen wrote: > On Tue, 2008-02-12 at 14:07 -0800, Badari Pulavarty wrote: > > On Tue, 2008-02-12 at 13:57 -0800, Dave Hansen wrote: > > > On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: > > > > > > > > +static void __remove_section(struct zone

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Dave Hansen
On Tue, 2008-02-12 at 14:07 -0800, Badari Pulavarty wrote: > On Tue, 2008-02-12 at 13:57 -0800, Dave Hansen wrote: > > On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: > > > > > > +static void __remove_section(struct zone *zone, unsigned long > > > section_nr) > > > +{ > > > + if

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Dave Hansen
On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: > > > + /* > > > +* Its ugly, but this is the best I can do - HELP !! > > > +* We don't know where the allocations for section memmap and usemap > > > +* came from. If they are allocated at the boot time, they would come > >

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Badari Pulavarty
On Tue, 2008-02-12 at 13:57 -0800, Dave Hansen wrote: > On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: > > > > +static void __remove_section(struct zone *zone, unsigned long > > section_nr) > > +{ > > + if (!valid_section_nr(section_nr)) > > + return; > > + > > +

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Dave Hansen
On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: > > +static void __remove_section(struct zone *zone, unsigned long > section_nr) > +{ > + if (!valid_section_nr(section_nr)) > + return; > + > + unregister_memory_section(__nr_to_section(section_nr)); > +

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Badari Pulavarty
On Tue, 2008-02-12 at 12:59 -0800, Dave Hansen wrote: > On Tue, 2008-02-12 at 09:22 -0800, Badari Pulavarty wrote: > > +static void __remove_section(struct zone *zone, unsigned long > > phys_start_pfn) > > +{ > > + if (!pfn_valid(phys_start_pfn)) > > + return; > > I think you need at

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Dave Hansen
On Tue, 2008-02-12 at 09:22 -0800, Badari Pulavarty wrote: > +static void __remove_section(struct zone *zone, unsigned long phys_start_pfn) > +{ > + if (!pfn_valid(phys_start_pfn)) > + return; I think you need at least a WARN_ON() there. I'd probably also not use pfn_valid(),

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Badari Pulavarty
ry() static. > > > > > > > > Another bug fix - before calling unregister_memory() > > > > remove_memory_block() gets a ref on kobject. unregister_memory() > > > > need to drop that ref before calling sysdev_unregister(). > > > > > > >

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Yasunori Goto
gt; > the object just freed up. Since no one uses the code, > > > lets take the code out. And also, make register_memory() static. > > > > > > Another bug fix - before calling unregister_memory() > > > remove_memory_block() gets a ref on kobject. unregister_me

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Yasunori Goto
. unregister_memory() need to drop that ref before calling sysdev_unregister(). I'd say this: Subject: [-mm PATCH] register_memory/unregister_memory clean ups is rather tame. These are more than cleanups! These sound like machine-crashing bugs. Do they crash machines? How

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Yasunori Goto
Thanks Badari-san. I understand what was occured. :-) On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: + /* +* Its ugly, but this is the best I can do - HELP !! +* We don't know where the allocations for section memmap and usemap +* came from. If they are

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Dave Hansen
On Tue, 2008-02-12 at 15:03 -0800, Badari Pulavarty wrote: Here is the version with your suggestion. Do you like this better ? I do like how it looks, better, thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Dave Hansen
On Tue, 2008-02-12 at 14:07 -0800, Badari Pulavarty wrote: On Tue, 2008-02-12 at 13:57 -0800, Dave Hansen wrote: On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: +static void __remove_section(struct zone *zone, unsigned long section_nr) +{ + if

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Dave Hansen
On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: + /* +* Its ugly, but this is the best I can do - HELP !! +* We don't know where the allocations for section memmap and usemap +* came from. If they are allocated at the boot time, they would come +* from

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Badari Pulavarty
On Tue, 2008-02-12 at 13:57 -0800, Dave Hansen wrote: On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: +static void __remove_section(struct zone *zone, unsigned long section_nr) +{ + if (!valid_section_nr(section_nr)) + return; + +

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Dave Hansen
On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: +static void __remove_section(struct zone *zone, unsigned long section_nr) +{ + if (!valid_section_nr(section_nr)) + return; + + unregister_memory_section(__nr_to_section(section_nr)); +

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Dave Hansen
On Tue, 2008-02-12 at 09:22 -0800, Badari Pulavarty wrote: +static void __remove_section(struct zone *zone, unsigned long phys_start_pfn) +{ + if (!pfn_valid(phys_start_pfn)) + return; I think you need at least a WARN_ON() there. I'd probably also not use pfn_valid(),

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Badari Pulavarty
On Tue, 2008-02-12 at 14:15 -0800, Dave Hansen wrote: On Tue, 2008-02-12 at 14:07 -0800, Badari Pulavarty wrote: On Tue, 2008-02-12 at 13:57 -0800, Dave Hansen wrote: On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote: +static void __remove_section(struct zone *zone, unsigned

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Badari Pulavarty
calling unregister_memory() remove_memory_block() gets a ref on kobject. unregister_memory() need to drop that ref before calling sysdev_unregister(). I'd say this: Subject: [-mm PATCH] register_memory/unregister_memory clean ups is rather tame. These are more

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-12 Thread Badari Pulavarty
On Tue, 2008-02-12 at 12:59 -0800, Dave Hansen wrote: On Tue, 2008-02-12 at 09:22 -0800, Badari Pulavarty wrote: +static void __remove_section(struct zone *zone, unsigned long phys_start_pfn) +{ + if (!pfn_valid(phys_start_pfn)) + return; I think you need at least a

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-11 Thread Badari Pulavarty
nd also, make register_memory() static. > > > > Another bug fix - before calling unregister_memory() > > remove_memory_block() gets a ref on kobject. unregister_memory() > > need to drop that ref before calling sysdev_unregister(). > > > > I'd say this: > &

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-11 Thread Greg KH
lets take the code out. And also, make register_memory() static. > > > > Another bug fix - before calling unregister_memory() > > remove_memory_block() gets a ref on kobject. unregister_memory() > > need to drop that ref before calling sysdev_unregister(). > > > > I'd say this: &g

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-11 Thread Andrew Morton
unregister_memory() > need to drop that ref before calling sysdev_unregister(). > I'd say this: > Subject: [-mm PATCH] register_memory/unregister_memory clean ups is rather tame. These are more than cleanups! These sound like machine-crashing bugs. Do they crash machines? How come nobo

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-11 Thread Badari Pulavarty
On Mon, 2008-02-11 at 09:54 -0800, Greg KH wrote: > On Mon, Feb 11, 2008 at 09:23:18AM -0800, Badari Pulavarty wrote: > > Hi Andrew, > > > > While testing hotplug memory remove against -mm, I noticed > > that unregister_memory() is not cleaning up /sysfs entries > > correctly. It also

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-11 Thread Greg KH
On Mon, Feb 11, 2008 at 09:23:18AM -0800, Badari Pulavarty wrote: > Hi Andrew, > > While testing hotplug memory remove against -mm, I noticed > that unregister_memory() is not cleaning up /sysfs entries > correctly. It also de-references structures after destroying > them (luckily in the code

[-mm PATCH] register_memory/unregister_memory clean ups

2008-02-11 Thread Badari Pulavarty
Hi Andrew, While testing hotplug memory remove against -mm, I noticed that unregister_memory() is not cleaning up /sysfs entries correctly. It also de-references structures after destroying them (luckily in the code which never gets used). So, I cleaned up the code and fixed the extra reference

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-11 Thread Andrew Morton
: [-mm PATCH] register_memory/unregister_memory clean ups is rather tame. These are more than cleanups! These sound like machine-crashing bugs. Do they crash machines? How come nobody noticed it? All very strange... -- To unsubscribe from this list: send the line unsubscribe linux-kernel

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-11 Thread Badari Pulavarty
On Mon, 2008-02-11 at 09:54 -0800, Greg KH wrote: On Mon, Feb 11, 2008 at 09:23:18AM -0800, Badari Pulavarty wrote: Hi Andrew, While testing hotplug memory remove against -mm, I noticed that unregister_memory() is not cleaning up /sysfs entries correctly. It also de-references

[-mm PATCH] register_memory/unregister_memory clean ups

2008-02-11 Thread Badari Pulavarty
Hi Andrew, While testing hotplug memory remove against -mm, I noticed that unregister_memory() is not cleaning up /sysfs entries correctly. It also de-references structures after destroying them (luckily in the code which never gets used). So, I cleaned up the code and fixed the extra reference

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-11 Thread Badari Pulavarty
to drop that ref before calling sysdev_unregister(). I'd say this: Subject: [-mm PATCH] register_memory/unregister_memory clean ups is rather tame. These are more than cleanups! These sound like machine-crashing bugs. Do they crash machines? How come nobody noticed

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-11 Thread Greg KH
() need to drop that ref before calling sysdev_unregister(). I'd say this: Subject: [-mm PATCH] register_memory/unregister_memory clean ups is rather tame. These are more than cleanups! These sound like machine-crashing bugs. Do they crash machines? How come nobody noticed

Re: [-mm PATCH] register_memory/unregister_memory clean ups

2008-02-11 Thread Greg KH
On Mon, Feb 11, 2008 at 09:23:18AM -0800, Badari Pulavarty wrote: Hi Andrew, While testing hotplug memory remove against -mm, I noticed that unregister_memory() is not cleaning up /sysfs entries correctly. It also de-references structures after destroying them (luckily in the code which