Messages in this thread | | | Date | Tue, 12 Feb 2008 17:06:30 +0900 | From | Yasunori Goto <> | Subject | Re: [-mm PATCH] register_memory/unregister_memory clean ups |
| |
> On Mon, 2008-02-11 at 11:48 -0800, Andrew Morton wrote: > > On Mon, 11 Feb 2008 09:23:18 -0800 > > Badari Pulavarty <pbadari@us.ibm.com> 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 never gets used). So, I cleaned > > > up the code and fixed the extra reference issue. > > > > > > Could you please include it in -mm ? > > > > > > Thanks, > > > Badari > > > > > > register_memory()/unregister_memory() never gets called with > > > "root". unregister_memory() is accessing kobject_name of > > > 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_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 nobody noticed > > it? > > > > No they don't crash machine - mainly because, they never get called > with "root" argument (where we have the bug). They were never tested > before, since we don't have memory remove work yet. All it does > is, it leave /sysfs directory laying around and causing next > memory add failure.
Badari-san.
Which function does call unregister_memory() or unregister_memory_section()? I can't find its caller in current 2.6.24-mm1.
???????() | |nothing calls? | +-->unregister_memory_section() | |call | +---> remove_memory_block() | |call | +----> unregister_memory()
unregister_memory_section() is only externed in linux/memory.h.
Do you have any another patch to call it? I think it is necessary for physical memory removing.
If you have not posted it or it is not merged to -mm, I can understand why this bug remains. If you posted it, could you point it to me?
Or do I misunderstand something?
Thanks.
-- Yasunori Goto
| |