[PATCH -mm] module: fix __find_symbl() error checks
The patch: http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.24/2.6.24-mm1/broken-out/modules-handle-symbols-that-have-a-zero-value.patch changes the return value of __find_symbol() so that it can handle symbols that are zero and now it returns -errno for failure. But some __find_symbl() error checks remain unchanged. Signed-off-by: Akinobu Mita [EMAIL PROTECTED] Cc: Christoph Lameter [EMAIL PROTECTED] --- kernel/module.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) Index: 2.6.24-mm1/kernel/module.c === --- 2.6.24-mm1.orig/kernel/module.c +++ 2.6.24-mm1/kernel/module.c @@ -977,7 +977,7 @@ static unsigned long resolve_symbol(Elf_ ret = __find_symbol(name, owner, crc, !(mod-taints TAINT_PROPRIETARY_MODULE)); - if (ret) { + if (!IS_ERR_VALUE(ret)) { /* use_module can fail due to OOM, or module initialization or unloading */ if (!check_version(sechdrs, versindex, name, mod, crc) || @@ -1370,7 +1370,9 @@ void *__symbol_get(const char *symbol) preempt_disable(); value = __find_symbol(symbol, owner, crc, 1); - if (value strong_try_module_get(owner) != 0) + if (IS_ERR_VALUE(value)) + value = 0; + else if (strong_try_module_get(owner)) value = 0; preempt_enable(); -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] xip: fix get_zeroed_page with __GFP_HIGHMEM
The use of get_zeroed_page() with __GFP_HIGHMEM is invalid. Use alloc_page() with __GFP_ZERO instead of invalid get_zeroed_page(). (This patch is only compile tested) Cc: Carsten Otte <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- mm/filemap_xip.c |9 + 1 file changed, 5 insertions(+), 4 deletions(-) Index: 2.6-git/mm/filemap_xip.c === --- 2.6-git.orig/mm/filemap_xip.c +++ 2.6-git/mm/filemap_xip.c @@ -25,14 +25,15 @@ static struct page *__xip_sparse_page; static struct page *xip_sparse_page(void) { if (!__xip_sparse_page) { - unsigned long zeroes = get_zeroed_page(GFP_HIGHUSER); - if (zeroes) { + struct page *page = alloc_page(GFP_HIGHUSER | __GFP_ZERO); + + if (page) { static DEFINE_SPINLOCK(xip_alloc_lock); spin_lock(_alloc_lock); if (!__xip_sparse_page) - __xip_sparse_page = virt_to_page(zeroes); + __xip_sparse_page = page; else - free_page(zeroes); + __free_page(page); spin_unlock(_alloc_lock); } } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] xip: fix get_zeroed_page with __GFP_HIGHMEM
The use of get_zeroed_page() with __GFP_HIGHMEM is invalid. Use alloc_page() with __GFP_ZERO instead of invalid get_zeroed_page(). (This patch is only compile tested) Cc: Carsten Otte [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- mm/filemap_xip.c |9 + 1 file changed, 5 insertions(+), 4 deletions(-) Index: 2.6-git/mm/filemap_xip.c === --- 2.6-git.orig/mm/filemap_xip.c +++ 2.6-git/mm/filemap_xip.c @@ -25,14 +25,15 @@ static struct page *__xip_sparse_page; static struct page *xip_sparse_page(void) { if (!__xip_sparse_page) { - unsigned long zeroes = get_zeroed_page(GFP_HIGHUSER); - if (zeroes) { + struct page *page = alloc_page(GFP_HIGHUSER | __GFP_ZERO); + + if (page) { static DEFINE_SPINLOCK(xip_alloc_lock); spin_lock(xip_alloc_lock); if (!__xip_sparse_page) - __xip_sparse_page = virt_to_page(zeroes); + __xip_sparse_page = page; else - free_page(zeroes); + __free_page(page); spin_unlock(xip_alloc_lock); } } -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs: use list_for_each_entry_reverse and kill sb_entry
Use list_for_each_entry_reverse for super_blocks list and remove unused sb_entry macro. Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- fs/fs-writeback.c |7 ++- include/linux/fs.h |1 - 2 files changed, 2 insertions(+), 6 deletions(-) Index: 2.6-mm/fs/fs-writeback.c === --- 2.6-mm.orig/fs/fs-writeback.c +++ 2.6-mm/fs/fs-writeback.c @@ -528,8 +528,7 @@ int writeback_inodes(struct writeback_co might_sleep(); spin_lock(_lock); restart: - sb = sb_entry(super_blocks.prev); - for (; sb != sb_entry(_blocks); sb = sb_entry(sb->s_list.prev)) { + list_for_each_entry_reverse(sb, _blocks, s_list) { if (sb_has_dirty_inodes(sb)) { /* we're making our own get_super here */ sb->s_count++; @@ -593,10 +592,8 @@ static void set_sb_syncing(int val) { struct super_block *sb; spin_lock(_lock); - sb = sb_entry(super_blocks.prev); - for (; sb != sb_entry(_blocks); sb = sb_entry(sb->s_list.prev)) { + list_for_each_entry_reverse(sb, _blocks, s_list) sb->s_syncing = val; - } spin_unlock(_lock); } Index: 2.6-mm/include/linux/fs.h === --- 2.6-mm.orig/include/linux/fs.h +++ 2.6-mm/include/linux/fs.h @@ -984,7 +984,6 @@ extern int send_sigurg(struct fown_struc extern struct list_head super_blocks; extern spinlock_t sb_lock; -#define sb_entry(list) list_entry((list), struct super_block, s_list) #define S_BIAS (1<<30) struct super_block { struct list_heads_list; /* Keep this first */ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] partitions: use kasprintf
Use kasprintf instead of kmalloc()-strcpy()-strcat(). Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- fs/partitions/check.c |7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) Index: 2.6-mm/fs/partitions/check.c === --- 2.6-mm.orig/fs/partitions/check.c +++ 2.6-mm/fs/partitions/check.c @@ -453,16 +453,11 @@ void add_partition(struct gendisk *disk, static char *make_block_name(struct gendisk *disk) { char *name; - static char *block_str = "block:"; - int size; char *s; - size = strlen(block_str) + strlen(disk->disk_name) + 1; - name = kmalloc(size, GFP_KERNEL); + name = kasprintf(GFP_KERNEL, "block:%s", disk->disk_name); if (!name) return NULL; - strcpy(name, block_str); - strcat(name, disk->disk_name); /* ewww... some of these buggers have / in name... */ s = strchr(name, '/'); if (s) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] power: use kasprintf
Use kasprintf instead of kmalloc()-strcpy()-strcat(). Cc: Anton Vorontsov <[EMAIL PROTECTED]> Cc: David Woodhouse <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- drivers/power/power_supply_leds.c | 25 +++-- 1 file changed, 7 insertions(+), 18 deletions(-) Index: 2.6-mm/drivers/power/power_supply_leds.c === --- 2.6-mm.orig/drivers/power/power_supply_leds.c +++ 2.6-mm/drivers/power/power_supply_leds.c @@ -10,6 +10,7 @@ * You may use this code as per GPL version 2 */ +#include #include #include "power_supply.h" @@ -48,28 +49,20 @@ static int power_supply_create_bat_trigg { int rc = 0; - psy->charging_full_trig_name = kmalloc(strlen(psy->name) + - sizeof("-charging-or-full"), GFP_KERNEL); + psy->charging_full_trig_name = kasprintf(GFP_KERNEL, + "%s-charging-or-full", psy->name); if (!psy->charging_full_trig_name) goto charging_full_failed; - psy->charging_trig_name = kmalloc(strlen(psy->name) + - sizeof("-charging"), GFP_KERNEL); + psy->charging_trig_name = kasprintf(GFP_KERNEL, + "%s-charging", psy->name); if (!psy->charging_trig_name) goto charging_failed; - psy->full_trig_name = kmalloc(strlen(psy->name) + - sizeof("-full"), GFP_KERNEL); + psy->full_trig_name = kasprintf(GFP_KERNEL, "%s-full", psy->name); if (!psy->full_trig_name) goto full_failed; - strcpy(psy->charging_full_trig_name, psy->name); - strcat(psy->charging_full_trig_name, "-charging-or-full"); - strcpy(psy->charging_trig_name, psy->name); - strcat(psy->charging_trig_name, "-charging"); - strcpy(psy->full_trig_name, psy->name); - strcat(psy->full_trig_name, "-full"); - led_trigger_register_simple(psy->charging_full_trig_name, >charging_full_trig); led_trigger_register_simple(psy->charging_trig_name, @@ -120,14 +113,10 @@ static int power_supply_create_gen_trigg { int rc = 0; - psy->online_trig_name = kmalloc(strlen(psy->name) + sizeof("-online"), - GFP_KERNEL); + psy->online_trig_name = kasprintf(GFP_KERNEL, "%s-online", psy->name); if (!psy->online_trig_name) goto online_failed; - strcpy(psy->online_trig_name, psy->name); - strcat(psy->online_trig_name, "-online"); - led_trigger_register_simple(psy->online_trig_name, >online_trig); goto success; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] power: use kasprintf
Use kasprintf instead of kmalloc()-strcpy()-strcat(). Cc: Anton Vorontsov [EMAIL PROTECTED] Cc: David Woodhouse [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- drivers/power/power_supply_leds.c | 25 +++-- 1 file changed, 7 insertions(+), 18 deletions(-) Index: 2.6-mm/drivers/power/power_supply_leds.c === --- 2.6-mm.orig/drivers/power/power_supply_leds.c +++ 2.6-mm/drivers/power/power_supply_leds.c @@ -10,6 +10,7 @@ * You may use this code as per GPL version 2 */ +#include linux/kernel.h #include linux/power_supply.h #include power_supply.h @@ -48,28 +49,20 @@ static int power_supply_create_bat_trigg { int rc = 0; - psy-charging_full_trig_name = kmalloc(strlen(psy-name) + - sizeof(-charging-or-full), GFP_KERNEL); + psy-charging_full_trig_name = kasprintf(GFP_KERNEL, + %s-charging-or-full, psy-name); if (!psy-charging_full_trig_name) goto charging_full_failed; - psy-charging_trig_name = kmalloc(strlen(psy-name) + - sizeof(-charging), GFP_KERNEL); + psy-charging_trig_name = kasprintf(GFP_KERNEL, + %s-charging, psy-name); if (!psy-charging_trig_name) goto charging_failed; - psy-full_trig_name = kmalloc(strlen(psy-name) + - sizeof(-full), GFP_KERNEL); + psy-full_trig_name = kasprintf(GFP_KERNEL, %s-full, psy-name); if (!psy-full_trig_name) goto full_failed; - strcpy(psy-charging_full_trig_name, psy-name); - strcat(psy-charging_full_trig_name, -charging-or-full); - strcpy(psy-charging_trig_name, psy-name); - strcat(psy-charging_trig_name, -charging); - strcpy(psy-full_trig_name, psy-name); - strcat(psy-full_trig_name, -full); - led_trigger_register_simple(psy-charging_full_trig_name, psy-charging_full_trig); led_trigger_register_simple(psy-charging_trig_name, @@ -120,14 +113,10 @@ static int power_supply_create_gen_trigg { int rc = 0; - psy-online_trig_name = kmalloc(strlen(psy-name) + sizeof(-online), - GFP_KERNEL); + psy-online_trig_name = kasprintf(GFP_KERNEL, %s-online, psy-name); if (!psy-online_trig_name) goto online_failed; - strcpy(psy-online_trig_name, psy-name); - strcat(psy-online_trig_name, -online); - led_trigger_register_simple(psy-online_trig_name, psy-online_trig); goto success; - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] partitions: use kasprintf
Use kasprintf instead of kmalloc()-strcpy()-strcat(). Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- fs/partitions/check.c |7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) Index: 2.6-mm/fs/partitions/check.c === --- 2.6-mm.orig/fs/partitions/check.c +++ 2.6-mm/fs/partitions/check.c @@ -453,16 +453,11 @@ void add_partition(struct gendisk *disk, static char *make_block_name(struct gendisk *disk) { char *name; - static char *block_str = block:; - int size; char *s; - size = strlen(block_str) + strlen(disk-disk_name) + 1; - name = kmalloc(size, GFP_KERNEL); + name = kasprintf(GFP_KERNEL, block:%s, disk-disk_name); if (!name) return NULL; - strcpy(name, block_str); - strcat(name, disk-disk_name); /* ewww... some of these buggers have / in name... */ s = strchr(name, '/'); if (s) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs: use list_for_each_entry_reverse and kill sb_entry
Use list_for_each_entry_reverse for super_blocks list and remove unused sb_entry macro. Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- fs/fs-writeback.c |7 ++- include/linux/fs.h |1 - 2 files changed, 2 insertions(+), 6 deletions(-) Index: 2.6-mm/fs/fs-writeback.c === --- 2.6-mm.orig/fs/fs-writeback.c +++ 2.6-mm/fs/fs-writeback.c @@ -528,8 +528,7 @@ int writeback_inodes(struct writeback_co might_sleep(); spin_lock(sb_lock); restart: - sb = sb_entry(super_blocks.prev); - for (; sb != sb_entry(super_blocks); sb = sb_entry(sb-s_list.prev)) { + list_for_each_entry_reverse(sb, super_blocks, s_list) { if (sb_has_dirty_inodes(sb)) { /* we're making our own get_super here */ sb-s_count++; @@ -593,10 +592,8 @@ static void set_sb_syncing(int val) { struct super_block *sb; spin_lock(sb_lock); - sb = sb_entry(super_blocks.prev); - for (; sb != sb_entry(super_blocks); sb = sb_entry(sb-s_list.prev)) { + list_for_each_entry_reverse(sb, super_blocks, s_list) sb-s_syncing = val; - } spin_unlock(sb_lock); } Index: 2.6-mm/include/linux/fs.h === --- 2.6-mm.orig/include/linux/fs.h +++ 2.6-mm/include/linux/fs.h @@ -984,7 +984,6 @@ extern int send_sigurg(struct fown_struc extern struct list_head super_blocks; extern spinlock_t sb_lock; -#define sb_entry(list) list_entry((list), struct super_block, s_list) #define S_BIAS (130) struct super_block { struct list_heads_list; /* Keep this first */ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs: use hlist_unhashed
Use hlist_unhashed() instead of opencoded equivalent. Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- fs/dcache.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: 2.6-mm/fs/dcache.c === --- 2.6-mm.orig/fs/dcache.c +++ 2.6-mm/fs/dcache.c @@ -89,7 +89,7 @@ static void d_free(struct dentry *dentry if (dentry->d_op && dentry->d_op->d_release) dentry->d_op->d_release(dentry); /* if dentry was never inserted into hash, immediate free is OK */ - if (dentry->d_hash.pprev == NULL) + if (hlist_unhashed(>d_hash)) __d_free(dentry); else call_rcu(>d_u.d_rcu, d_callback); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs: use hlist_unhashed
Use hlist_unhashed() instead of opencoded equivalent. Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- fs/dcache.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: 2.6-mm/fs/dcache.c === --- 2.6-mm.orig/fs/dcache.c +++ 2.6-mm/fs/dcache.c @@ -89,7 +89,7 @@ static void d_free(struct dentry *dentry if (dentry-d_op dentry-d_op-d_release) dentry-d_op-d_release(dentry); /* if dentry was never inserted into hash, immediate free is OK */ - if (dentry-d_hash.pprev == NULL) + if (hlist_unhashed(dentry-d_hash)) __d_free(dentry); else call_rcu(dentry-d_u.d_rcu, d_callback); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] slab: fix typo in allocation failure handling
This patch fixes wrong array index in allocation failure handling. Cc: Pekka Enberg <[EMAIL PROTECTED]> Cc: Christoph Lameter <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- mm/slab.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: 2.6-git/mm/slab.c === --- 2.6-git.orig/mm/slab.c +++ 2.6-git/mm/slab.c @@ -1043,7 +1043,7 @@ static struct array_cache **alloc_alien_ } ac_ptr[i] = alloc_arraycache(node, limit, 0xbaadf00d); if (!ac_ptr[i]) { - for (i--; i <= 0; i--) + for (i--; i >= 0; i--) kfree(ac_ptr[i]); kfree(ac_ptr); return NULL; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: fix cpu-hotplug regression
> [PATCH] x86: fix cpu hotplug regression (don't call mce_create_device on > CPU_UP_PREPARE) > > Fix regression introduced with d435d862baca3e25e5eec236762a43251b1e7ffc > ("cpu hotplug: mce: fix cpu hotplug error handling"). > > For CPUs not brought up during boot (using maxcpus and additional_cpus > parameters) we don't know whether mce is supported or not at > "CPU_UP_PREPARE"-time. > Thus mce_cpu_callback should be called after the CPU is online. Thank you for finding and fixing the problem. I added two fixes to your patch: - Avoid mce_remove_device() for the CPU that is not correctly initialized by mce_create_device() failure. - make CPU_ONLINE callback always return NOTIFY_OK. Because CPU_ONLINE callback return value is always ignored. > Signed-off-by: Andreas Herrmann <[EMAIL PROTECTED]> [EMAIL PROTECTED]: make CPU_ONLINE callback always return NOTIFY_OK] [EMAIL PROTECTED]: avoid mce_remove_device() for not initialized device] Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- arch/x86/kernel/cpu/mcheck/mce_64.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) Index: 2.6-git/arch/x86/kernel/cpu/mcheck/mce_64.c === --- 2.6-git.orig/arch/x86/kernel/cpu/mcheck/mce_64.c +++ 2.6-git/arch/x86/kernel/cpu/mcheck/mce_64.c @@ -802,6 +802,8 @@ static struct sysdev_attribute *mce_attr NULL }; +static cpumask_t mce_device_initialized = CPU_MASK_NONE; + /* Per cpu sysdev init. All of the cpus still share the same ctl bank */ static __cpuinit int mce_create_device(unsigned int cpu) { @@ -825,6 +827,7 @@ static __cpuinit int mce_create_device(u if (err) goto error; } + cpu_set(cpu, mce_device_initialized); return 0; error: @@ -841,10 +844,14 @@ static void mce_remove_device(unsigned i { int i; + if (!cpu_isset(cpu, mce_device_initialized)) + return; + for (i = 0; mce_attributes[i]; i++) sysdev_remove_file(_cpu(device_mce,cpu), mce_attributes[i]); sysdev_unregister(_cpu(device_mce,cpu)); + cpu_clear(cpu, mce_device_initialized); } /* Get notified when a cpu comes on/off. Be hotplug friendly. */ @@ -852,21 +859,18 @@ static int mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; - int err = 0; switch (action) { - case CPU_UP_PREPARE: - case CPU_UP_PREPARE_FROZEN: - err = mce_create_device(cpu); + case CPU_ONLINE: + case CPU_ONLINE_FROZEN: + mce_create_device(cpu); break; - case CPU_UP_CANCELED: - case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: mce_remove_device(cpu); break; } - return err ? NOTIFY_BAD : NOTIFY_OK; + return NOTIFY_OK; } static struct notifier_block mce_cpu_notifier = { - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] slab: fix typo in allocation failure handling
This patch fixes wrong array index in allocation failure handling. Cc: Pekka Enberg [EMAIL PROTECTED] Cc: Christoph Lameter [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- mm/slab.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: 2.6-git/mm/slab.c === --- 2.6-git.orig/mm/slab.c +++ 2.6-git/mm/slab.c @@ -1043,7 +1043,7 @@ static struct array_cache **alloc_alien_ } ac_ptr[i] = alloc_arraycache(node, limit, 0xbaadf00d); if (!ac_ptr[i]) { - for (i--; i = 0; i--) + for (i--; i = 0; i--) kfree(ac_ptr[i]); kfree(ac_ptr); return NULL; - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: fix cpu-hotplug regression
[PATCH] x86: fix cpu hotplug regression (don't call mce_create_device on CPU_UP_PREPARE) Fix regression introduced with d435d862baca3e25e5eec236762a43251b1e7ffc (cpu hotplug: mce: fix cpu hotplug error handling). For CPUs not brought up during boot (using maxcpus and additional_cpus parameters) we don't know whether mce is supported or not at CPU_UP_PREPARE-time. Thus mce_cpu_callback should be called after the CPU is online. Thank you for finding and fixing the problem. I added two fixes to your patch: - Avoid mce_remove_device() for the CPU that is not correctly initialized by mce_create_device() failure. - make CPU_ONLINE callback always return NOTIFY_OK. Because CPU_ONLINE callback return value is always ignored. Signed-off-by: Andreas Herrmann [EMAIL PROTECTED] [EMAIL PROTECTED]: make CPU_ONLINE callback always return NOTIFY_OK] [EMAIL PROTECTED]: avoid mce_remove_device() for not initialized device] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- arch/x86/kernel/cpu/mcheck/mce_64.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) Index: 2.6-git/arch/x86/kernel/cpu/mcheck/mce_64.c === --- 2.6-git.orig/arch/x86/kernel/cpu/mcheck/mce_64.c +++ 2.6-git/arch/x86/kernel/cpu/mcheck/mce_64.c @@ -802,6 +802,8 @@ static struct sysdev_attribute *mce_attr NULL }; +static cpumask_t mce_device_initialized = CPU_MASK_NONE; + /* Per cpu sysdev init. All of the cpus still share the same ctl bank */ static __cpuinit int mce_create_device(unsigned int cpu) { @@ -825,6 +827,7 @@ static __cpuinit int mce_create_device(u if (err) goto error; } + cpu_set(cpu, mce_device_initialized); return 0; error: @@ -841,10 +844,14 @@ static void mce_remove_device(unsigned i { int i; + if (!cpu_isset(cpu, mce_device_initialized)) + return; + for (i = 0; mce_attributes[i]; i++) sysdev_remove_file(per_cpu(device_mce,cpu), mce_attributes[i]); sysdev_unregister(per_cpu(device_mce,cpu)); + cpu_clear(cpu, mce_device_initialized); } /* Get notified when a cpu comes on/off. Be hotplug friendly. */ @@ -852,21 +859,18 @@ static int mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; - int err = 0; switch (action) { - case CPU_UP_PREPARE: - case CPU_UP_PREPARE_FROZEN: - err = mce_create_device(cpu); + case CPU_ONLINE: + case CPU_ONLINE_FROZEN: + mce_create_device(cpu); break; - case CPU_UP_CANCELED: - case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: mce_remove_device(cpu); break; } - return err ? NOTIFY_BAD : NOTIFY_OK; + return NOTIFY_OK; } static struct notifier_block mce_cpu_notifier = { - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] slub: fix cpu hotplug offline/online path
2007/10/12, Christoph Lameter <[EMAIL PROTECTED]>: > On Thu, 11 Oct 2007, Akinobu Mita wrote: > > > > Why would get_cpu_slab not work? > > > > > > case CPU_DEAD: > > case CPU_DEAD_FROZEN: > > down_read(_lock); > > list_for_each_entry(s, _caches, list) { > > struct kmem_cache_cpu *c = get_cpu_slab(s, cpu); > > > > local_irq_save(flags); > > __flush_cpu_slab(s, cpu); > > local_irq_restore(flags); > > free_kmem_cache_cpu(c, cpu); > > s->cpu_slab[cpu] = NULL; <-- > > } > > up_read(_lock); > > break; > > > > When CPU is offlined, cpu-hotplug notifier sets s->cpu_slab[cpu] = NULL. > > This means get_cpu_slab() always return NULL when CPU is being onlined. > > So I can't use get_cpu_slab to check whether kmem_cache_cpu_free > > initalization for the CPU has already been done or not. > > If you have set it to NULL then the earlier kmem_cache_cpu structures has > been freed. Why is it a problem to allocate another one when the cpu comes > up again? > kmem_cache_cpu_free per-cpu singly list will be broken by calling init_alloc_cpu_cpu() twice. It happens when online/offlining CPU. All three patches I send attempt to make init_alloc_cpu_cpu() called only once for each cpu. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] slub: fix cpu hotplug offline/online path
On Wed, Oct 10, 2007 at 10:39:32AM -0700, Christoph Lameter wrote: > On Wed, 10 Oct 2007, Akinobu Mita wrote: > > > I couldn't use get_cpu_slab() for that check. But I reviced the patch to do > > what you said. > > Why would get_cpu_slab not work? case CPU_DEAD: case CPU_DEAD_FROZEN: down_read(_lock); list_for_each_entry(s, _caches, list) { struct kmem_cache_cpu *c = get_cpu_slab(s, cpu); local_irq_save(flags); __flush_cpu_slab(s, cpu); local_irq_restore(flags); free_kmem_cache_cpu(c, cpu); s->cpu_slab[cpu] = NULL; <-- } up_read(_lock); break; When CPU is offlined, cpu-hotplug notifier sets s->cpu_slab[cpu] = NULL. This means get_cpu_slab() always return NULL when CPU is being onlined. So I can't use get_cpu_slab to check whether kmem_cache_cpu_free initalization for the CPU has already been done or not. > > > + if (per_cpu(kmem_cache_cpu_free, cpu)) { > > + /* Already initialized once */ > > + return; > > + } > > + > > kmem_cache_cpu_free is not only NULL if the cpu is not up yet but it is > also NULL if the per cpu pool of kmem_cache_cpu structures was > exhausted. cpu-hotplug notifier by CPU_DEAD event frees kmem_cache_cpu structures for the CPU being offlined. So we have 100 kmem_cache cpu structures when the CPU will be onlined again. But I agree that it is not so trivial. It is why I added the comment /* Already initialized once */ in previous patch. This is another approach for the fix. Use cpumask to check whether kmem_cache_cpu_free initalization for the CPU has already been done or not. --- mm/slub.c |6 ++ 1 file changed, 6 insertions(+) Index: 2.6-mm/mm/slub.c === --- 2.6-mm.orig/mm/slub.c +++ 2.6-mm/mm/slub.c @@ -1959,6 +1959,7 @@ static DEFINE_PER_CPU(struct kmem_cache_ kmem_cache_cpu)[NR_KMEM_CACHE_CPU]; static DEFINE_PER_CPU(struct kmem_cache_cpu *, kmem_cache_cpu_free); +static cpumask_t kmem_cach_cpu_free_init_once = CPU_MASK_NONE; static struct kmem_cache_cpu *alloc_kmem_cache_cpu(struct kmem_cache *s, int cpu, gfp_t flags) @@ -2033,8 +2034,13 @@ static void init_alloc_cpu_cpu(int cpu) { int i; + if (cpu_isset(cpu, kmem_cach_cpu_free_init_once)) + return; + for (i = NR_KMEM_CACHE_CPU - 1; i >= 0; i--) free_kmem_cache_cpu(_cpu(kmem_cache_cpu, cpu)[i], cpu); + + cpu_set(cpu, kmem_cach_cpu_free_init_once); } static void __init init_alloc_cpu(void) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] slub: fix cpu hotplug offline/online path
On Wed, Oct 10, 2007 at 10:39:32AM -0700, Christoph Lameter wrote: On Wed, 10 Oct 2007, Akinobu Mita wrote: I couldn't use get_cpu_slab() for that check. But I reviced the patch to do what you said. Why would get_cpu_slab not work? case CPU_DEAD: case CPU_DEAD_FROZEN: down_read(slub_lock); list_for_each_entry(s, slab_caches, list) { struct kmem_cache_cpu *c = get_cpu_slab(s, cpu); local_irq_save(flags); __flush_cpu_slab(s, cpu); local_irq_restore(flags); free_kmem_cache_cpu(c, cpu); s-cpu_slab[cpu] = NULL; -- } up_read(slub_lock); break; When CPU is offlined, cpu-hotplug notifier sets s-cpu_slab[cpu] = NULL. This means get_cpu_slab() always return NULL when CPU is being onlined. So I can't use get_cpu_slab to check whether kmem_cache_cpu_free initalization for the CPU has already been done or not. + if (per_cpu(kmem_cache_cpu_free, cpu)) { + /* Already initialized once */ + return; + } + kmem_cache_cpu_free is not only NULL if the cpu is not up yet but it is also NULL if the per cpu pool of kmem_cache_cpu structures was exhausted. cpu-hotplug notifier by CPU_DEAD event frees kmem_cache_cpu structures for the CPU being offlined. So we have 100 kmem_cache cpu structures when the CPU will be onlined again. But I agree that it is not so trivial. It is why I added the comment /* Already initialized once */ in previous patch. This is another approach for the fix. Use cpumask to check whether kmem_cache_cpu_free initalization for the CPU has already been done or not. --- mm/slub.c |6 ++ 1 file changed, 6 insertions(+) Index: 2.6-mm/mm/slub.c === --- 2.6-mm.orig/mm/slub.c +++ 2.6-mm/mm/slub.c @@ -1959,6 +1959,7 @@ static DEFINE_PER_CPU(struct kmem_cache_ kmem_cache_cpu)[NR_KMEM_CACHE_CPU]; static DEFINE_PER_CPU(struct kmem_cache_cpu *, kmem_cache_cpu_free); +static cpumask_t kmem_cach_cpu_free_init_once = CPU_MASK_NONE; static struct kmem_cache_cpu *alloc_kmem_cache_cpu(struct kmem_cache *s, int cpu, gfp_t flags) @@ -2033,8 +2034,13 @@ static void init_alloc_cpu_cpu(int cpu) { int i; + if (cpu_isset(cpu, kmem_cach_cpu_free_init_once)) + return; + for (i = NR_KMEM_CACHE_CPU - 1; i = 0; i--) free_kmem_cache_cpu(per_cpu(kmem_cache_cpu, cpu)[i], cpu); + + cpu_set(cpu, kmem_cach_cpu_free_init_once); } static void __init init_alloc_cpu(void) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] slub: fix cpu hotplug offline/online path
2007/10/12, Christoph Lameter [EMAIL PROTECTED]: On Thu, 11 Oct 2007, Akinobu Mita wrote: Why would get_cpu_slab not work? case CPU_DEAD: case CPU_DEAD_FROZEN: down_read(slub_lock); list_for_each_entry(s, slab_caches, list) { struct kmem_cache_cpu *c = get_cpu_slab(s, cpu); local_irq_save(flags); __flush_cpu_slab(s, cpu); local_irq_restore(flags); free_kmem_cache_cpu(c, cpu); s-cpu_slab[cpu] = NULL; -- } up_read(slub_lock); break; When CPU is offlined, cpu-hotplug notifier sets s-cpu_slab[cpu] = NULL. This means get_cpu_slab() always return NULL when CPU is being onlined. So I can't use get_cpu_slab to check whether kmem_cache_cpu_free initalization for the CPU has already been done or not. If you have set it to NULL then the earlier kmem_cache_cpu structures has been freed. Why is it a problem to allocate another one when the cpu comes up again? kmem_cache_cpu_free per-cpu singly list will be broken by calling init_alloc_cpu_cpu() twice. It happens when online/offlining CPU. All three patches I send attempt to make init_alloc_cpu_cpu() called only once for each cpu. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] update sb->s_frozen when freezing read-only mounted device, too
freeze_bdev() with read-only mounted device(*) does not change sb->s_frozen from SB_UNFROZEN to SB_FREEZE_TRANS. Because of this behavior, xfs_freeze can break read-only filesystem. Because xfs_thaw does nothing for the filesystem whose sb->s_frozen is SB_UNFROZEN. So frozen read-only XFS filesystem will never be unfrozen. Thus we cannot do any unmount/remount operations for that filesystem. This patch updates sb->s_frozen when freeze_bdev() is called for read-only mounted device, too. (*) freezing read-only filesystem is not so pointless. Because it can prevent from someone trying to remount read/write while freezing. Cc: David Chinner <[EMAIL PROTECTED]> Cc: Tim Shimmin <[EMAIL PROTECTED]> Cc: Christoph Hellwig <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- fs/buffer.c | 25 - 1 file changed, 16 insertions(+), 9 deletions(-) Index: 2.6-git/fs/buffer.c === --- 2.6-git.orig/fs/buffer.c +++ 2.6-git/fs/buffer.c @@ -190,21 +190,28 @@ struct super_block *freeze_bdev(struct b down(>bd_mount_sem); sb = get_super(bdev); - if (sb && !(sb->s_flags & MS_RDONLY)) { - sb->s_frozen = SB_FREEZE_WRITE; - smp_wmb(); - - __fsync_super(sb); + if (!sb) + goto out; + if (sb->s_flags & MS_RDONLY) { sb->s_frozen = SB_FREEZE_TRANS; smp_wmb(); + goto out; + } - sync_blockdev(sb->s_bdev); + sb->s_frozen = SB_FREEZE_WRITE; + smp_wmb(); - if (sb->s_op->write_super_lockfs) - sb->s_op->write_super_lockfs(sb); - } + __fsync_super(sb); + + sb->s_frozen = SB_FREEZE_TRANS; + smp_wmb(); + sync_blockdev(sb->s_bdev); + + if (sb->s_op->write_super_lockfs) + sb->s_op->write_super_lockfs(sb); +out: sync_blockdev(bdev); return sb; /* thaw_bdev releases s->s_umount and bd_mount_sem */ } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] slub: fix cpu hotplug offline/online path
On Tue, Oct 09, 2007 at 11:46:14AM -0700, Christoph Lameter wrote: > On Wed, 10 Oct 2007, Akinobu Mita wrote: > > > This patch removes init_alloc_cpu_cpu() from cpu hotplug notifier. But > > call it for each possible CPUs not only online CPUs at initialization time. > > Could you check if a per cpu structure has already been allocated and then > simply skip the call to init? Otherwise we end up with lots of per cpu > structures for cpus that will never show up. > > if (!get_cpu_slab(s, cpu)) > init_alloc_cpu_cpu( ...) > I couldn't use get_cpu_slab() for that check. But I reviced the patch to do what you said. Subject: slub: fix cpu hotplug offline/online path This patch fixes the problem introduced by: http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/broken-out/slub-place-kmem_cache_cpu-structures-in-a-numa-aware-way.patch I got slub BUG report when I tried to do cpu hotplug/unplug $ while true; do echo 0 > /sys/devices/system/cpu/cpu1/online echo 1 > /sys/devices/system/cpu/cpu1/online done This is because init_alloc_cpu_cpu() is called every time when the CPU is going to be onlined but init_alloc_cpu_cpu() is not intented to be called twice or more for same CPU. Then it breaks kmem_cache_cpu_free list for the CPU. This patch checks if a per cpu structure has already been allocated and then simply skip the call to init_alloc_cpu_cpu(). Cc: Christoph Lameter <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- mm/slub.c |5 + 1 file changed, 5 insertions(+) Index: 2.6-mm/mm/slub.c === --- 2.6-mm.orig/mm/slub.c +++ 2.6-mm/mm/slub.c @@ -2033,6 +2033,11 @@ static void init_alloc_cpu_cpu(int cpu) { int i; + if (per_cpu(kmem_cache_cpu_free, cpu)) { + /* Already initialized once */ + return; + } + for (i = NR_KMEM_CACHE_CPU - 1; i >= 0; i--) free_kmem_cache_cpu(_cpu(kmem_cache_cpu, cpu)[i], cpu); } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] slub: fix cpu hotplug offline/online path
On Tue, Oct 09, 2007 at 11:46:14AM -0700, Christoph Lameter wrote: On Wed, 10 Oct 2007, Akinobu Mita wrote: This patch removes init_alloc_cpu_cpu() from cpu hotplug notifier. But call it for each possible CPUs not only online CPUs at initialization time. Could you check if a per cpu structure has already been allocated and then simply skip the call to init? Otherwise we end up with lots of per cpu structures for cpus that will never show up. if (!get_cpu_slab(s, cpu)) init_alloc_cpu_cpu( ...) I couldn't use get_cpu_slab() for that check. But I reviced the patch to do what you said. Subject: slub: fix cpu hotplug offline/online path This patch fixes the problem introduced by: http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/broken-out/slub-place-kmem_cache_cpu-structures-in-a-numa-aware-way.patch I got slub BUG report when I tried to do cpu hotplug/unplug $ while true; do echo 0 /sys/devices/system/cpu/cpu1/online echo 1 /sys/devices/system/cpu/cpu1/online done This is because init_alloc_cpu_cpu() is called every time when the CPU is going to be onlined but init_alloc_cpu_cpu() is not intented to be called twice or more for same CPU. Then it breaks kmem_cache_cpu_free list for the CPU. This patch checks if a per cpu structure has already been allocated and then simply skip the call to init_alloc_cpu_cpu(). Cc: Christoph Lameter [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- mm/slub.c |5 + 1 file changed, 5 insertions(+) Index: 2.6-mm/mm/slub.c === --- 2.6-mm.orig/mm/slub.c +++ 2.6-mm/mm/slub.c @@ -2033,6 +2033,11 @@ static void init_alloc_cpu_cpu(int cpu) { int i; + if (per_cpu(kmem_cache_cpu_free, cpu)) { + /* Already initialized once */ + return; + } + for (i = NR_KMEM_CACHE_CPU - 1; i = 0; i--) free_kmem_cache_cpu(per_cpu(kmem_cache_cpu, cpu)[i], cpu); } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] update sb-s_frozen when freezing read-only mounted device, too
freeze_bdev() with read-only mounted device(*) does not change sb-s_frozen from SB_UNFROZEN to SB_FREEZE_TRANS. Because of this behavior, xfs_freeze can break read-only filesystem. Because xfs_thaw does nothing for the filesystem whose sb-s_frozen is SB_UNFROZEN. So frozen read-only XFS filesystem will never be unfrozen. Thus we cannot do any unmount/remount operations for that filesystem. This patch updates sb-s_frozen when freeze_bdev() is called for read-only mounted device, too. (*) freezing read-only filesystem is not so pointless. Because it can prevent from someone trying to remount read/write while freezing. Cc: David Chinner [EMAIL PROTECTED] Cc: Tim Shimmin [EMAIL PROTECTED] Cc: Christoph Hellwig [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- fs/buffer.c | 25 - 1 file changed, 16 insertions(+), 9 deletions(-) Index: 2.6-git/fs/buffer.c === --- 2.6-git.orig/fs/buffer.c +++ 2.6-git/fs/buffer.c @@ -190,21 +190,28 @@ struct super_block *freeze_bdev(struct b down(bdev-bd_mount_sem); sb = get_super(bdev); - if (sb !(sb-s_flags MS_RDONLY)) { - sb-s_frozen = SB_FREEZE_WRITE; - smp_wmb(); - - __fsync_super(sb); + if (!sb) + goto out; + if (sb-s_flags MS_RDONLY) { sb-s_frozen = SB_FREEZE_TRANS; smp_wmb(); + goto out; + } - sync_blockdev(sb-s_bdev); + sb-s_frozen = SB_FREEZE_WRITE; + smp_wmb(); - if (sb-s_op-write_super_lockfs) - sb-s_op-write_super_lockfs(sb); - } + __fsync_super(sb); + + sb-s_frozen = SB_FREEZE_TRANS; + smp_wmb(); + sync_blockdev(sb-s_bdev); + + if (sb-s_op-write_super_lockfs) + sb-s_op-write_super_lockfs(sb); +out: sync_blockdev(bdev); return sb; /* thaw_bdev releases s-s_umount and bd_mount_sem */ } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm] slub: fix cpu hotplug offline/online path
This patch fixes the problem introduced by: http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/broken-out/slub-place-kmem_cache_cpu-structures-in-a-numa-aware-way.patch I got slub BUG report when I tried to do cpu hotplug/unplug $ while true; do echo 0 > /sys/devices/system/cpu/cpu1/online echo 1 > /sys/devices/system/cpu/cpu1/online done This is because init_alloc_cpu_cpu() is called every time when the CPU is going to be onlined but init_alloc_cpu_cpu() is not intented to be called twice or more for same CPU. Then it breaks kmem_cache_cpu_free list for the CPU. This patch removes init_alloc_cpu_cpu() from cpu hotplug notifier. But call it for each possible CPUs not only online CPUs at initialization time. Cc: Christoph Lameter <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- mm/slub.c |5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) Index: 2.6-mm/mm/slub.c === --- 2.6-mm.orig/mm/slub.c +++ 2.6-mm/mm/slub.c @@ -2029,7 +2029,7 @@ static int alloc_kmem_cache_cpus(struct /* * Initialize the per cpu array. */ -static void init_alloc_cpu_cpu(int cpu) +static void __init init_alloc_cpu_cpu(int cpu) { int i; @@ -2041,7 +2041,7 @@ static void __init init_alloc_cpu(void) { int cpu; - for_each_online_cpu(cpu) + for_each_possible_cpu(cpu) init_alloc_cpu_cpu(cpu); } @@ -2973,7 +2973,6 @@ static int __cpuinit slab_cpuup_callback switch (action) { case CPU_UP_PREPARE: case CPU_UP_PREPARE_FROZEN: - init_alloc_cpu_cpu(cpu); down_read(_lock); list_for_each_entry(s, _caches, list) s->cpu_slab[cpu] = alloc_kmem_cache_cpu(s, cpu, - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm] fix wrong /proc/cpuinfo output
This patch fixes the problem introduced by: http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/broken-out/x86-convert-cpuinfo_x86-array-to-a-per_cpu-array.patch The problem is that every processor line in /proc/cpuinfo displays zero on x86_64. $ grep processor /proc/cpuinfo processor : 0 processor : 0 Because early_identify_cpu() overwrites c->cpu_index for every cpuinfo. This patch removes that unnecessary initialization for c->cpu_index. Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> Index: 2.6-mm/arch/x86_64/kernel/setup.c === --- 2.6-mm.orig/arch/x86_64/kernel/setup.c +++ 2.6-mm/arch/x86_64/kernel/setup.c @@ -967,7 +967,6 @@ void __cpuinit early_identify_cpu(struct #ifdef CONFIG_SMP c->phys_proc_id = (cpuid_ebx(1) >> 24) & 0xff; - c->cpu_index = 0; #endif /* AMD-defined flags: level 0x8001 */ xlvl = cpuid_eax(0x8000); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm] fix wrong /proc/cpuinfo output
This patch fixes the problem introduced by: http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/broken-out/x86-convert-cpuinfo_x86-array-to-a-per_cpu-array.patch The problem is that every processor line in /proc/cpuinfo displays zero on x86_64. $ grep processor /proc/cpuinfo processor : 0 processor : 0 Because early_identify_cpu() overwrites c-cpu_index for every cpuinfo. This patch removes that unnecessary initialization for c-cpu_index. Signed-off-by: Akinobu Mita [EMAIL PROTECTED] Index: 2.6-mm/arch/x86_64/kernel/setup.c === --- 2.6-mm.orig/arch/x86_64/kernel/setup.c +++ 2.6-mm/arch/x86_64/kernel/setup.c @@ -967,7 +967,6 @@ void __cpuinit early_identify_cpu(struct #ifdef CONFIG_SMP c-phys_proc_id = (cpuid_ebx(1) 24) 0xff; - c-cpu_index = 0; #endif /* AMD-defined flags: level 0x8001 */ xlvl = cpuid_eax(0x8000); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm] slub: fix cpu hotplug offline/online path
This patch fixes the problem introduced by: http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/broken-out/slub-place-kmem_cache_cpu-structures-in-a-numa-aware-way.patch I got slub BUG report when I tried to do cpu hotplug/unplug $ while true; do echo 0 /sys/devices/system/cpu/cpu1/online echo 1 /sys/devices/system/cpu/cpu1/online done This is because init_alloc_cpu_cpu() is called every time when the CPU is going to be onlined but init_alloc_cpu_cpu() is not intented to be called twice or more for same CPU. Then it breaks kmem_cache_cpu_free list for the CPU. This patch removes init_alloc_cpu_cpu() from cpu hotplug notifier. But call it for each possible CPUs not only online CPUs at initialization time. Cc: Christoph Lameter [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- mm/slub.c |5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) Index: 2.6-mm/mm/slub.c === --- 2.6-mm.orig/mm/slub.c +++ 2.6-mm/mm/slub.c @@ -2029,7 +2029,7 @@ static int alloc_kmem_cache_cpus(struct /* * Initialize the per cpu array. */ -static void init_alloc_cpu_cpu(int cpu) +static void __init init_alloc_cpu_cpu(int cpu) { int i; @@ -2041,7 +2041,7 @@ static void __init init_alloc_cpu(void) { int cpu; - for_each_online_cpu(cpu) + for_each_possible_cpu(cpu) init_alloc_cpu_cpu(cpu); } @@ -2973,7 +2973,6 @@ static int __cpuinit slab_cpuup_callback switch (action) { case CPU_UP_PREPARE: case CPU_UP_PREPARE_FROZEN: - init_alloc_cpu_cpu(cpu); down_read(slub_lock); list_for_each_entry(s, slab_caches, list) s-cpu_slab[cpu] = alloc_kmem_cache_cpu(s, cpu, - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] update sb->s_frozen when freezing read-only mounted device, too
2007/10/5, Christoph Hellwig <[EMAIL PROTECTED]>: > On Sat, Sep 29, 2007 at 07:09:12PM +0900, Akinobu Mita wrote: > > freeze_bdev() with the device which is mounted as read only > > does not change sb->s_frozen from SB_UNFROZEN to SB_FREEZE_TRANS. > > > > Because of this behavior, xfs_freeze can break read-only XFS filesystem. > > > > Because xfs_thaw does nothing for the filesystem whose sb->s_frozen is > > SB_UNFROZEN. So freezed readonly XFS filesystem will never be unfreezed. > > Then we cannot do any unmount/remount operations for that filesystem. > > > > This patch updates sb->s_frozen when freeze_bdev() is called for read-only > > mounted device, too. > > I think this fix is valid, but it might be a tad cleaner to just > set s_frozen to SB_FREEZE_TRANS directly in a separate branch, ala: It looks cleaner than mine. I'll resubmit it. > struct super_block *freeze_bdev(struct block_device *bdev) > { > struct super_block *sb; > > down(>bd_mount_sem); > sb = get_super(bdev); > if (!sb) > goto out; > > if (sb->s_flags & MS_RDONLY) { > sb->s_frozen = SB_FREEZE_TRANS; > smp_wmb(); > goto out; > } > > sb->s_frozen = SB_FREEZE_WRITE; > smp_wmb(); > > __fsync_super(sb); > > sb->s_frozen = SB_FREEZE_TRANS > smp_wmb(); > > sync_blockdev(sb->s_bdev); > > if (sb->s_op->write_super_lockfs) > sb->s_op->write_super_lockfs(sb); > > out: > sync_blockdev(bdev); > return sb; /* thaw_bdev releases s->s_umount and bd_mount_sem */ > } > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] update sb-s_frozen when freezing read-only mounted device, too
2007/10/5, Christoph Hellwig [EMAIL PROTECTED]: On Sat, Sep 29, 2007 at 07:09:12PM +0900, Akinobu Mita wrote: freeze_bdev() with the device which is mounted as read only does not change sb-s_frozen from SB_UNFROZEN to SB_FREEZE_TRANS. Because of this behavior, xfs_freeze can break read-only XFS filesystem. Because xfs_thaw does nothing for the filesystem whose sb-s_frozen is SB_UNFROZEN. So freezed readonly XFS filesystem will never be unfreezed. Then we cannot do any unmount/remount operations for that filesystem. This patch updates sb-s_frozen when freeze_bdev() is called for read-only mounted device, too. I think this fix is valid, but it might be a tad cleaner to just set s_frozen to SB_FREEZE_TRANS directly in a separate branch, ala: It looks cleaner than mine. I'll resubmit it. struct super_block *freeze_bdev(struct block_device *bdev) { struct super_block *sb; down(bdev-bd_mount_sem); sb = get_super(bdev); if (!sb) goto out; if (sb-s_flags MS_RDONLY) { sb-s_frozen = SB_FREEZE_TRANS; smp_wmb(); goto out; } sb-s_frozen = SB_FREEZE_WRITE; smp_wmb(); __fsync_super(sb); sb-s_frozen = SB_FREEZE_TRANS smp_wmb(); sync_blockdev(sb-s_bdev); if (sb-s_op-write_super_lockfs) sb-s_op-write_super_lockfs(sb); out: sync_blockdev(bdev); return sb; /* thaw_bdev releases s-s_umount and bd_mount_sem */ } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6] Xilinxfb: add of_platform bus binding
2007/10/2, Grant Likely <[EMAIL PROTECTED]>: > static int __init > xilinxfb_init(void) > { > - /* > -* No kernel boot options used, > -* so we just need to register the driver > -*/ > + int rc; > + rc = xilinxfb_of_register(); > + if (rc) > + return rc; > + > return platform_driver_register(_platform_driver); Is it better to add error handling for platform_driver_register()? rc = platform_driver_register(_platform_driver); if (rc) xilinxfb_of_unregister(); return rc; > } > > @@ -398,6 +482,7 @@ static void __exit > xilinxfb_cleanup(void) > { > platform_driver_unregister(_platform_driver); > + xilinxfb_of_unregister(); > } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6] Xilinxfb: add of_platform bus binding
2007/10/2, Grant Likely [EMAIL PROTECTED]: static int __init xilinxfb_init(void) { - /* -* No kernel boot options used, -* so we just need to register the driver -*/ + int rc; + rc = xilinxfb_of_register(); + if (rc) + return rc; + return platform_driver_register(xilinxfb_platform_driver); Is it better to add error handling for platform_driver_register()? rc = platform_driver_register(xilinxfb_platform_driver); if (rc) xilinxfb_of_unregister(); return rc; } @@ -398,6 +482,7 @@ static void __exit xilinxfb_cleanup(void) { platform_driver_unregister(xilinxfb_platform_driver); + xilinxfb_of_unregister(); } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] module: return error when mod_sysfs_init() failed
2007/9/29, Greg KH <[EMAIL PROTECTED]>: > > Index: 2.6-git/kernel/module.c > > === > > --- 2.6-git.orig/kernel/module.c > > +++ 2.6-git/kernel/module.c > > @@ -1782,7 +1782,8 @@ static struct module *load_module(void _ > > module_unload_init(mod); > > > > /* Initialize kobject, so we can reference it. */ > > - if (mod_sysfs_init(mod) != 0) > > + err = mod_sysfs_init(mod); > > + if (err) > > goto cleanup; > > I must be still asleep this morning, but I think this patch does the > exact same thing as the original code does, right? Otherwise, this > code would always be failing. > > Or do I just need to go get my morning coffee to wake up and see the > problem here? Hello, In the original code, the "err" is zero before goto cleanup. This "err" will be the return value of load_module(). load_module() is the function which returns error as pointer and the expression IS_ERR(NULL) is false. So the caller of load_module() cannot catch that error. I found this problem when I was running the fault injection test script in Documentation/fault-injection/fault-injection.txt with random module. #!/bin/bash FAILTYPE=failslab echo Y > /debug/$FAILTYPE/task-filter echo 10 > /debug/$FAILTYPE/probability echo 100 > /debug/$FAILTYPE/interval echo -1 > /debug/$FAILTYPE/times echo 0 > /debug/$FAILTYPE/space echo 2 > /debug/$FAILTYPE/verbose echo 0 > /debug/$FAILTYPE/ignore-gfp-wait faulty_system() { bash -c "echo 1 > /proc/self/make-it-fail && exec $*" } if [ $# -eq 0 ] then echo "Usage: $0 modulename [ modulename ... ]" exit 1 fi for m in $* do echo inserting $m... faulty_system modprobe $m echo removing $m... faulty_system modprobe -r $m done - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] update sb->s_frozen when freezing read-only mounted device, too
freeze_bdev() with the device which is mounted as read only does not change sb->s_frozen from SB_UNFROZEN to SB_FREEZE_TRANS. Because of this behavior, xfs_freeze can break read-only XFS filesystem. Because xfs_thaw does nothing for the filesystem whose sb->s_frozen is SB_UNFROZEN. So freezed readonly XFS filesystem will never be unfreezed. Then we cannot do any unmount/remount operations for that filesystem. This patch updates sb->s_frozen when freeze_bdev() is called for read-only mounted device, too. Cc: Tim Shimmin <[EMAIL PROTECTED]> Cc: Christoph Hellwig <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- fs/buffer.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) Index: 2.6-git/fs/buffer.c === --- 2.6-git.orig/fs/buffer.c +++ 2.6-git/fs/buffer.c @@ -190,19 +190,22 @@ struct super_block *freeze_bdev(struct b down(>bd_mount_sem); sb = get_super(bdev); - if (sb && !(sb->s_flags & MS_RDONLY)) { + if (sb) { sb->s_frozen = SB_FREEZE_WRITE; smp_wmb(); - __fsync_super(sb); + if (!(sb->s_flags & MS_RDONLY)) + __fsync_super(sb); sb->s_frozen = SB_FREEZE_TRANS; smp_wmb(); - sync_blockdev(sb->s_bdev); + if (!(sb->s_flags & MS_RDONLY)) { + sync_blockdev(sb->s_bdev); - if (sb->s_op->write_super_lockfs) - sb->s_op->write_super_lockfs(sb); + if (sb->s_op->write_super_lockfs) + sb->s_op->write_super_lockfs(sb); + } } sync_blockdev(bdev); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] module: return error when mod_sysfs_init() failed
load_module() returns zero when mod_sysfs_init() fails, then the module loading will succeed accidentally. This patch makes load_module() return error correctly in that case. Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]> Cc: Rusty Russell <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- kernel/module.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: 2.6-git/kernel/module.c === --- 2.6-git.orig/kernel/module.c +++ 2.6-git/kernel/module.c @@ -1782,7 +1782,8 @@ static struct module *load_module(void _ module_unload_init(mod); /* Initialize kobject, so we can reference it. */ - if (mod_sysfs_init(mod) != 0) + err = mod_sysfs_init(mod); + if (err) goto cleanup; /* Set up license info based on the info section */ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] module: return error when mod_sysfs_init() failed
load_module() returns zero when mod_sysfs_init() fails, then the module loading will succeed accidentally. This patch makes load_module() return error correctly in that case. Cc: Greg Kroah-Hartman [EMAIL PROTECTED] Cc: Rusty Russell [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- kernel/module.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: 2.6-git/kernel/module.c === --- 2.6-git.orig/kernel/module.c +++ 2.6-git/kernel/module.c @@ -1782,7 +1782,8 @@ static struct module *load_module(void _ module_unload_init(mod); /* Initialize kobject, so we can reference it. */ - if (mod_sysfs_init(mod) != 0) + err = mod_sysfs_init(mod); + if (err) goto cleanup; /* Set up license info based on the info section */ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] update sb-s_frozen when freezing read-only mounted device, too
freeze_bdev() with the device which is mounted as read only does not change sb-s_frozen from SB_UNFROZEN to SB_FREEZE_TRANS. Because of this behavior, xfs_freeze can break read-only XFS filesystem. Because xfs_thaw does nothing for the filesystem whose sb-s_frozen is SB_UNFROZEN. So freezed readonly XFS filesystem will never be unfreezed. Then we cannot do any unmount/remount operations for that filesystem. This patch updates sb-s_frozen when freeze_bdev() is called for read-only mounted device, too. Cc: Tim Shimmin [EMAIL PROTECTED] Cc: Christoph Hellwig [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- fs/buffer.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) Index: 2.6-git/fs/buffer.c === --- 2.6-git.orig/fs/buffer.c +++ 2.6-git/fs/buffer.c @@ -190,19 +190,22 @@ struct super_block *freeze_bdev(struct b down(bdev-bd_mount_sem); sb = get_super(bdev); - if (sb !(sb-s_flags MS_RDONLY)) { + if (sb) { sb-s_frozen = SB_FREEZE_WRITE; smp_wmb(); - __fsync_super(sb); + if (!(sb-s_flags MS_RDONLY)) + __fsync_super(sb); sb-s_frozen = SB_FREEZE_TRANS; smp_wmb(); - sync_blockdev(sb-s_bdev); + if (!(sb-s_flags MS_RDONLY)) { + sync_blockdev(sb-s_bdev); - if (sb-s_op-write_super_lockfs) - sb-s_op-write_super_lockfs(sb); + if (sb-s_op-write_super_lockfs) + sb-s_op-write_super_lockfs(sb); + } } sync_blockdev(bdev); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] module: return error when mod_sysfs_init() failed
2007/9/29, Greg KH [EMAIL PROTECTED]: Index: 2.6-git/kernel/module.c === --- 2.6-git.orig/kernel/module.c +++ 2.6-git/kernel/module.c @@ -1782,7 +1782,8 @@ static struct module *load_module(void _ module_unload_init(mod); /* Initialize kobject, so we can reference it. */ - if (mod_sysfs_init(mod) != 0) + err = mod_sysfs_init(mod); + if (err) goto cleanup; I must be still asleep this morning, but I think this patch does the exact same thing as the original code does, right? Otherwise, this code would always be failing. Or do I just need to go get my morning coffee to wake up and see the problem here? Hello, In the original code, the err is zero before goto cleanup. This err will be the return value of load_module(). load_module() is the function which returns error as pointer and the expression IS_ERR(NULL) is false. So the caller of load_module() cannot catch that error. I found this problem when I was running the fault injection test script in Documentation/fault-injection/fault-injection.txt with random module. #!/bin/bash FAILTYPE=failslab echo Y /debug/$FAILTYPE/task-filter echo 10 /debug/$FAILTYPE/probability echo 100 /debug/$FAILTYPE/interval echo -1 /debug/$FAILTYPE/times echo 0 /debug/$FAILTYPE/space echo 2 /debug/$FAILTYPE/verbose echo 0 /debug/$FAILTYPE/ignore-gfp-wait faulty_system() { bash -c echo 1 /proc/self/make-it-fail exec $* } if [ $# -eq 0 ] then echo Usage: $0 modulename [ modulename ... ] exit 1 fi for m in $* do echo inserting $m... faulty_system modprobe $m echo removing $m... faulty_system modprobe -r $m done - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] use pgd_list_add/pgd_list_del
Cleanup by using pgd_list_add() and pgd_list_del() in the right place. Cc: Andi Kleen <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- include/asm-x86_64/pgalloc.h | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) Index: 2.6-git/include/asm-x86_64/pgalloc.h === --- 2.6-git.orig/include/asm-x86_64/pgalloc.h +++ 2.6-git/include/asm-x86_64/pgalloc.h @@ -65,7 +65,6 @@ static inline void pgd_ctor(void *x) { unsigned boundary; pgd_t *pgd = x; - struct page *page = virt_to_page(pgd); /* * Copy kernel pointers in from init. @@ -75,19 +74,14 @@ static inline void pgd_ctor(void *x) init_level4_pgt + boundary, (PTRS_PER_PGD - boundary) * sizeof(pgd_t)); - spin_lock(_lock); - list_add(>lru, _list); - spin_unlock(_lock); + pgd_list_add(pgd); } static inline void pgd_dtor(void *x) { pgd_t *pgd = x; - struct page *page = virt_to_page(pgd); -spin_lock(_lock); - list_del(>lru); - spin_unlock(_lock); + pgd_list_del(pgd); } static inline pgd_t *pgd_alloc(struct mm_struct *mm) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [35/50] i386: Do cpuid_device_create() in CPU_UP_PREPARE instead of CPU_ONLINE.
2007/9/23, Thomas Gleixner <[EMAIL PROTECTED]>: > On Sat, 2007-09-22 at 00:32 +0200, Andi Kleen wrote: > > From: Akinobu Mita <[EMAIL PROTECTED]> > > > > Do cpuid_device_create() in CPU_UP_PREPARE instead of CPU_ONLINE. > > > > Cc: "H. Peter Anvin" <[EMAIL PROTECTED]> > > Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> > > Signed-off-by: Andi Kleen <[EMAIL PROTECTED]> > > Cc: Gautham R Shenoy <[EMAIL PROTECTED]> > > Cc: Oleg Nesterov <[EMAIL PROTECTED]> > > Signed-off-by: Andrew Morton <[EMAIL PROTECTED]> > > --- > > > > arch/i386/kernel/cpuid.c | 32 +++- > > 1 file changed, 19 insertions(+), 13 deletions(-) > > > > Index: linux/arch/i386/kernel/cpuid.c > > === > > --- linux.orig/arch/i386/kernel/cpuid.c > > +++ linux/arch/i386/kernel/cpuid.c > > @@ -136,15 +136,18 @@ static const struct file_operations cpui > > .open = cpuid_open, > > }; > > > > -static int __cpuinit cpuid_device_create(int i) > > +static int cpuid_device_create(int cpu) > > __cpuinit please > Yes. This eliminates earlier patch in this series. ([22/50] i386: Misc cpuinit annotation) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [35/50] i386: Do cpuid_device_create() in CPU_UP_PREPARE instead of CPU_ONLINE.
2007/9/23, Thomas Gleixner [EMAIL PROTECTED]: On Sat, 2007-09-22 at 00:32 +0200, Andi Kleen wrote: From: Akinobu Mita [EMAIL PROTECTED] Do cpuid_device_create() in CPU_UP_PREPARE instead of CPU_ONLINE. Cc: H. Peter Anvin [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] Signed-off-by: Andi Kleen [EMAIL PROTECTED] Cc: Gautham R Shenoy [EMAIL PROTECTED] Cc: Oleg Nesterov [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] --- arch/i386/kernel/cpuid.c | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) Index: linux/arch/i386/kernel/cpuid.c === --- linux.orig/arch/i386/kernel/cpuid.c +++ linux/arch/i386/kernel/cpuid.c @@ -136,15 +136,18 @@ static const struct file_operations cpui .open = cpuid_open, }; -static int __cpuinit cpuid_device_create(int i) +static int cpuid_device_create(int cpu) __cpuinit please Yes. This eliminates earlier patch in this series. ([22/50] i386: Misc cpuinit annotation) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] use pgd_list_add/pgd_list_del
Cleanup by using pgd_list_add() and pgd_list_del() in the right place. Cc: Andi Kleen [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- include/asm-x86_64/pgalloc.h | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) Index: 2.6-git/include/asm-x86_64/pgalloc.h === --- 2.6-git.orig/include/asm-x86_64/pgalloc.h +++ 2.6-git/include/asm-x86_64/pgalloc.h @@ -65,7 +65,6 @@ static inline void pgd_ctor(void *x) { unsigned boundary; pgd_t *pgd = x; - struct page *page = virt_to_page(pgd); /* * Copy kernel pointers in from init. @@ -75,19 +74,14 @@ static inline void pgd_ctor(void *x) init_level4_pgt + boundary, (PTRS_PER_PGD - boundary) * sizeof(pgd_t)); - spin_lock(pgd_lock); - list_add(page-lru, pgd_list); - spin_unlock(pgd_lock); + pgd_list_add(pgd); } static inline void pgd_dtor(void *x) { pgd_t *pgd = x; - struct page *page = virt_to_page(pgd); -spin_lock(pgd_lock); - list_del(page-lru); - spin_unlock(pgd_lock); + pgd_list_del(pgd); } static inline pgd_t *pgd_alloc(struct mm_struct *mm) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: cpu hotplug support broken in 2.6.23-rc3
2007/8/28, Rafael J. Wysocki <[EMAIL PROTECTED]>: > On Monday, 27 August 2007 23:58, Pavel Machek wrote: > > On Mon 2007-08-27 23:59:31, Rafael J. Wysocki wrote: > > > On Monday, 27 August 2007 23:32, Pavel Machek wrote: > > > > On Mon 2007-08-27 22:36:57, Jeff Chua wrote: > > > > > On 8/27/07, Pavel Machek <[EMAIL PROTECTED]> wrote: > > > > > > On Mon 2007-08-27 12:43:50, Pavel Machek wrote: > > > > > > > Hi! > > > > > > > > > > > > > > Trying to do few onlines/offlines reliably hangs my machine > > > > > > > (thinkpad > > > > > > > x60, i386 architecture). > > > > > > > > > > I just 3 cycles of on-line/off-line on 2.6.23-rc3 on ThinkPad x60s, > > > > > and my system still survives. > > > > > > > > Can you try 20-or-so tests? Mine hangs randomly, so it survived 4 or > > > > so cycles at one point. > > > > > > > > ...or maybe difference is in the .config, or maybe I broken something > > > > in my kernel sources I have been doing enough CPU offline/online test these days and it works fine. But there is no cpufreq driver which supports my machine. So my test didn't cover test cpu hotplug code in cpufreq. If you have cpufreq driver and it is built as module, it is worth trying same test after unloading cpufreq driver in order to narrow down the problem area. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: cpu hotplug support broken in 2.6.23-rc3
2007/8/28, Rafael J. Wysocki [EMAIL PROTECTED]: On Monday, 27 August 2007 23:58, Pavel Machek wrote: On Mon 2007-08-27 23:59:31, Rafael J. Wysocki wrote: On Monday, 27 August 2007 23:32, Pavel Machek wrote: On Mon 2007-08-27 22:36:57, Jeff Chua wrote: On 8/27/07, Pavel Machek [EMAIL PROTECTED] wrote: On Mon 2007-08-27 12:43:50, Pavel Machek wrote: Hi! Trying to do few onlines/offlines reliably hangs my machine (thinkpad x60, i386 architecture). I just 3 cycles of on-line/off-line on 2.6.23-rc3 on ThinkPad x60s, and my system still survives. Can you try 20-or-so tests? Mine hangs randomly, so it survived 4 or so cycles at one point. ...or maybe difference is in the .config, or maybe I broken something in my kernel sources I have been doing enough CPU offline/online test these days and it works fine. But there is no cpufreq driver which supports my machine. So my test didn't cover test cpu hotplug code in cpufreq. If you have cpufreq driver and it is built as module, it is worth trying same test after unloading cpufreq driver in order to narrow down the problem area. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] sysdev: remove global sysdev drivers list
On Mon, Aug 13, 2007 at 09:43:31AM +0200, Cornelia Huck wrote: > On Sun, 12 Aug 2007 13:44:07 +0900, > Akinobu Mita <[EMAIL PROTECTED]> wrote: > > > No one uses sysdev_drivers. Because no one calls sysdev_driver_register > > with NULL class. > > > > And it is difficult to imagine that someone want to implement a global > > sysdev driver which is called with all sys_device on any kind of > > sysdev_class. > > > > So this patch removes global sysdev_drivers list. > > This makes sense, I guess, especially since it simplyfies the code. > > Some minor comments below. Yes. All your comments are oviously correct. Please check updated patch. From: Akinobu Mita <[EMAIL PROTECTED]> Subject: [PATCH] sysdev: remove global sysdev drivers list No one uses sysdev_drivers. Because no one calls sysdev_driver_register with NULL class. And it is difficult to imagine that someone want to implement a global sysdev driver which is called with all sys_device on any kind of sysdev_class. So this patch removes global sysdev_drivers list and update comments for this change. Cc: Tejun Heo <[EMAIL PROTECTED]> Cc: Cornelia Huck <[EMAIL PROTECTED]> Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- drivers/base/sys.c | 71 ++--- 1 file changed, 14 insertions(+), 57 deletions(-) Index: 2.6-git/drivers/base/sys.c === --- 2.6-git.orig/drivers/base/sys.c +++ 2.6-git/drivers/base/sys.c @@ -153,25 +153,22 @@ void sysdev_class_unregister(struct sysd EXPORT_SYMBOL_GPL(sysdev_class_register); EXPORT_SYMBOL_GPL(sysdev_class_unregister); - -static LIST_HEAD(sysdev_drivers); static DEFINE_MUTEX(sysdev_drivers_lock); /** * sysdev_driver_register - Register auxillary driver - * @cls: Device class driver belongs to. + * @cls: Device class driver belongs to. * @drv: Driver. * - * If @cls is valid, then @drv is inserted into @cls->drivers to be + * @drv is inserted into @cls->drivers to be * called on each operation on devices of that class. The refcount * of @cls is incremented. - * Otherwise, @drv is inserted into sysdev_drivers, and called for - * each device. */ -int sysdev_driver_register(struct sysdev_class * cls, - struct sysdev_driver * drv) +int sysdev_driver_register(struct sysdev_class *cls, struct sysdev_driver *drv) { + int err = 0; + mutex_lock(_drivers_lock); if (cls && kset_get(>kset)) { list_add_tail(>entry, >drivers); @@ -182,10 +179,13 @@ int sysdev_driver_register(struct sysdev list_for_each_entry(dev, >kset.list, kobj.entry) drv->add(dev); } - } else - list_add_tail(>entry, _drivers); + } else { + err = -EINVAL; + printk(KERN_ERR "%s: invalid device class\n", __FUNCTION__); + WARN_ON(1); + } mutex_unlock(_drivers_lock); - return 0; + return err; } @@ -251,12 +251,6 @@ int sysdev_register(struct sys_device * * code that should have called us. */ - /* Notify global drivers */ - list_for_each_entry(drv, _drivers, entry) { - if (drv->add) - drv->add(sysdev); - } - /* Notify class auxillary drivers */ list_for_each_entry(drv, >drivers, entry) { if (drv->add) @@ -272,11 +266,6 @@ void sysdev_unregister(struct sys_device struct sysdev_driver * drv; mutex_lock(_drivers_lock); - list_for_each_entry(drv, _drivers, entry) { - if (drv->remove) - drv->remove(sysdev); - } - list_for_each_entry(drv, >cls->drivers, entry) { if (drv->remove) drv->remove(sysdev); @@ -293,7 +282,7 @@ void sysdev_unregister(struct sys_device * * Loop over each class of system devices, and the devices in each * of those classes. For each device, we call the shutdown method for - * each driver registered for the device - the globals, the auxillaries, + * each driver registered for the device - the auxillaries, * and the class driver. * * Note: The list is iterated in reverse order, so that we shut down @@ -320,13 +309,7 @@ void sysdev_shutdown(void) struct sysdev_driver * drv; pr_debug(" %s\n", kobject_name(>kobj)); - /* Call global drivers first. */ - list_for_each_entry(drv, _drivers, en
Re: [PATCH] sysdev: remove global sysdev drivers list
On Mon, Aug 13, 2007 at 09:43:31AM +0200, Cornelia Huck wrote: On Sun, 12 Aug 2007 13:44:07 +0900, Akinobu Mita [EMAIL PROTECTED] wrote: No one uses sysdev_drivers. Because no one calls sysdev_driver_register with NULL class. And it is difficult to imagine that someone want to implement a global sysdev driver which is called with all sys_device on any kind of sysdev_class. So this patch removes global sysdev_drivers list. This makes sense, I guess, especially since it simplyfies the code. Some minor comments below. Yes. All your comments are oviously correct. Please check updated patch. From: Akinobu Mita [EMAIL PROTECTED] Subject: [PATCH] sysdev: remove global sysdev drivers list No one uses sysdev_drivers. Because no one calls sysdev_driver_register with NULL class. And it is difficult to imagine that someone want to implement a global sysdev driver which is called with all sys_device on any kind of sysdev_class. So this patch removes global sysdev_drivers list and update comments for this change. Cc: Tejun Heo [EMAIL PROTECTED] Cc: Cornelia Huck [EMAIL PROTECTED] Cc: Greg Kroah-Hartman [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- drivers/base/sys.c | 71 ++--- 1 file changed, 14 insertions(+), 57 deletions(-) Index: 2.6-git/drivers/base/sys.c === --- 2.6-git.orig/drivers/base/sys.c +++ 2.6-git/drivers/base/sys.c @@ -153,25 +153,22 @@ void sysdev_class_unregister(struct sysd EXPORT_SYMBOL_GPL(sysdev_class_register); EXPORT_SYMBOL_GPL(sysdev_class_unregister); - -static LIST_HEAD(sysdev_drivers); static DEFINE_MUTEX(sysdev_drivers_lock); /** * sysdev_driver_register - Register auxillary driver - * @cls: Device class driver belongs to. + * @cls: Device class driver belongs to. * @drv: Driver. * - * If @cls is valid, then @drv is inserted into @cls-drivers to be + * @drv is inserted into @cls-drivers to be * called on each operation on devices of that class. The refcount * of @cls is incremented. - * Otherwise, @drv is inserted into sysdev_drivers, and called for - * each device. */ -int sysdev_driver_register(struct sysdev_class * cls, - struct sysdev_driver * drv) +int sysdev_driver_register(struct sysdev_class *cls, struct sysdev_driver *drv) { + int err = 0; + mutex_lock(sysdev_drivers_lock); if (cls kset_get(cls-kset)) { list_add_tail(drv-entry, cls-drivers); @@ -182,10 +179,13 @@ int sysdev_driver_register(struct sysdev list_for_each_entry(dev, cls-kset.list, kobj.entry) drv-add(dev); } - } else - list_add_tail(drv-entry, sysdev_drivers); + } else { + err = -EINVAL; + printk(KERN_ERR %s: invalid device class\n, __FUNCTION__); + WARN_ON(1); + } mutex_unlock(sysdev_drivers_lock); - return 0; + return err; } @@ -251,12 +251,6 @@ int sysdev_register(struct sys_device * * code that should have called us. */ - /* Notify global drivers */ - list_for_each_entry(drv, sysdev_drivers, entry) { - if (drv-add) - drv-add(sysdev); - } - /* Notify class auxillary drivers */ list_for_each_entry(drv, cls-drivers, entry) { if (drv-add) @@ -272,11 +266,6 @@ void sysdev_unregister(struct sys_device struct sysdev_driver * drv; mutex_lock(sysdev_drivers_lock); - list_for_each_entry(drv, sysdev_drivers, entry) { - if (drv-remove) - drv-remove(sysdev); - } - list_for_each_entry(drv, sysdev-cls-drivers, entry) { if (drv-remove) drv-remove(sysdev); @@ -293,7 +282,7 @@ void sysdev_unregister(struct sys_device * * Loop over each class of system devices, and the devices in each * of those classes. For each device, we call the shutdown method for - * each driver registered for the device - the globals, the auxillaries, + * each driver registered for the device - the auxillaries, * and the class driver. * * Note: The list is iterated in reverse order, so that we shut down @@ -320,13 +309,7 @@ void sysdev_shutdown(void) struct sysdev_driver * drv; pr_debug( %s\n, kobject_name(sysdev-kobj)); - /* Call global drivers first. */ - list_for_each_entry(drv, sysdev_drivers, entry) { - if (drv-shutdown) - drv-shutdown(sysdev
[PATCH] sysdev: remove global sysdev drivers list
No one uses sysdev_drivers. Because no one calls sysdev_driver_register with NULL class. And it is difficult to imagine that someone want to implement a global sysdev driver which is called with all sys_device on any kind of sysdev_class. So this patch removes global sysdev_drivers list. Cc: Tejun Heo <[EMAIL PROTECTED]> Cc: Cornelia Huck <[EMAIL PROTECTED]> Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- drivers/base/sys.c | 66 + 1 file changed, 12 insertions(+), 54 deletions(-) Index: 2.6-git/drivers/base/sys.c === --- 2.6-git.orig/drivers/base/sys.c +++ 2.6-git/drivers/base/sys.c @@ -153,25 +153,22 @@ void sysdev_class_unregister(struct sysd EXPORT_SYMBOL_GPL(sysdev_class_register); EXPORT_SYMBOL_GPL(sysdev_class_unregister); - -static LIST_HEAD(sysdev_drivers); static DEFINE_MUTEX(sysdev_drivers_lock); /** * sysdev_driver_register - Register auxillary driver - * @cls: Device class driver belongs to. + * @cls: Device class driver belongs to. * @drv: Driver. * - * If @cls is valid, then @drv is inserted into @cls->drivers to be + * @drv is inserted into @cls->drivers to be * called on each operation on devices of that class. The refcount * of @cls is incremented. - * Otherwise, @drv is inserted into sysdev_drivers, and called for - * each device. */ -int sysdev_driver_register(struct sysdev_class * cls, - struct sysdev_driver * drv) +int sysdev_driver_register(struct sysdev_class *cls, struct sysdev_driver *drv) { + int err = 0; + mutex_lock(_drivers_lock); if (cls && kset_get(>kset)) { list_add_tail(>entry, >drivers); @@ -182,10 +179,12 @@ int sysdev_driver_register(struct sysdev list_for_each_entry(dev, >kset.list, kobj.entry) drv->add(dev); } - } else - list_add_tail(>entry, _drivers); + } else { + err = -EINVAL; + WARN_ON(1); + } mutex_unlock(_drivers_lock); - return 0; + return err; } @@ -206,6 +205,8 @@ void sysdev_driver_unregister(struct sys drv->remove(dev); } kset_put(>kset); + } else { + WARN_ON(1); } mutex_unlock(_drivers_lock); } @@ -251,12 +252,6 @@ int sysdev_register(struct sys_device * * code that should have called us. */ - /* Notify global drivers */ - list_for_each_entry(drv, _drivers, entry) { - if (drv->add) - drv->add(sysdev); - } - /* Notify class auxillary drivers */ list_for_each_entry(drv, >drivers, entry) { if (drv->add) @@ -272,11 +267,6 @@ void sysdev_unregister(struct sys_device struct sysdev_driver * drv; mutex_lock(_drivers_lock); - list_for_each_entry(drv, _drivers, entry) { - if (drv->remove) - drv->remove(sysdev); - } - list_for_each_entry(drv, >cls->drivers, entry) { if (drv->remove) drv->remove(sysdev); @@ -320,12 +310,6 @@ void sysdev_shutdown(void) struct sysdev_driver * drv; pr_debug(" %s\n", kobject_name(>kobj)); - /* Call global drivers first. */ - list_for_each_entry(drv, _drivers, entry) { - if (drv->shutdown) - drv->shutdown(sysdev); - } - /* Call auxillary drivers next. */ list_for_each_entry(drv, >drivers, entry) { if (drv->shutdown) @@ -354,12 +338,6 @@ static void __sysdev_resume(struct sys_d if (drv->resume) drv->resume(dev); } - - /* Call global drivers. */ - list_for_each_entry(drv, _drivers, entry) { - if (drv->resume) - drv->resume(dev); - } } /** @@ -393,15 +371,6 @@ int sysdev_suspend(pm_message_t state) list_for_each_entry(sysdev, >kset.list, kobj.entry) { pr_debug(" %s\n", kobject_name(>kobj)); - /* Call global drivers first. */ - list_for_each_entry(drv, _drivers, entry) { - if (drv->suspend) { -
[PATCH] sysdev: remove global sysdev drivers list
No one uses sysdev_drivers. Because no one calls sysdev_driver_register with NULL class. And it is difficult to imagine that someone want to implement a global sysdev driver which is called with all sys_device on any kind of sysdev_class. So this patch removes global sysdev_drivers list. Cc: Tejun Heo [EMAIL PROTECTED] Cc: Cornelia Huck [EMAIL PROTECTED] Cc: Greg Kroah-Hartman [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- drivers/base/sys.c | 66 + 1 file changed, 12 insertions(+), 54 deletions(-) Index: 2.6-git/drivers/base/sys.c === --- 2.6-git.orig/drivers/base/sys.c +++ 2.6-git/drivers/base/sys.c @@ -153,25 +153,22 @@ void sysdev_class_unregister(struct sysd EXPORT_SYMBOL_GPL(sysdev_class_register); EXPORT_SYMBOL_GPL(sysdev_class_unregister); - -static LIST_HEAD(sysdev_drivers); static DEFINE_MUTEX(sysdev_drivers_lock); /** * sysdev_driver_register - Register auxillary driver - * @cls: Device class driver belongs to. + * @cls: Device class driver belongs to. * @drv: Driver. * - * If @cls is valid, then @drv is inserted into @cls-drivers to be + * @drv is inserted into @cls-drivers to be * called on each operation on devices of that class. The refcount * of @cls is incremented. - * Otherwise, @drv is inserted into sysdev_drivers, and called for - * each device. */ -int sysdev_driver_register(struct sysdev_class * cls, - struct sysdev_driver * drv) +int sysdev_driver_register(struct sysdev_class *cls, struct sysdev_driver *drv) { + int err = 0; + mutex_lock(sysdev_drivers_lock); if (cls kset_get(cls-kset)) { list_add_tail(drv-entry, cls-drivers); @@ -182,10 +179,12 @@ int sysdev_driver_register(struct sysdev list_for_each_entry(dev, cls-kset.list, kobj.entry) drv-add(dev); } - } else - list_add_tail(drv-entry, sysdev_drivers); + } else { + err = -EINVAL; + WARN_ON(1); + } mutex_unlock(sysdev_drivers_lock); - return 0; + return err; } @@ -206,6 +205,8 @@ void sysdev_driver_unregister(struct sys drv-remove(dev); } kset_put(cls-kset); + } else { + WARN_ON(1); } mutex_unlock(sysdev_drivers_lock); } @@ -251,12 +252,6 @@ int sysdev_register(struct sys_device * * code that should have called us. */ - /* Notify global drivers */ - list_for_each_entry(drv, sysdev_drivers, entry) { - if (drv-add) - drv-add(sysdev); - } - /* Notify class auxillary drivers */ list_for_each_entry(drv, cls-drivers, entry) { if (drv-add) @@ -272,11 +267,6 @@ void sysdev_unregister(struct sys_device struct sysdev_driver * drv; mutex_lock(sysdev_drivers_lock); - list_for_each_entry(drv, sysdev_drivers, entry) { - if (drv-remove) - drv-remove(sysdev); - } - list_for_each_entry(drv, sysdev-cls-drivers, entry) { if (drv-remove) drv-remove(sysdev); @@ -320,12 +310,6 @@ void sysdev_shutdown(void) struct sysdev_driver * drv; pr_debug( %s\n, kobject_name(sysdev-kobj)); - /* Call global drivers first. */ - list_for_each_entry(drv, sysdev_drivers, entry) { - if (drv-shutdown) - drv-shutdown(sysdev); - } - /* Call auxillary drivers next. */ list_for_each_entry(drv, cls-drivers, entry) { if (drv-shutdown) @@ -354,12 +338,6 @@ static void __sysdev_resume(struct sys_d if (drv-resume) drv-resume(dev); } - - /* Call global drivers. */ - list_for_each_entry(drv, sysdev_drivers, entry) { - if (drv-resume) - drv-resume(dev); - } } /** @@ -393,15 +371,6 @@ int sysdev_suspend(pm_message_t state) list_for_each_entry(sysdev, cls-kset.list, kobj.entry) { pr_debug( %s\n, kobject_name(sysdev-kobj)); - /* Call global drivers first. */ - list_for_each_entry(drv, sysdev_drivers, entry) { - if (drv-suspend) { - ret = drv-suspend(sysdev, state); - if (ret
Re: [linux-usb-devel] 2.6.23-rc1-mm2 + cpufreq patch + hot-fixes -- [] usb_stor_scan_thread+0xbd/0x15a [usb_storage]
2007/8/6, Alan Stern <[EMAIL PROTECTED]>: > On Sat, 4 Aug 2007, Miles Lane wrote: > > > Initializing USB Mass Storage driver... > > usb-storage 4-3:1.0: usb_probe_interface > > usb-storage 4-3:1.0: usb_probe_interface - got id > > scsi2 : SCSI emulation for USB Mass Storage devices > > usbcore: registered new interface driver usb-storage > > usb-storage: device found at 2 > > usb-storage: waiting for device to settle before scanning > > schedule_timeout: wrong timeout value f8ea51d2 > > [] show_trace_log_lvl+0x12/0x25 > > [] show_trace+0xd/0x10 > > [] dump_stack+0x16/0x18 > > [] schedule_timeout+0x2c/0x8b > > [] usb_stor_scan_thread+0xbd/0x15a [usb_storage] > > [] kthread+0x3b/0x63 > > [] kernel_thread_helper+0x7/0x10 > > === > > Does this happen repeatably? > > Did you set usb-storage's delay_use parameter to something peculiar? I also have same problem. It is caused by http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc1/2.6.23-rc1-mm2/broken-out/freezer-introduce-freezer-firendly-waiting-macros.patch The patch below may not be good fix. But it shows what is problem. Index: 2.6-mm/include/linux/freezer.h === --- 2.6-mm.orig/include/linux/freezer.h +++ 2.6-mm/include/linux/freezer.h @@ -149,13 +149,13 @@ static inline void set_freezable(void) #define wait_event_freezable_timeout(wq, condition, timeout) \ ({ \ - long __ret = timeout; \ + long ret = timeout; \ do {\ - __ret = wait_event_interruptible_timeout(wq,\ + ret = wait_event_interruptible_timeout(wq, \ (condition) || freezing(current), \ - __ret); \ + ret); \ } while (try_to_freeze()); \ - __ret; \ + ret;\ }) #else /* !CONFIG_PM_SLEEP */ static inline int frozen(struct task_struct *p) { return 0; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] 2.6.23-rc1-mm2 + cpufreq patch + hot-fixes -- [f8ea528f] usb_stor_scan_thread+0xbd/0x15a [usb_storage]
2007/8/6, Alan Stern [EMAIL PROTECTED]: On Sat, 4 Aug 2007, Miles Lane wrote: Initializing USB Mass Storage driver... usb-storage 4-3:1.0: usb_probe_interface usb-storage 4-3:1.0: usb_probe_interface - got id scsi2 : SCSI emulation for USB Mass Storage devices usbcore: registered new interface driver usb-storage usb-storage: device found at 2 usb-storage: waiting for device to settle before scanning schedule_timeout: wrong timeout value f8ea51d2 [c01080ab] show_trace_log_lvl+0x12/0x25 [c0108a9e] show_trace+0xd/0x10 [c0108bac] dump_stack+0x16/0x18 [c031e31e] schedule_timeout+0x2c/0x8b [f8ea528f] usb_stor_scan_thread+0xbd/0x15a [usb_storage] [c0139d64] kthread+0x3b/0x63 [c0107c63] kernel_thread_helper+0x7/0x10 === Does this happen repeatably? Did you set usb-storage's delay_use parameter to something peculiar? I also have same problem. It is caused by http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc1/2.6.23-rc1-mm2/broken-out/freezer-introduce-freezer-firendly-waiting-macros.patch The patch below may not be good fix. But it shows what is problem. Index: 2.6-mm/include/linux/freezer.h === --- 2.6-mm.orig/include/linux/freezer.h +++ 2.6-mm/include/linux/freezer.h @@ -149,13 +149,13 @@ static inline void set_freezable(void) #define wait_event_freezable_timeout(wq, condition, timeout) \ ({ \ - long __ret = timeout; \ + long ret = timeout; \ do {\ - __ret = wait_event_interruptible_timeout(wq,\ + ret = wait_event_interruptible_timeout(wq, \ (condition) || freezing(current), \ - __ret); \ + ret); \ } while (try_to_freeze()); \ - __ret; \ + ret;\ }) #else /* !CONFIG_PM_SLEEP */ static inline int frozen(struct task_struct *p) { return 0; } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Crypto API Weirdnesses
> > "Failed to setup dm-crypt key mapping. > > Check kernel for support for the aes-cbc-essiv:sha256 cipher spec and > > verify that /dev/hda4 contains at least 133 sectors. > > Failed to read from key storage" > > Looks like a "cryptsetup" error message, not kernel's. I can't find CONFIG_DM_CRYPT in the .config. Maybe cryptsetup needs it. We should improve cryptsetup error message. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Crypto API Weirdnesses
Failed to setup dm-crypt key mapping. Check kernel for support for the aes-cbc-essiv:sha256 cipher spec and verify that /dev/hda4 contains at least 133 sectors. Failed to read from key storage Looks like a cryptsetup error message, not kernel's. I can't find CONFIG_DM_CRYPT in the .config. Maybe cryptsetup needs it. We should improve cryptsetup error message. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 9/9] intel_cacheinfo: fix cpu hotplug error handling
- Fix resource leakage in error case within detect_cache_attributes() - Don't register hotcpu notifier when cache_add_dev() returns error - Introduce cache_dev_map cpumask to track whether cache interface for CPU is successfully added by cache_add_dev() or not. cache_add_dev() may fail with out of memory error. In order to avoid cache_remove_dev() with that uninitialized cache interface when CPU_DEAD event is delivered we need to have the cache_dev_map cpumask. (We cannot change cache_add_dev() from CPU_ONLINE event handler to CPU_UP_PREPARE event handler. Because cache_add_dev() needs to do cpuid and store the results with its CPU online.) Cc: Ashok Raj <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- arch/i386/kernel/cpu/intel_cacheinfo.c | 64 +++-- 1 file changed, 45 insertions(+), 19 deletions(-) Index: 2.6-git/arch/i386/kernel/cpu/intel_cacheinfo.c === --- 2.6-git.orig/arch/i386/kernel/cpu/intel_cacheinfo.c +++ 2.6-git/arch/i386/kernel/cpu/intel_cacheinfo.c @@ -466,6 +466,11 @@ static void __init cache_remove_shared_c static void free_cache_attributes(unsigned int cpu) { + int i; + + for (i = 0; i < num_cache_leaves; i++) + cache_remove_shared_cpu_map(cpu, i); + kfree(cpuid4_info[cpu]); cpuid4_info[cpu] = NULL; } @@ -473,8 +478,8 @@ static void free_cache_attributes(unsign static int __cpuinit detect_cache_attributes(unsigned int cpu) { struct _cpuid4_info *this_leaf; - unsigned long j; - int retval; + unsigned long j; + int retval; cpumask_t oldmask; if (num_cache_leaves == 0) @@ -491,19 +496,26 @@ static int __cpuinit detect_cache_attrib goto out; /* Do cpuid and store the results */ - retval = 0; for (j = 0; j < num_cache_leaves; j++) { this_leaf = CPUID4_INFO_IDX(cpu, j); retval = cpuid4_cache_lookup(j, this_leaf); - if (unlikely(retval < 0)) + if (unlikely(retval < 0)) { + int i; + + for (i = 0; i < j; i++) + cache_remove_shared_cpu_map(cpu, i); break; + } cache_shared_cpu_map_setup(cpu, j); } set_cpus_allowed(current, oldmask); out: - if (retval) - free_cache_attributes(cpu); + if (retval) { + kfree(cpuid4_info[cpu]); + cpuid4_info[cpu] = NULL; + } + return retval; } @@ -647,13 +659,14 @@ static void cpuid4_cache_sysfs_exit(unsi static int __cpuinit cpuid4_cache_sysfs_init(unsigned int cpu) { + int err; if (num_cache_leaves == 0) return -ENOENT; - detect_cache_attributes(cpu); - if (cpuid4_info[cpu] == NULL) - return -ENOENT; + err = detect_cache_attributes(cpu); + if (err) + return err; /* Allocate all required memory */ cache_kobject[cpu] = kzalloc(sizeof(struct kobject), GFP_KERNEL); @@ -672,13 +685,15 @@ err_out: return -ENOMEM; } +static cpumask_t cache_dev_map = CPU_MASK_NONE; + /* Add/Remove cache interface for CPU device */ static int __cpuinit cache_add_dev(struct sys_device * sys_dev) { unsigned int cpu = sys_dev->id; unsigned long i, j; struct _index_kobject *this_object; - int retval = 0; + int retval; retval = cpuid4_cache_sysfs_init(cpu); if (unlikely(retval < 0)) @@ -688,6 +703,10 @@ static int __cpuinit cache_add_dev(struc kobject_set_name(cache_kobject[cpu], "%s", "cache"); cache_kobject[cpu]->ktype = _percpu_entry; retval = kobject_register(cache_kobject[cpu]); + if (retval < 0) { + cpuid4_cache_sysfs_exit(cpu); + return retval; + } for (i = 0; i < num_cache_leaves; i++) { this_object = INDEX_KOBJECT_PTR(cpu,i); @@ -707,6 +726,9 @@ static int __cpuinit cache_add_dev(struc break; } } + if (!retval) + cpu_set(cpu, cache_dev_map); + return retval; } @@ -715,13 +737,14 @@ static void __cpuexit cache_remove_dev(s unsigned int cpu = sys_dev->id; unsigned long i; - for (i = 0; i < num_cache_leaves; i++) { - cache_remove_shared_cpu_map(cpu, i); + if (!cpu_isset(cpu, cache_dev_map)) + return; + cpu_clear(cpu, cache_dev_map); + + for (i = 0; i < num_cache_leaves; i++) kobject_unregister(&(INDEX_KOBJECT_PTR(cpu,i)->kobj)); - } kobject_unregi
[patch 6/9] msr: fix cpu hotplug error handling
Do msr_device_create() in CPU_UP_PREPARE instead of CPU_ONLINE. Cc: "H. Peter Anvin" <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- arch/i386/kernel/msr.c | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) Index: 2.6-git/arch/i386/kernel/msr.c === --- 2.6-git.orig/arch/i386/kernel/msr.c +++ 2.6-git/arch/i386/kernel/msr.c @@ -135,33 +135,39 @@ static const struct file_operations msr_ .open = msr_open, }; -static int msr_device_create(int i) +static int msr_device_create(int cpu) { - int err = 0; struct device *dev; - dev = device_create(msr_class, NULL, MKDEV(MSR_MAJOR, i), "msr%d",i); - if (IS_ERR(dev)) - err = PTR_ERR(dev); - return err; + dev = device_create(msr_class, NULL, MKDEV(MSR_MAJOR, cpu), + "msr%d", cpu); + return IS_ERR(dev) ? PTR_ERR(dev) : 0; +} + +static void msr_device_destroy(int cpu) +{ + device_destroy(msr_class, MKDEV(MSR_MAJOR, cpu)); } static int msr_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; + int err = 0; switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: - msr_device_create(cpu); + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: + err = msr_device_create(cpu); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: - device_destroy(msr_class, MKDEV(MSR_MAJOR, cpu)); + msr_device_destroy(cpu); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block __cpuinitdata msr_class_cpu_notifier = @@ -198,7 +204,7 @@ static int __init msr_init(void) out_class: i = 0; for_each_online_cpu(i) - device_destroy(msr_class, MKDEV(MSR_MAJOR, i)); + msr_device_destroy(i); class_destroy(msr_class); out_chrdev: unregister_chrdev(MSR_MAJOR, "cpu/msr"); @@ -210,7 +216,7 @@ static void __exit msr_exit(void) { int cpu = 0; for_each_online_cpu(cpu) - device_destroy(msr_class, MKDEV(MSR_MAJOR, cpu)); + msr_device_destroy(cpu); class_destroy(msr_class); unregister_chrdev(MSR_MAJOR, "cpu/msr"); unregister_hotcpu_notifier(_class_cpu_notifier); -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 7/9] cpuid: fix cpu hotplug error handling
Do cpuid_device_create() in CPU_UP_PREPARE instead of CPU_ONLINE. Cc: "H. Peter Anvin" <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- arch/i386/kernel/cpuid.c | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) Index: 2.6-git/arch/i386/kernel/cpuid.c === --- 2.6-git.orig/arch/i386/kernel/cpuid.c +++ 2.6-git/arch/i386/kernel/cpuid.c @@ -152,32 +152,38 @@ static const struct file_operations cpui .open = cpuid_open, }; -static int cpuid_device_create(int i) +static int cpuid_device_create(int cpu) { - int err = 0; struct device *dev; - dev = device_create(cpuid_class, NULL, MKDEV(CPUID_MAJOR, i), "cpu%d",i); - if (IS_ERR(dev)) - err = PTR_ERR(dev); - return err; + dev = device_create(cpuid_class, NULL, MKDEV(CPUID_MAJOR, cpu), + "cpu%d", cpu); + return IS_ERR(dev) ? PTR_ERR(dev) : 0; +} + +static void cpuid_device_destroy(int cpu) +{ + device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, cpu)); } static int cpuid_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; + int err = 0; switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: - cpuid_device_create(cpu); + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: + err = cpuid_device_create(cpu); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: - device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, cpu)); + cpuid_device_destroy(cpu); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block __cpuinitdata cpuid_class_cpu_notifier = @@ -214,7 +220,7 @@ static int __init cpuid_init(void) out_class: i = 0; for_each_online_cpu(i) { - device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, i)); + cpuid_device_destroy(i); } class_destroy(cpuid_class); out_chrdev: @@ -228,7 +234,7 @@ static void __exit cpuid_exit(void) int cpu = 0; for_each_online_cpu(cpu) - device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, cpu)); + cpuid_device_destroy(cpu); class_destroy(cpuid_class); unregister_chrdev(CPUID_MAJOR, "cpu/cpuid"); unregister_hotcpu_notifier(_class_cpu_notifier); -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 8/9] mce: fix cpu hotplug error handling
- Clear kobject in percpu device_mce before calling sysdev_register() with Because mce_create_device() may fail and it leaves kobject filled with junk. It will be the problem when mce_create_device() will be called next time. - Fix error handling in mce_create_device() Error handling should not do sysdev_remove_file() with not yet added attributes. - Don't register hotcpu notifier when mce_create_device() returns error - Do mce_create_device() in CPU_UP_PREPARE instead of CPU_ONLINE Cc: Andi Kleen <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- arch/x86_64/kernel/mce.c | 39 +-- 1 file changed, 29 insertions(+), 10 deletions(-) Index: 2.6-git/arch/x86_64/kernel/mce.c === --- 2.6-git.orig/arch/x86_64/kernel/mce.c +++ 2.6-git/arch/x86_64/kernel/mce.c @@ -690,16 +690,29 @@ static __cpuinit int mce_create_device(u if (!mce_available(_data[cpu])) return -EIO; + memset(_cpu(device_mce, cpu).kobj, 0, sizeof(struct kobject)); per_cpu(device_mce,cpu).id = cpu; per_cpu(device_mce,cpu).cls = _sysclass; err = sysdev_register(_cpu(device_mce,cpu)); + if (err) + return err; + + for (i = 0; mce_attributes[i]; i++) { + err = sysdev_create_file(_cpu(device_mce,cpu), +mce_attributes[i]); + if (err) + goto error; + } - if (!err) { - for (i = 0; mce_attributes[i]; i++) - sysdev_create_file(_cpu(device_mce,cpu), - mce_attributes[i]); + return 0; +error: + while (i--) { + sysdev_remove_file(_cpu(device_mce,cpu), + mce_attributes[i]); } + sysdev_unregister(_cpu(device_mce,cpu)); + return err; } @@ -711,7 +724,6 @@ static void mce_remove_device(unsigned i sysdev_remove_file(_cpu(device_mce,cpu), mce_attributes[i]); sysdev_unregister(_cpu(device_mce,cpu)); - memset(_cpu(device_mce, cpu).kobj, 0, sizeof(struct kobject)); } /* Get notified when a cpu comes on/off. Be hotplug friendly. */ @@ -719,18 +731,21 @@ static int mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; + int err = 0; switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: - mce_create_device(cpu); + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: + err = mce_create_device(cpu); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: mce_remove_device(cpu); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block mce_cpu_notifier = { @@ -745,9 +760,13 @@ static __init int mce_init_device(void) if (!mce_available(_cpu_data)) return -EIO; err = sysdev_class_register(_sysclass); + if (err) + return err; for_each_online_cpu(i) { - mce_create_device(i); + err = mce_create_device(i); + if (err) + return err; } register_hotcpu_notifier(_cpu_notifier); -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 3/9] cpu: deliver CPU_UP_CANCELED only to NOTIFY_OKed callbacks with CPU_UP_PREPARE
From: Akinobu Mita <[EMAIL PROTECTED]> The functions in a CPU notifier chain is called with CPU_UP_PREPARE event before making the CPU online. If one of the callback returns NOTIFY_BAD, it stops to deliver CPU_UP_PREPARE event, and CPU online operation is canceled. Then CPU_UP_CANCELED event is delivered to the functions in a CPU notifier chain again. This CPU_UP_CANCELED event is delivered to the functions which have been called with CPU_UP_PREPARE, not delivered to the functions which haven't been called with CPU_UP_PREPARE. The problem that makes existing cpu hotplug error handlings complex is that the CPU_UP_CANCELED event is delivered to the function that has returned NOTIFY_BAD, too. Usually we don't expect to call destructor function against the object that has failed to initialize. It is like: err = register_something(); if (err) { unregister_something(); return err; } So it is natural to deliver CPU_UP_CANCELED event only to the functions that have returned NOTIFY_OK with CPU_UP_PREPARE event and not to call the function that have returned NOTIFY_BAD. This is what this patch is doing. Otherwise, every cpu hotplug notifiler has to track whether notifiler event is failed or not for each cpu. (drivers/base/topology.c is doing this with topology_dev_map) Similary this patch makes same thing with CPU_DOWN_PREPARE and CPU_DOWN_FAILED evnets. Cc: Rusty Russell <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- kernel/cpu.c |2 ++ 1 file changed, 2 insertions(+) Index: 2.6-git/kernel/cpu.c === --- 2.6-git.orig/kernel/cpu.c +++ 2.6-git/kernel/cpu.c @@ -150,6 +150,7 @@ static int _cpu_down(unsigned int cpu, i err = __raw_notifier_call_chain(_chain, CPU_DOWN_PREPARE | mod, hcpu, -1, _calls); if (err == NOTIFY_BAD) { + nr_calls--; __raw_notifier_call_chain(_chain, CPU_DOWN_FAILED | mod, hcpu, nr_calls, NULL); printk("%s: attempt to take down CPU %u failed\n", @@ -233,6 +234,7 @@ static int __cpuinit _cpu_up(unsigned in ret = __raw_notifier_call_chain(_chain, CPU_UP_PREPARE | mod, hcpu, -1, _calls); if (ret == NOTIFY_BAD) { + nr_calls--; printk("%s: attempt to bring up CPU %u failed\n", __FUNCTION__, cpu); ret = -EINVAL; -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 4/9] topology: remove topology_dev_map
By previous cpu hotplug notifier change, we don't need to track topology_dev existence for each cpu by topology_dev_map. Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- drivers/base/topology.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) Index: 2.6-git/drivers/base/topology.c === --- 2.6-git.orig/drivers/base/topology.c +++ 2.6-git/drivers/base/topology.c @@ -94,27 +94,18 @@ static struct attribute_group topology_a .name = "topology" }; -static cpumask_t topology_dev_map = CPU_MASK_NONE; - /* Add/Remove cpu_topology interface for CPU device */ static int __cpuinit topology_add_dev(unsigned int cpu) { - int rc; struct sys_device *sys_dev = get_cpu_sysdev(cpu); - rc = sysfs_create_group(_dev->kobj, _attr_group); - if (!rc) - cpu_set(cpu, topology_dev_map); - return rc; + return sysfs_create_group(_dev->kobj, _attr_group); } static void __cpuinit topology_remove_dev(unsigned int cpu) { struct sys_device *sys_dev = get_cpu_sysdev(cpu); - if (!cpu_isset(cpu, topology_dev_map)) - return; - cpu_clear(cpu, topology_dev_map); sysfs_remove_group(_dev->kobj, _attr_group); } -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 5/9] thermal_throttle: fix cpu hotplug error handling
Do thermal_throttle_add_dev() in CPU_UP_PREPARE instead of CPU_ONLINE. Cc: Dmitriy Zavin <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- arch/i386/kernel/cpu/mcheck/therm_throt.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) Index: 2.6-git/arch/i386/kernel/cpu/mcheck/therm_throt.c === --- 2.6-git.orig/arch/i386/kernel/cpu/mcheck/therm_throt.c +++ 2.6-git/arch/i386/kernel/cpu/mcheck/therm_throt.c @@ -131,17 +131,19 @@ static __cpuinit int thermal_throttle_cp { unsigned int cpu = (unsigned long)hcpu; struct sys_device *sys_dev; - int err; + int err = 0; sys_dev = get_cpu_sysdev(cpu); switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: mutex_lock(_cpu_lock); err = thermal_throttle_add_dev(sys_dev); mutex_unlock(_cpu_lock); WARN_ON(err); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: mutex_lock(_cpu_lock); @@ -149,7 +151,7 @@ static __cpuinit int thermal_throttle_cp mutex_unlock(_cpu_lock); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block thermal_throttle_cpu_notifier = -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 2/9] slab: fix memory leak in cpu hotplug error path
This patch fixes memory leak in error path. In reality, we don't need to call cpuup_canceled(cpu) for now. But upcoming cpu hotplug error handling change needs this. Cc: Christoph Lameter <[EMAIL PROTECTED]> Cc: Gautham R Shenoy <[EMAIL PROTECTED]> Cc: Pekka Enberg <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- mm/slab.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) Index: 2.6-git/mm/slab.c === --- 2.6-git.orig/mm/slab.c +++ 2.6-git/mm/slab.c @@ -1282,13 +1282,18 @@ static int __cpuinit cpuup_prepare(long shared = alloc_arraycache(node, cachep->shared * cachep->batchcount, 0xbaadf00d); - if (!shared) + if (!shared) { + kfree(nc); goto bad; + } } if (use_alien_caches) { alien = alloc_alien_cache(node, cachep->limit); - if (!alien) + if (!alien) { + kfree(shared); + kfree(nc); goto bad; + } } cachep->array[cpu] = nc; l3 = cachep->nodelists[node]; @@ -1315,6 +1320,7 @@ static int __cpuinit cpuup_prepare(long } return 0; bad: + cpuup_canceled(cpu); return -ENOMEM; } -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 0/9] CPU hotplug error handling fixes take2
This series of patches fixes the error handling for cpu hotplug. The problem is revealed by CPU hotplug/unplug test with fault-injection. The changes from previous patchset: - Removed the patch titled sysfs: avoid kmem_cache_free(NULL) Because it was merged into mainline. - Removed the patch titled sysdev: add error check in sysdev_register() It turned out that the patch needs more fixes for microcode and cpufreq. So I'll separate them from this series of patches. - Added slab fix noticed by Gautham R Shenoy The series of patches: [patch 1/9] slab: cleanup cpuup_callback() [patch 2/9] slab: fix memory leak in cpu hotplug error path [patch 3/9] cpu: deliver CPU_UP_CANCELED only to NOTIFY_OKed callbacks with CPU_UP_PREPARE [patch 4/9] topology: remove topology_dev_map [patch 5/9] thermal_throttle: fix cpu hotplug error handling [patch 6/9] msr: fix cpu hotplug error handling [patch 7/9] cpuid: fix cpu hotplug error handling [patch 8/9] mce: fix cpu hotplug error handling [patch 9/9] intel_cacheinfo: fix cpu hotplug error handling The patch 1/9 factors out some code from cpuup_callback() to simplify what the patch 2/9 does. The patch 2/9 fixes memory leak in allocation failure path. Also it avoids the memory leak that will be introduced by the patch 3/9. The patch 3/9 changes the behavior when one of the callbacks in notifier chain returns NOTIFY_BAD with CPU_UP_PREPARE event. This change makes cpu hotplug error handling simple. The patch 4/9 simplifies the cpu hotplug event handling in topology.c enabled by the patch 3/9. The patch 5-9/9 are cpu hotplug error handling fixes in drivers. Mainly these patches shift some callbacks from CPU_ONLINE to CPU_UP_PREPARE. CPU hotplug/unplug test script: --[ cut here ]-- #!/bin/bash FAILTYPE=failslab CPU=1 CPU_ONLINE=/sys/devices/system/cpu/cpu${CPU}/online faulty_system() { bash -c "echo 1 > /proc/self/make-it-fail && exec $*" } [ "$UID" == 0 ] || exit 1 [ -n "$FAILTYPE" -a -f /debug/$FAILTYPE/probability ] || exit 1 [ -f $CPU_ONLINE ] || exit 1 echo N > /debug/$FAILTYPE/ignore-gfp-wait echo Y > /debug/$FAILTYPE/task-filter echo 1 > /debug/$FAILTYPE/probability echo -1 > /debug/$FAILTYPE/times while true do faulty_system "echo 0 > $CPU_ONLINE" faulty_system "echo 1 > $CPU_ONLINE" done - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 1/9] slab: cleanup cpuup_callback()
cpuup_callback() is too long. This patch factors out CPU_UP_CANCELLED and CPU_UP_PREPARE handlings from cpuup_callback(). Cc: Christoph Lameter <[EMAIL PROTECTED]> Cc: Gautham R Shenoy <[EMAIL PROTECTED]> Cc: Pekka Enberg <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- mm/slab.c | 301 -- 1 file changed, 159 insertions(+), 142 deletions(-) Index: 2.6-git/mm/slab.c === --- 2.6-git.orig/mm/slab.c +++ 2.6-git/mm/slab.c @@ -1156,105 +1156,181 @@ static inline int cache_free_alien(struc } #endif -static int __cpuinit cpuup_callback(struct notifier_block *nfb, - unsigned long action, void *hcpu) +static void __cpuinit cpuup_canceled(long cpu) +{ + struct kmem_cache *cachep; + struct kmem_list3 *l3 = NULL; + int node = cpu_to_node(cpu); + + list_for_each_entry(cachep, _chain, next) { + struct array_cache *nc; + struct array_cache *shared; + struct array_cache **alien; + cpumask_t mask; + + mask = node_to_cpumask(node); + /* cpu is dead; no one can alloc from it. */ + nc = cachep->array[cpu]; + cachep->array[cpu] = NULL; + l3 = cachep->nodelists[node]; + + if (!l3) + goto free_array_cache; + + spin_lock_irq(>list_lock); + + /* Free limit for this kmem_list3 */ + l3->free_limit -= cachep->batchcount; + if (nc) + free_block(cachep, nc->entry, nc->avail, node); + + if (!cpus_empty(mask)) { + spin_unlock_irq(>list_lock); + goto free_array_cache; + } + + shared = l3->shared; + if (shared) { + free_block(cachep, shared->entry, + shared->avail, node); + l3->shared = NULL; + } + + alien = l3->alien; + l3->alien = NULL; + + spin_unlock_irq(>list_lock); + + kfree(shared); + if (alien) { + drain_alien_cache(cachep, alien); + free_alien_cache(alien); + } +free_array_cache: + kfree(nc); + } + /* +* In the previous loop, all the objects were freed to +* the respective cache's slabs, now we can go ahead and +* shrink each nodelist to its limit. +*/ + list_for_each_entry(cachep, _chain, next) { + l3 = cachep->nodelists[node]; + if (!l3) + continue; + drain_freelist(cachep, l3, l3->free_objects); + } +} + +static int __cpuinit cpuup_prepare(long cpu) { - long cpu = (long)hcpu; struct kmem_cache *cachep; struct kmem_list3 *l3 = NULL; int node = cpu_to_node(cpu); const int memsize = sizeof(struct kmem_list3); - switch (action) { - case CPU_LOCK_ACQUIRE: - mutex_lock(_chain_mutex); - break; - case CPU_UP_PREPARE: - case CPU_UP_PREPARE_FROZEN: + /* +* We need to do this right in the beginning since +* alloc_arraycache's are going to use this list. +* kmalloc_node allows us to add the slab to the right +* kmem_list3 and not this cpu's kmem_list3 +*/ + + list_for_each_entry(cachep, _chain, next) { /* -* We need to do this right in the beginning since -* alloc_arraycache's are going to use this list. -* kmalloc_node allows us to add the slab to the right -* kmem_list3 and not this cpu's kmem_list3 +* Set up the size64 kmemlist for cpu before we can +* begin anything. Make sure some other cpu on this +* node has not already allocated this */ + if (!cachep->nodelists[node]) { + l3 = kmalloc_node(memsize, GFP_KERNEL, node); + if (!l3) + goto bad; + kmem_list3_init(l3); + l3->next_reap = jiffies + REAPTIMEOUT_LIST3 + + ((unsigned long)cachep) % REAPTIMEOUT_LIST3; - list_for_each_entry(cachep, _chain, next) { /* -* Set up the size64 kmemlist for cpu before we can -* begin anything. Make sure some other cpu on this -* node has not already allocated this +* The l3s don't come and
[patch 0/9] CPU hotplug error handling fixes take2
This series of patches fixes the error handling for cpu hotplug. The problem is revealed by CPU hotplug/unplug test with fault-injection. The changes from previous patchset: - Removed the patch titled sysfs: avoid kmem_cache_free(NULL) Because it was merged into mainline. - Removed the patch titled sysdev: add error check in sysdev_register() It turned out that the patch needs more fixes for microcode and cpufreq. So I'll separate them from this series of patches. - Added slab fix noticed by Gautham R Shenoy The series of patches: [patch 1/9] slab: cleanup cpuup_callback() [patch 2/9] slab: fix memory leak in cpu hotplug error path [patch 3/9] cpu: deliver CPU_UP_CANCELED only to NOTIFY_OKed callbacks with CPU_UP_PREPARE [patch 4/9] topology: remove topology_dev_map [patch 5/9] thermal_throttle: fix cpu hotplug error handling [patch 6/9] msr: fix cpu hotplug error handling [patch 7/9] cpuid: fix cpu hotplug error handling [patch 8/9] mce: fix cpu hotplug error handling [patch 9/9] intel_cacheinfo: fix cpu hotplug error handling The patch 1/9 factors out some code from cpuup_callback() to simplify what the patch 2/9 does. The patch 2/9 fixes memory leak in allocation failure path. Also it avoids the memory leak that will be introduced by the patch 3/9. The patch 3/9 changes the behavior when one of the callbacks in notifier chain returns NOTIFY_BAD with CPU_UP_PREPARE event. This change makes cpu hotplug error handling simple. The patch 4/9 simplifies the cpu hotplug event handling in topology.c enabled by the patch 3/9. The patch 5-9/9 are cpu hotplug error handling fixes in drivers. Mainly these patches shift some callbacks from CPU_ONLINE to CPU_UP_PREPARE. CPU hotplug/unplug test script: --[ cut here ]-- #!/bin/bash FAILTYPE=failslab CPU=1 CPU_ONLINE=/sys/devices/system/cpu/cpu${CPU}/online faulty_system() { bash -c echo 1 /proc/self/make-it-fail exec $* } [ $UID == 0 ] || exit 1 [ -n $FAILTYPE -a -f /debug/$FAILTYPE/probability ] || exit 1 [ -f $CPU_ONLINE ] || exit 1 echo N /debug/$FAILTYPE/ignore-gfp-wait echo Y /debug/$FAILTYPE/task-filter echo 1 /debug/$FAILTYPE/probability echo -1 /debug/$FAILTYPE/times while true do faulty_system echo 0 $CPU_ONLINE faulty_system echo 1 $CPU_ONLINE done - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 1/9] slab: cleanup cpuup_callback()
cpuup_callback() is too long. This patch factors out CPU_UP_CANCELLED and CPU_UP_PREPARE handlings from cpuup_callback(). Cc: Christoph Lameter [EMAIL PROTECTED] Cc: Gautham R Shenoy [EMAIL PROTECTED] Cc: Pekka Enberg [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- mm/slab.c | 301 -- 1 file changed, 159 insertions(+), 142 deletions(-) Index: 2.6-git/mm/slab.c === --- 2.6-git.orig/mm/slab.c +++ 2.6-git/mm/slab.c @@ -1156,105 +1156,181 @@ static inline int cache_free_alien(struc } #endif -static int __cpuinit cpuup_callback(struct notifier_block *nfb, - unsigned long action, void *hcpu) +static void __cpuinit cpuup_canceled(long cpu) +{ + struct kmem_cache *cachep; + struct kmem_list3 *l3 = NULL; + int node = cpu_to_node(cpu); + + list_for_each_entry(cachep, cache_chain, next) { + struct array_cache *nc; + struct array_cache *shared; + struct array_cache **alien; + cpumask_t mask; + + mask = node_to_cpumask(node); + /* cpu is dead; no one can alloc from it. */ + nc = cachep-array[cpu]; + cachep-array[cpu] = NULL; + l3 = cachep-nodelists[node]; + + if (!l3) + goto free_array_cache; + + spin_lock_irq(l3-list_lock); + + /* Free limit for this kmem_list3 */ + l3-free_limit -= cachep-batchcount; + if (nc) + free_block(cachep, nc-entry, nc-avail, node); + + if (!cpus_empty(mask)) { + spin_unlock_irq(l3-list_lock); + goto free_array_cache; + } + + shared = l3-shared; + if (shared) { + free_block(cachep, shared-entry, + shared-avail, node); + l3-shared = NULL; + } + + alien = l3-alien; + l3-alien = NULL; + + spin_unlock_irq(l3-list_lock); + + kfree(shared); + if (alien) { + drain_alien_cache(cachep, alien); + free_alien_cache(alien); + } +free_array_cache: + kfree(nc); + } + /* +* In the previous loop, all the objects were freed to +* the respective cache's slabs, now we can go ahead and +* shrink each nodelist to its limit. +*/ + list_for_each_entry(cachep, cache_chain, next) { + l3 = cachep-nodelists[node]; + if (!l3) + continue; + drain_freelist(cachep, l3, l3-free_objects); + } +} + +static int __cpuinit cpuup_prepare(long cpu) { - long cpu = (long)hcpu; struct kmem_cache *cachep; struct kmem_list3 *l3 = NULL; int node = cpu_to_node(cpu); const int memsize = sizeof(struct kmem_list3); - switch (action) { - case CPU_LOCK_ACQUIRE: - mutex_lock(cache_chain_mutex); - break; - case CPU_UP_PREPARE: - case CPU_UP_PREPARE_FROZEN: + /* +* We need to do this right in the beginning since +* alloc_arraycache's are going to use this list. +* kmalloc_node allows us to add the slab to the right +* kmem_list3 and not this cpu's kmem_list3 +*/ + + list_for_each_entry(cachep, cache_chain, next) { /* -* We need to do this right in the beginning since -* alloc_arraycache's are going to use this list. -* kmalloc_node allows us to add the slab to the right -* kmem_list3 and not this cpu's kmem_list3 +* Set up the size64 kmemlist for cpu before we can +* begin anything. Make sure some other cpu on this +* node has not already allocated this */ + if (!cachep-nodelists[node]) { + l3 = kmalloc_node(memsize, GFP_KERNEL, node); + if (!l3) + goto bad; + kmem_list3_init(l3); + l3-next_reap = jiffies + REAPTIMEOUT_LIST3 + + ((unsigned long)cachep) % REAPTIMEOUT_LIST3; - list_for_each_entry(cachep, cache_chain, next) { /* -* Set up the size64 kmemlist for cpu before we can -* begin anything. Make sure some other cpu on this -* node has not already allocated this +* The l3s don't come and go as CPUs come and +* go. cache_chain_mutex is sufficient
[patch 2/9] slab: fix memory leak in cpu hotplug error path
This patch fixes memory leak in error path. In reality, we don't need to call cpuup_canceled(cpu) for now. But upcoming cpu hotplug error handling change needs this. Cc: Christoph Lameter [EMAIL PROTECTED] Cc: Gautham R Shenoy [EMAIL PROTECTED] Cc: Pekka Enberg [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- mm/slab.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) Index: 2.6-git/mm/slab.c === --- 2.6-git.orig/mm/slab.c +++ 2.6-git/mm/slab.c @@ -1282,13 +1282,18 @@ static int __cpuinit cpuup_prepare(long shared = alloc_arraycache(node, cachep-shared * cachep-batchcount, 0xbaadf00d); - if (!shared) + if (!shared) { + kfree(nc); goto bad; + } } if (use_alien_caches) { alien = alloc_alien_cache(node, cachep-limit); - if (!alien) + if (!alien) { + kfree(shared); + kfree(nc); goto bad; + } } cachep-array[cpu] = nc; l3 = cachep-nodelists[node]; @@ -1315,6 +1320,7 @@ static int __cpuinit cpuup_prepare(long } return 0; bad: + cpuup_canceled(cpu); return -ENOMEM; } -- - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 3/9] cpu: deliver CPU_UP_CANCELED only to NOTIFY_OKed callbacks with CPU_UP_PREPARE
From: Akinobu Mita [EMAIL PROTECTED] The functions in a CPU notifier chain is called with CPU_UP_PREPARE event before making the CPU online. If one of the callback returns NOTIFY_BAD, it stops to deliver CPU_UP_PREPARE event, and CPU online operation is canceled. Then CPU_UP_CANCELED event is delivered to the functions in a CPU notifier chain again. This CPU_UP_CANCELED event is delivered to the functions which have been called with CPU_UP_PREPARE, not delivered to the functions which haven't been called with CPU_UP_PREPARE. The problem that makes existing cpu hotplug error handlings complex is that the CPU_UP_CANCELED event is delivered to the function that has returned NOTIFY_BAD, too. Usually we don't expect to call destructor function against the object that has failed to initialize. It is like: err = register_something(); if (err) { unregister_something(); return err; } So it is natural to deliver CPU_UP_CANCELED event only to the functions that have returned NOTIFY_OK with CPU_UP_PREPARE event and not to call the function that have returned NOTIFY_BAD. This is what this patch is doing. Otherwise, every cpu hotplug notifiler has to track whether notifiler event is failed or not for each cpu. (drivers/base/topology.c is doing this with topology_dev_map) Similary this patch makes same thing with CPU_DOWN_PREPARE and CPU_DOWN_FAILED evnets. Cc: Rusty Russell [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- kernel/cpu.c |2 ++ 1 file changed, 2 insertions(+) Index: 2.6-git/kernel/cpu.c === --- 2.6-git.orig/kernel/cpu.c +++ 2.6-git/kernel/cpu.c @@ -150,6 +150,7 @@ static int _cpu_down(unsigned int cpu, i err = __raw_notifier_call_chain(cpu_chain, CPU_DOWN_PREPARE | mod, hcpu, -1, nr_calls); if (err == NOTIFY_BAD) { + nr_calls--; __raw_notifier_call_chain(cpu_chain, CPU_DOWN_FAILED | mod, hcpu, nr_calls, NULL); printk(%s: attempt to take down CPU %u failed\n, @@ -233,6 +234,7 @@ static int __cpuinit _cpu_up(unsigned in ret = __raw_notifier_call_chain(cpu_chain, CPU_UP_PREPARE | mod, hcpu, -1, nr_calls); if (ret == NOTIFY_BAD) { + nr_calls--; printk(%s: attempt to bring up CPU %u failed\n, __FUNCTION__, cpu); ret = -EINVAL; -- - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 4/9] topology: remove topology_dev_map
By previous cpu hotplug notifier change, we don't need to track topology_dev existence for each cpu by topology_dev_map. Cc: Greg Kroah-Hartman [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- drivers/base/topology.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) Index: 2.6-git/drivers/base/topology.c === --- 2.6-git.orig/drivers/base/topology.c +++ 2.6-git/drivers/base/topology.c @@ -94,27 +94,18 @@ static struct attribute_group topology_a .name = topology }; -static cpumask_t topology_dev_map = CPU_MASK_NONE; - /* Add/Remove cpu_topology interface for CPU device */ static int __cpuinit topology_add_dev(unsigned int cpu) { - int rc; struct sys_device *sys_dev = get_cpu_sysdev(cpu); - rc = sysfs_create_group(sys_dev-kobj, topology_attr_group); - if (!rc) - cpu_set(cpu, topology_dev_map); - return rc; + return sysfs_create_group(sys_dev-kobj, topology_attr_group); } static void __cpuinit topology_remove_dev(unsigned int cpu) { struct sys_device *sys_dev = get_cpu_sysdev(cpu); - if (!cpu_isset(cpu, topology_dev_map)) - return; - cpu_clear(cpu, topology_dev_map); sysfs_remove_group(sys_dev-kobj, topology_attr_group); } -- - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 5/9] thermal_throttle: fix cpu hotplug error handling
Do thermal_throttle_add_dev() in CPU_UP_PREPARE instead of CPU_ONLINE. Cc: Dmitriy Zavin [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- arch/i386/kernel/cpu/mcheck/therm_throt.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) Index: 2.6-git/arch/i386/kernel/cpu/mcheck/therm_throt.c === --- 2.6-git.orig/arch/i386/kernel/cpu/mcheck/therm_throt.c +++ 2.6-git/arch/i386/kernel/cpu/mcheck/therm_throt.c @@ -131,17 +131,19 @@ static __cpuinit int thermal_throttle_cp { unsigned int cpu = (unsigned long)hcpu; struct sys_device *sys_dev; - int err; + int err = 0; sys_dev = get_cpu_sysdev(cpu); switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: mutex_lock(therm_cpu_lock); err = thermal_throttle_add_dev(sys_dev); mutex_unlock(therm_cpu_lock); WARN_ON(err); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: mutex_lock(therm_cpu_lock); @@ -149,7 +151,7 @@ static __cpuinit int thermal_throttle_cp mutex_unlock(therm_cpu_lock); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block thermal_throttle_cpu_notifier = -- - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 6/9] msr: fix cpu hotplug error handling
Do msr_device_create() in CPU_UP_PREPARE instead of CPU_ONLINE. Cc: H. Peter Anvin [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- arch/i386/kernel/msr.c | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) Index: 2.6-git/arch/i386/kernel/msr.c === --- 2.6-git.orig/arch/i386/kernel/msr.c +++ 2.6-git/arch/i386/kernel/msr.c @@ -135,33 +135,39 @@ static const struct file_operations msr_ .open = msr_open, }; -static int msr_device_create(int i) +static int msr_device_create(int cpu) { - int err = 0; struct device *dev; - dev = device_create(msr_class, NULL, MKDEV(MSR_MAJOR, i), msr%d,i); - if (IS_ERR(dev)) - err = PTR_ERR(dev); - return err; + dev = device_create(msr_class, NULL, MKDEV(MSR_MAJOR, cpu), + msr%d, cpu); + return IS_ERR(dev) ? PTR_ERR(dev) : 0; +} + +static void msr_device_destroy(int cpu) +{ + device_destroy(msr_class, MKDEV(MSR_MAJOR, cpu)); } static int msr_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; + int err = 0; switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: - msr_device_create(cpu); + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: + err = msr_device_create(cpu); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: - device_destroy(msr_class, MKDEV(MSR_MAJOR, cpu)); + msr_device_destroy(cpu); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block __cpuinitdata msr_class_cpu_notifier = @@ -198,7 +204,7 @@ static int __init msr_init(void) out_class: i = 0; for_each_online_cpu(i) - device_destroy(msr_class, MKDEV(MSR_MAJOR, i)); + msr_device_destroy(i); class_destroy(msr_class); out_chrdev: unregister_chrdev(MSR_MAJOR, cpu/msr); @@ -210,7 +216,7 @@ static void __exit msr_exit(void) { int cpu = 0; for_each_online_cpu(cpu) - device_destroy(msr_class, MKDEV(MSR_MAJOR, cpu)); + msr_device_destroy(cpu); class_destroy(msr_class); unregister_chrdev(MSR_MAJOR, cpu/msr); unregister_hotcpu_notifier(msr_class_cpu_notifier); -- - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 7/9] cpuid: fix cpu hotplug error handling
Do cpuid_device_create() in CPU_UP_PREPARE instead of CPU_ONLINE. Cc: H. Peter Anvin [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- arch/i386/kernel/cpuid.c | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) Index: 2.6-git/arch/i386/kernel/cpuid.c === --- 2.6-git.orig/arch/i386/kernel/cpuid.c +++ 2.6-git/arch/i386/kernel/cpuid.c @@ -152,32 +152,38 @@ static const struct file_operations cpui .open = cpuid_open, }; -static int cpuid_device_create(int i) +static int cpuid_device_create(int cpu) { - int err = 0; struct device *dev; - dev = device_create(cpuid_class, NULL, MKDEV(CPUID_MAJOR, i), cpu%d,i); - if (IS_ERR(dev)) - err = PTR_ERR(dev); - return err; + dev = device_create(cpuid_class, NULL, MKDEV(CPUID_MAJOR, cpu), + cpu%d, cpu); + return IS_ERR(dev) ? PTR_ERR(dev) : 0; +} + +static void cpuid_device_destroy(int cpu) +{ + device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, cpu)); } static int cpuid_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; + int err = 0; switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: - cpuid_device_create(cpu); + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: + err = cpuid_device_create(cpu); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: - device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, cpu)); + cpuid_device_destroy(cpu); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block __cpuinitdata cpuid_class_cpu_notifier = @@ -214,7 +220,7 @@ static int __init cpuid_init(void) out_class: i = 0; for_each_online_cpu(i) { - device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, i)); + cpuid_device_destroy(i); } class_destroy(cpuid_class); out_chrdev: @@ -228,7 +234,7 @@ static void __exit cpuid_exit(void) int cpu = 0; for_each_online_cpu(cpu) - device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, cpu)); + cpuid_device_destroy(cpu); class_destroy(cpuid_class); unregister_chrdev(CPUID_MAJOR, cpu/cpuid); unregister_hotcpu_notifier(cpuid_class_cpu_notifier); -- - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 8/9] mce: fix cpu hotplug error handling
- Clear kobject in percpu device_mce before calling sysdev_register() with Because mce_create_device() may fail and it leaves kobject filled with junk. It will be the problem when mce_create_device() will be called next time. - Fix error handling in mce_create_device() Error handling should not do sysdev_remove_file() with not yet added attributes. - Don't register hotcpu notifier when mce_create_device() returns error - Do mce_create_device() in CPU_UP_PREPARE instead of CPU_ONLINE Cc: Andi Kleen [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- arch/x86_64/kernel/mce.c | 39 +-- 1 file changed, 29 insertions(+), 10 deletions(-) Index: 2.6-git/arch/x86_64/kernel/mce.c === --- 2.6-git.orig/arch/x86_64/kernel/mce.c +++ 2.6-git/arch/x86_64/kernel/mce.c @@ -690,16 +690,29 @@ static __cpuinit int mce_create_device(u if (!mce_available(cpu_data[cpu])) return -EIO; + memset(per_cpu(device_mce, cpu).kobj, 0, sizeof(struct kobject)); per_cpu(device_mce,cpu).id = cpu; per_cpu(device_mce,cpu).cls = mce_sysclass; err = sysdev_register(per_cpu(device_mce,cpu)); + if (err) + return err; + + for (i = 0; mce_attributes[i]; i++) { + err = sysdev_create_file(per_cpu(device_mce,cpu), +mce_attributes[i]); + if (err) + goto error; + } - if (!err) { - for (i = 0; mce_attributes[i]; i++) - sysdev_create_file(per_cpu(device_mce,cpu), - mce_attributes[i]); + return 0; +error: + while (i--) { + sysdev_remove_file(per_cpu(device_mce,cpu), + mce_attributes[i]); } + sysdev_unregister(per_cpu(device_mce,cpu)); + return err; } @@ -711,7 +724,6 @@ static void mce_remove_device(unsigned i sysdev_remove_file(per_cpu(device_mce,cpu), mce_attributes[i]); sysdev_unregister(per_cpu(device_mce,cpu)); - memset(per_cpu(device_mce, cpu).kobj, 0, sizeof(struct kobject)); } /* Get notified when a cpu comes on/off. Be hotplug friendly. */ @@ -719,18 +731,21 @@ static int mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; + int err = 0; switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: - mce_create_device(cpu); + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: + err = mce_create_device(cpu); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: mce_remove_device(cpu); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block mce_cpu_notifier = { @@ -745,9 +760,13 @@ static __init int mce_init_device(void) if (!mce_available(boot_cpu_data)) return -EIO; err = sysdev_class_register(mce_sysclass); + if (err) + return err; for_each_online_cpu(i) { - mce_create_device(i); + err = mce_create_device(i); + if (err) + return err; } register_hotcpu_notifier(mce_cpu_notifier); -- - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 9/9] intel_cacheinfo: fix cpu hotplug error handling
- Fix resource leakage in error case within detect_cache_attributes() - Don't register hotcpu notifier when cache_add_dev() returns error - Introduce cache_dev_map cpumask to track whether cache interface for CPU is successfully added by cache_add_dev() or not. cache_add_dev() may fail with out of memory error. In order to avoid cache_remove_dev() with that uninitialized cache interface when CPU_DEAD event is delivered we need to have the cache_dev_map cpumask. (We cannot change cache_add_dev() from CPU_ONLINE event handler to CPU_UP_PREPARE event handler. Because cache_add_dev() needs to do cpuid and store the results with its CPU online.) Cc: Ashok Raj [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- arch/i386/kernel/cpu/intel_cacheinfo.c | 64 +++-- 1 file changed, 45 insertions(+), 19 deletions(-) Index: 2.6-git/arch/i386/kernel/cpu/intel_cacheinfo.c === --- 2.6-git.orig/arch/i386/kernel/cpu/intel_cacheinfo.c +++ 2.6-git/arch/i386/kernel/cpu/intel_cacheinfo.c @@ -466,6 +466,11 @@ static void __init cache_remove_shared_c static void free_cache_attributes(unsigned int cpu) { + int i; + + for (i = 0; i num_cache_leaves; i++) + cache_remove_shared_cpu_map(cpu, i); + kfree(cpuid4_info[cpu]); cpuid4_info[cpu] = NULL; } @@ -473,8 +478,8 @@ static void free_cache_attributes(unsign static int __cpuinit detect_cache_attributes(unsigned int cpu) { struct _cpuid4_info *this_leaf; - unsigned long j; - int retval; + unsigned long j; + int retval; cpumask_t oldmask; if (num_cache_leaves == 0) @@ -491,19 +496,26 @@ static int __cpuinit detect_cache_attrib goto out; /* Do cpuid and store the results */ - retval = 0; for (j = 0; j num_cache_leaves; j++) { this_leaf = CPUID4_INFO_IDX(cpu, j); retval = cpuid4_cache_lookup(j, this_leaf); - if (unlikely(retval 0)) + if (unlikely(retval 0)) { + int i; + + for (i = 0; i j; i++) + cache_remove_shared_cpu_map(cpu, i); break; + } cache_shared_cpu_map_setup(cpu, j); } set_cpus_allowed(current, oldmask); out: - if (retval) - free_cache_attributes(cpu); + if (retval) { + kfree(cpuid4_info[cpu]); + cpuid4_info[cpu] = NULL; + } + return retval; } @@ -647,13 +659,14 @@ static void cpuid4_cache_sysfs_exit(unsi static int __cpuinit cpuid4_cache_sysfs_init(unsigned int cpu) { + int err; if (num_cache_leaves == 0) return -ENOENT; - detect_cache_attributes(cpu); - if (cpuid4_info[cpu] == NULL) - return -ENOENT; + err = detect_cache_attributes(cpu); + if (err) + return err; /* Allocate all required memory */ cache_kobject[cpu] = kzalloc(sizeof(struct kobject), GFP_KERNEL); @@ -672,13 +685,15 @@ err_out: return -ENOMEM; } +static cpumask_t cache_dev_map = CPU_MASK_NONE; + /* Add/Remove cache interface for CPU device */ static int __cpuinit cache_add_dev(struct sys_device * sys_dev) { unsigned int cpu = sys_dev-id; unsigned long i, j; struct _index_kobject *this_object; - int retval = 0; + int retval; retval = cpuid4_cache_sysfs_init(cpu); if (unlikely(retval 0)) @@ -688,6 +703,10 @@ static int __cpuinit cache_add_dev(struc kobject_set_name(cache_kobject[cpu], %s, cache); cache_kobject[cpu]-ktype = ktype_percpu_entry; retval = kobject_register(cache_kobject[cpu]); + if (retval 0) { + cpuid4_cache_sysfs_exit(cpu); + return retval; + } for (i = 0; i num_cache_leaves; i++) { this_object = INDEX_KOBJECT_PTR(cpu,i); @@ -707,6 +726,9 @@ static int __cpuinit cache_add_dev(struc break; } } + if (!retval) + cpu_set(cpu, cache_dev_map); + return retval; } @@ -715,13 +737,14 @@ static void __cpuexit cache_remove_dev(s unsigned int cpu = sys_dev-id; unsigned long i; - for (i = 0; i num_cache_leaves; i++) { - cache_remove_shared_cpu_map(cpu, i); + if (!cpu_isset(cpu, cache_dev_map)) + return; + cpu_clear(cpu, cache_dev_map); + + for (i = 0; i num_cache_leaves; i++) kobject_unregister((INDEX_KOBJECT_PTR(cpu,i)-kobj)); - } kobject_unregister(cache_kobject[cpu]); cpuid4_cache_sysfs_exit(cpu); - return; } static int __cpuinit
Re: [PATCH 4/10] cpu: deliver CPU_UP_CANCELED only to NOTIFY_OKed callbacks with CPU_UP_PREPARE
> >[...] However, it might break slab. > >If I am not mistaken, slab code initializes multiple objects in > >CPU_UP_PREPARE and relies on the CPU_UP_CANCELLED to destroy the > >objects which successfully got initialized before the some object's > >initialization went bad. > > My testing machine is ordinary dual core non numa box. So it might not > trigger the problem that you are warried about under heavy slab alloc > failure injection. > > At first glance I couln't find the problem in cpu hottplug code in slab.c > yet, > but found some memory leak path. (it doesn't break slab though) That's what I meant. I shouldn't have used the word "break" :-) In case of slab, freeing up of resources on an error during CPU_UP_PREPARE, is currently handled in CPU_UP_CANCELLED. Now I perfectly understand your concern. The last memleak fix patch did not cover for each cachep->array[cpu] in cache_chain. So cpu hotplug error handling in slab becomes worse by this change. But, like you reasoned out, it makes more sense for such a subsystem to free up all the correctly allocated resources before sending a NOTIFY_BAD, rather than handling it in CPU_UP_CANCELLED. And slab needed that fix, which you've provided, before we send the notification to (nr_calls - 1) callers. So could you add this patch to series? Sure, and I'll CC you on the slab change. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/10] cpu: deliver CPU_UP_CANCELED only to NOTIFY_OKed callbacks with CPU_UP_PREPARE
[...] However, it might break slab. If I am not mistaken, slab code initializes multiple objects in CPU_UP_PREPARE and relies on the CPU_UP_CANCELLED to destroy the objects which successfully got initialized before the some object's initialization went bad. My testing machine is ordinary dual core non numa box. So it might not trigger the problem that you are warried about under heavy slab alloc failure injection. At first glance I couln't find the problem in cpu hottplug code in slab.c yet, but found some memory leak path. (it doesn't break slab though) That's what I meant. I shouldn't have used the word break :-) In case of slab, freeing up of resources on an error during CPU_UP_PREPARE, is currently handled in CPU_UP_CANCELLED. Now I perfectly understand your concern. The last memleak fix patch did not cover for each cachep-array[cpu] in cache_chain. So cpu hotplug error handling in slab becomes worse by this change. But, like you reasoned out, it makes more sense for such a subsystem to free up all the correctly allocated resources before sending a NOTIFY_BAD, rather than handling it in CPU_UP_CANCELLED. And slab needed that fix, which you've provided, before we send the notification to (nr_calls - 1) callers. So could you add this patch to series? Sure, and I'll CC you on the slab change. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/10] cpu: deliver CPU_UP_CANCELED only to NOTIFY_OKed callbacks with CPU_UP_PREPARE
> So it is natural to deliver CPU_UP_CANCELED event only to the functions > that have returned NOTIFY_OK with CPU_UP_PREPARE event and not to call > the function that have returned NOTIFY_BAD. This is what this patch is doing. Yes, this makes sense. Thank you for making sure of it. [...] However, it might break slab. If I am not mistaken, slab code initializes multiple objects in CPU_UP_PREPARE and relies on the CPU_UP_CANCELLED to destroy the objects which successfully got initialized before the some object's initialization went bad. My testing machine is ordinary dual core non numa box. So it might not trigger the problem that you are warried about under heavy slab alloc failure injection. At first glance I couln't find the problem in cpu hottplug code in slab.c yet, but found some memory leak path. (it doesn't break slab though) Index: 2.6-git/mm/slab.c === --- 2.6-git.orig/mm/slab.c +++ 2.6-git/mm/slab.c @@ -1221,13 +1221,18 @@ static int __cpuinit cpuup_callback(stru shared = alloc_arraycache(node, cachep->shared * cachep->batchcount, 0xbaadf00d); - if (!shared) + if (!shared) { + kfree(nc); goto bad; + } } if (use_alien_caches) { alien = alloc_alien_cache(node, cachep->limit); -if (!alien) -goto bad; +if (!alien) { + kfree(shared); + kfree(nc); + goto bad; + } } cachep->array[cpu] = nc; l3 = cachep->nodelists[node]; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/10] sysfs: fix error handling in create_files()
2007/7/17, Greg KH <[EMAIL PROTECTED]>: On Mon, Jul 16, 2007 at 05:29:45PM +0200, Cornelia Huck wrote: > On Mon, 16 Jul 2007 22:52:30 +0900, > Akinobu Mita <[EMAIL PROTECTED]> wrote: > > > Current error handling in create_files() attempts to remove > > all attributes passed by argument by remove_files(). But it should > > only remove the attributes that have been successfully added. > > While this is certainly cleaner, a question out of curiousity: Does > this fix any problem you saw? sysfs_hash_and_remove() used to be safe > on non-existing attributes... I agree, the existing code should work just fine, are you finding that it does not for some reason? As you and Cornelia said, this patch doesn't fix anything. I made a wrong guess for some oops call trace. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/10] sysfs: fix error handling in create_files()
2007/7/17, Greg KH [EMAIL PROTECTED]: On Mon, Jul 16, 2007 at 05:29:45PM +0200, Cornelia Huck wrote: On Mon, 16 Jul 2007 22:52:30 +0900, Akinobu Mita [EMAIL PROTECTED] wrote: Current error handling in create_files() attempts to remove all attributes passed by argument by remove_files(). But it should only remove the attributes that have been successfully added. While this is certainly cleaner, a question out of curiousity: Does this fix any problem you saw? sysfs_hash_and_remove() used to be safe on non-existing attributes... I agree, the existing code should work just fine, are you finding that it does not for some reason? As you and Cornelia said, this patch doesn't fix anything. I made a wrong guess for some oops call trace. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/10] cpu: deliver CPU_UP_CANCELED only to NOTIFY_OKed callbacks with CPU_UP_PREPARE
So it is natural to deliver CPU_UP_CANCELED event only to the functions that have returned NOTIFY_OK with CPU_UP_PREPARE event and not to call the function that have returned NOTIFY_BAD. This is what this patch is doing. Yes, this makes sense. Thank you for making sure of it. [...] However, it might break slab. If I am not mistaken, slab code initializes multiple objects in CPU_UP_PREPARE and relies on the CPU_UP_CANCELLED to destroy the objects which successfully got initialized before the some object's initialization went bad. My testing machine is ordinary dual core non numa box. So it might not trigger the problem that you are warried about under heavy slab alloc failure injection. At first glance I couln't find the problem in cpu hottplug code in slab.c yet, but found some memory leak path. (it doesn't break slab though) Index: 2.6-git/mm/slab.c === --- 2.6-git.orig/mm/slab.c +++ 2.6-git/mm/slab.c @@ -1221,13 +1221,18 @@ static int __cpuinit cpuup_callback(stru shared = alloc_arraycache(node, cachep-shared * cachep-batchcount, 0xbaadf00d); - if (!shared) + if (!shared) { + kfree(nc); goto bad; + } } if (use_alien_caches) { alien = alloc_alien_cache(node, cachep-limit); -if (!alien) -goto bad; +if (!alien) { + kfree(shared); + kfree(nc); + goto bad; + } } cachep-array[cpu] = nc; l3 = cachep-nodelists[node]; - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/10] sysfs: fix kmem_cache_free(NULL)
2007/7/17, Greg KH <[EMAIL PROTECTED]>: On Mon, Jul 16, 2007 at 10:50:28PM +0900, Akinobu Mita wrote: > This patch fixes out of memory error handling in sysfs_new_dirent(). > kmem_cache_free() with NULL is not allowed. Why not just allow kmem_cache_free() to allow NULL like other functions in the kernel? It seems there was discussion about this and no conclusion yet? http://lkml.org/lkml/2007/3/19/42 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/10] sysfs: fix error handling in create_files()
2007/7/17, Cornelia Huck <[EMAIL PROTECTED]>: On Mon, 16 Jul 2007 22:52:30 +0900, Akinobu Mita <[EMAIL PROTECTED]> wrote: > Current error handling in create_files() attempts to remove > all attributes passed by argument by remove_files(). But it should > only remove the attributes that have been successfully added. While this is certainly cleaner, a question out of curiousity: Does this fix any problem you saw? sysfs_hash_and_remove() used to be safe on non-existing attributes... Maybe I folded all suspicious bugfixes that I found while I spend a lot of time to pass my cpu hotplug/unplug test script. I'll test again without this patch. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/10] sysdev: add error check in sysdev_register()
2007/7/17, Cornelia Huck <[EMAIL PROTECTED]>: On Mon, 16 Jul 2007 22:51:38 +0900, Akinobu Mita <[EMAIL PROTECTED]> wrote: > This patch enables to catch the errors returned by add() procedure of > sysdev driver in sysdev_register. > @@ -253,23 +254,50 @@ int sysdev_register(struct sys_device * > > /* Notify global drivers */ > list_for_each_entry(drv, _drivers, entry) { > - if (drv->add) > - drv->add(sysdev); > + if (drv->add) { > + error = drv->add(sysdev); > + if (error) > + goto error_sysdev; > + } > + added_sysdev++; > } This looks asymmetric to me. Imagine we have a device/driver combination for which drv->add will return -ESOMEERROR. Now, it depends on the order: 1. The device is registered when the driver is already registered: registration of the device will fail. 2. The driver is registered when the device is already registered: driver registration will succeed, and the device will stay registered. I'm not sure if that is what we really want. Thanks, I didn't realize the asymmetry. I think we need to add same check to sysdev_driver_register(). But I need to check every sysdev driver's add() functions before making this change. So this patch dropped from this series of patches for now. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 10/10] intel_cacheinfo: fix cpu hotplug error handling
- Fix resource leakage in error case within detect_cache_attributes() - Introduce cache_dev_map cpumask to track whether cache interface for CPU is successfully added by cache_add_dev() or not. cache_add_dev() may fail with out of memory error. In order to avoid cache_remove_dev() with that uninitialized cache interface when CPU_DEAD event is delivered we need to have the cache_dev_map cpumask. (We cannot change cache_add_dev() from CPU_ONLINE event handler to CPU_UP_PREPARE event handler. Because cache_add_dev() needs to do cpuid and store the results with its CPU online.) Cc: Ashok Raj <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- arch/i386/kernel/cpu/intel_cacheinfo.c | 57 +++-- 1 file changed, 40 insertions(+), 17 deletions(-) Index: 2.6-mm/arch/i386/kernel/cpu/intel_cacheinfo.c === --- 2.6-mm.orig/arch/i386/kernel/cpu/intel_cacheinfo.c +++ 2.6-mm/arch/i386/kernel/cpu/intel_cacheinfo.c @@ -502,6 +502,11 @@ static void __init cache_remove_shared_c static void free_cache_attributes(unsigned int cpu) { + int i; + + for (i = 0; i < num_cache_leaves; i++) + cache_remove_shared_cpu_map(cpu, i); + kfree(cpuid4_info[cpu]); cpuid4_info[cpu] = NULL; } @@ -509,8 +514,8 @@ static void free_cache_attributes(unsign static int __cpuinit detect_cache_attributes(unsigned int cpu) { struct _cpuid4_info *this_leaf; - unsigned long j; - int retval; + unsigned long j; + int retval; cpumask_t oldmask; if (num_cache_leaves == 0) @@ -527,19 +532,26 @@ static int __cpuinit detect_cache_attrib goto out; /* Do cpuid and store the results */ - retval = 0; for (j = 0; j < num_cache_leaves; j++) { this_leaf = CPUID4_INFO_IDX(cpu, j); retval = cpuid4_cache_lookup(j, this_leaf); - if (unlikely(retval < 0)) + if (unlikely(retval < 0)) { + int i; + + for (i = 0; i < j; i++) + cache_remove_shared_cpu_map(cpu, i); break; + } cache_shared_cpu_map_setup(cpu, j); } set_cpus_allowed(current, oldmask); out: - if (retval) - free_cache_attributes(cpu); + if (retval) { + kfree(cpuid4_info[cpu]); + cpuid4_info[cpu] = NULL; + } + return retval; } @@ -683,13 +695,14 @@ static void cpuid4_cache_sysfs_exit(unsi static int __cpuinit cpuid4_cache_sysfs_init(unsigned int cpu) { + int err; if (num_cache_leaves == 0) return -ENOENT; - detect_cache_attributes(cpu); - if (cpuid4_info[cpu] == NULL) - return -ENOENT; + err = detect_cache_attributes(cpu); + if (err) + return err; /* Allocate all required memory */ cache_kobject[cpu] = kzalloc(sizeof(struct kobject), GFP_KERNEL); @@ -708,13 +721,15 @@ err_out: return -ENOMEM; } +static cpumask_t cache_dev_map = CPU_MASK_NONE; + /* Add/Remove cache interface for CPU device */ static int __cpuinit cache_add_dev(struct sys_device * sys_dev) { unsigned int cpu = sys_dev->id; unsigned long i, j; struct _index_kobject *this_object; - int retval = 0; + int retval; retval = cpuid4_cache_sysfs_init(cpu); if (unlikely(retval < 0)) @@ -724,6 +739,10 @@ static int __cpuinit cache_add_dev(struc kobject_set_name(cache_kobject[cpu], "%s", "cache"); cache_kobject[cpu]->ktype = _percpu_entry; retval = kobject_register(cache_kobject[cpu]); + if (retval < 0) { + cpuid4_cache_sysfs_exit(cpu); + return retval; + } for (i = 0; i < num_cache_leaves; i++) { this_object = INDEX_KOBJECT_PTR(cpu,i); @@ -743,6 +762,9 @@ static int __cpuinit cache_add_dev(struc break; } } + if (!retval) + cpu_set(cpu, cache_dev_map); + return retval; } @@ -751,13 +773,14 @@ static void __cpuinit cache_remove_dev(s unsigned int cpu = sys_dev->id; unsigned long i; - for (i = 0; i < num_cache_leaves; i++) { - cache_remove_shared_cpu_map(cpu, i); + if (!cpu_isset(cpu, cache_dev_map)) + return; + cpu_clear(cpu, cache_dev_map); + + for (i = 0; i < num_cache_leaves; i++) kobject_unregister(&(INDEX_KOBJECT_PTR(cpu,i)->kobj)); - } kobject_unregister(cache_kobject[cpu]); cpuid4_cache_sysfs_exit(cpu); - return
[PATCH 9/10] mce: fix cpu hotplug error handling
- Fix error handling in mce_create_device() Error handling should not do sysdev_remove_file() with not yet added attributes. - Change mce_create_device() from CPU_ONLINE event handler to CPU_UP_PREPARE event handler. Cc: Andi Kleen <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- arch/x86_64/kernel/mce.c | 35 --- 1 file changed, 24 insertions(+), 11 deletions(-) Index: 2.6-mm/arch/x86_64/kernel/mce.c === --- 2.6-mm.orig/arch/x86_64/kernel/mce.c +++ 2.6-mm/arch/x86_64/kernel/mce.c @@ -803,19 +803,30 @@ static __cpuinit int mce_create_device(u { int err; int i; - if (!mce_available(_data[cpu])) - return -EIO; + memset(_cpu(device_mce, cpu).kobj, 0, sizeof(struct kobject)); per_cpu(device_mce,cpu).id = cpu; per_cpu(device_mce,cpu).cls = _sysclass; err = sysdev_register(_cpu(device_mce,cpu)); + if (err) + return err; + + for (i = 0; mce_attributes[i]; i++) { + err = sysdev_create_file(_cpu(device_mce,cpu), +mce_attributes[i]); + if (err) + goto error; + } - if (!err) { - for (i = 0; mce_attributes[i]; i++) - sysdev_create_file(_cpu(device_mce,cpu), - mce_attributes[i]); + return 0; +error: + while (i--) { + sysdev_remove_file(_cpu(device_mce,cpu), + mce_attributes[i]); } + sysdev_unregister(_cpu(device_mce,cpu)); + return err; } @@ -827,7 +838,6 @@ static void mce_remove_device(unsigned i sysdev_remove_file(_cpu(device_mce,cpu), mce_attributes[i]); sysdev_unregister(_cpu(device_mce,cpu)); - memset(_cpu(device_mce, cpu).kobj, 0, sizeof(struct kobject)); } /* Get notified when a cpu comes on/off. Be hotplug friendly. */ @@ -835,18 +845,21 @@ static int mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; + int err = 0; switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: - mce_create_device(cpu); + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: + err = mce_create_device(cpu); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: mce_remove_device(cpu); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block mce_cpu_notifier = { - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 8/10] cpuid: fix cpu hotplug error handling
Make cpuid_device_create() CPU_UP_PREPARE event handler instead of CPU_ONLINE event handler. Cc: "H. Peter Anvin" <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- arch/i386/kernel/cpuid.c | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) Index: 2.6-mm/arch/i386/kernel/cpuid.c === --- 2.6-mm.orig/arch/i386/kernel/cpuid.c +++ 2.6-mm/arch/i386/kernel/cpuid.c @@ -152,32 +152,38 @@ static const struct file_operations cpui .open = cpuid_open, }; -static int cpuid_device_create(int i) +static int cpuid_device_create(int cpu) { - int err = 0; struct device *dev; - dev = device_create(cpuid_class, NULL, MKDEV(CPUID_MAJOR, i), "cpu%d",i); - if (IS_ERR(dev)) - err = PTR_ERR(dev); - return err; + dev = device_create(cpuid_class, NULL, MKDEV(CPUID_MAJOR, cpu), + "cpu%d", cpu); + return IS_ERR(dev) ? PTR_ERR(dev) : 0; +} + +static void cpuid_device_destroy(int cpu) +{ + device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, cpu)); } static int cpuid_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; + int err = 0; switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: - cpuid_device_create(cpu); + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: + err = cpuid_device_create(cpu); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: - device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, cpu)); + cpuid_device_destroy(cpu); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block __cpuinitdata cpuid_class_cpu_notifier = @@ -214,7 +220,7 @@ static int __init cpuid_init(void) out_class: i = 0; for_each_online_cpu(i) { - device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, i)); + cpuid_device_destroy(i); } class_destroy(cpuid_class); out_chrdev: @@ -228,7 +234,7 @@ static void __exit cpuid_exit(void) int cpu = 0; for_each_online_cpu(cpu) - device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, cpu)); + cpuid_device_destroy(cpu); class_destroy(cpuid_class); unregister_chrdev(CPUID_MAJOR, "cpu/cpuid"); unregister_hotcpu_notifier(_class_cpu_notifier); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 6/10] thermal_throttle: fix cpu hotplug error handling
Make thermal_throttle_add_dev() CPU_UP_PREPARE event handler instead of CPU_ONLINE event handler. Cc: Dmitriy Zavin <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- arch/i386/kernel/cpu/mcheck/therm_throt.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) Index: 2.6-mm/arch/i386/kernel/cpu/mcheck/therm_throt.c === --- 2.6-mm.orig/arch/i386/kernel/cpu/mcheck/therm_throt.c +++ 2.6-mm/arch/i386/kernel/cpu/mcheck/therm_throt.c @@ -131,23 +131,24 @@ static __cpuinit int thermal_throttle_cp { unsigned int cpu = (unsigned long)hcpu; struct sys_device *sys_dev; - int err; + int err = 0; sys_dev = get_cpu_sysdev(cpu); mutex_lock(_cpu_lock); switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: err = thermal_throttle_add_dev(sys_dev); - WARN_ON(err); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: thermal_throttle_remove_dev(sys_dev); break; } mutex_unlock(_cpu_lock); - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block thermal_throttle_cpu_notifier = - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/10] msr: fix cpu hotplug error handling
Make msr_device_create() CPU_UP_PREPARE event handler instead of CPU_ONLINE handler. Cc: "H. Peter Anvin" <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- arch/i386/kernel/msr.c | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) Index: 2.6-mm/arch/i386/kernel/msr.c === --- 2.6-mm.orig/arch/i386/kernel/msr.c +++ 2.6-mm/arch/i386/kernel/msr.c @@ -135,33 +135,39 @@ static const struct file_operations msr_ .open = msr_open, }; -static int msr_device_create(int i) +static int msr_device_create(int cpu) { - int err = 0; struct device *dev; - dev = device_create(msr_class, NULL, MKDEV(MSR_MAJOR, i), "msr%d",i); - if (IS_ERR(dev)) - err = PTR_ERR(dev); - return err; + dev = device_create(msr_class, NULL, MKDEV(MSR_MAJOR, cpu), + "msr%d", cpu); + return IS_ERR(dev) ? PTR_ERR(dev) : 0; +} + +static void msr_device_destroy(int cpu) +{ + device_destroy(msr_class, MKDEV(MSR_MAJOR, cpu)); } static int msr_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; + int err = 0; switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: - msr_device_create(cpu); + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: + err = msr_device_create(cpu); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: - device_destroy(msr_class, MKDEV(MSR_MAJOR, cpu)); + msr_device_destroy(cpu); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block __cpuinitdata msr_class_cpu_notifier = @@ -198,7 +204,7 @@ static int __init msr_init(void) out_class: i = 0; for_each_online_cpu(i) - device_destroy(msr_class, MKDEV(MSR_MAJOR, i)); + msr_device_destroy(i); class_destroy(msr_class); out_chrdev: unregister_chrdev(MSR_MAJOR, "cpu/msr"); @@ -210,7 +216,7 @@ static void __exit msr_exit(void) { int cpu = 0; for_each_online_cpu(cpu) - device_destroy(msr_class, MKDEV(MSR_MAJOR, cpu)); + msr_device_destroy(cpu); class_destroy(msr_class); unregister_chrdev(MSR_MAJOR, "cpu/msr"); unregister_hotcpu_notifier(_class_cpu_notifier); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/10] topology: remove topology_dev_map
By previous cpu hotplug notifier change, we don't need to track topology_dev existence for each cpu by topology_dev_map. Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- drivers/base/topology.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) Index: 2.6-mm/drivers/base/topology.c === --- 2.6-mm.orig/drivers/base/topology.c +++ 2.6-mm/drivers/base/topology.c @@ -94,27 +94,18 @@ static struct attribute_group topology_a .name = "topology" }; -static cpumask_t topology_dev_map = CPU_MASK_NONE; - /* Add/Remove cpu_topology interface for CPU device */ static int __cpuinit topology_add_dev(unsigned int cpu) { - int rc; struct sys_device *sys_dev = get_cpu_sysdev(cpu); - rc = sysfs_create_group(_dev->kobj, _attr_group); - if (!rc) - cpu_set(cpu, topology_dev_map); - return rc; + return sysfs_create_group(_dev->kobj, _attr_group); } static void __cpuinit topology_remove_dev(unsigned int cpu) { struct sys_device *sys_dev = get_cpu_sysdev(cpu); - if (!cpu_isset(cpu, topology_dev_map)) - return; - cpu_clear(cpu, topology_dev_map); sysfs_remove_group(_dev->kobj, _attr_group); } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/10] cpu: deliver CPU_UP_CANCELED only to NOTIFY_OKed callbacks with CPU_UP_PREPARE
The functions in a CPU notifier chain is called with CPU_UP_PREPARE event before making the CPU online. If one of the callback returns NOTIFY_BAD, it stops to deliver CPU_UP_PREPARE event, and CPU online operation is canceled. Then CPU_UP_CANCELED event is delivered to the functions in a CPU notifier chain again. This CPU_UP_CANCELED event is delivered to the functions which have been called with CPU_UP_PREPARE, not delivered to the functions which haven't been called with CPU_UP_PREPARE. The problem that makes existing cpu hotplug error handlings complex is that the CPU_UP_CANCELED event is delivered to the function that has returned NOTIFY_BAD, too. Usually we don't expect to call destructor function against the object that has failed to initialize. It is like: err = register_something(); if (err) { unregister_something(); // bug return err; } So it is natural to deliver CPU_UP_CANCELED event only to the functions that have returned NOTIFY_OK with CPU_UP_PREPARE event and not to call the function that have returned NOTIFY_BAD. This is what this patch is doing. Otherwise, every cpu hotplug notifiler has to track whether notifiler event is failed or not for each cpu. (drivers/base/topology.c is doing this with topology_dev_map) Similary this patch makes same thing with CPU_DOWN_PREPARE and CPU_DOWN_FAILED evnets. Cc: Rusty Russell <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- kernel/cpu.c |2 ++ 1 file changed, 2 insertions(+) Index: 2.6-mm/kernel/cpu.c === --- 2.6-mm.orig/kernel/cpu.c +++ 2.6-mm/kernel/cpu.c @@ -138,6 +138,7 @@ static int _cpu_down(unsigned int cpu, i err = __raw_notifier_call_chain(_chain, CPU_DOWN_PREPARE | mod, hcpu, -1, _calls); if (err == NOTIFY_BAD) { + nr_calls--; __raw_notifier_call_chain(_chain, CPU_DOWN_FAILED | mod, hcpu, nr_calls, NULL); printk("%s: attempt to take down CPU %u failed\n", @@ -221,6 +222,7 @@ static int __cpuinit _cpu_up(unsigned in ret = __raw_notifier_call_chain(_chain, CPU_UP_PREPARE | mod, hcpu, -1, _calls); if (ret == NOTIFY_BAD) { + nr_calls--; printk("%s: attempt to bring up CPU %u failed\n", __FUNCTION__, cpu); ret = -EINVAL; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/10] sysfs: fix error handling in create_files()
Current error handling in create_files() attempts to remove all attributes passed by argument by remove_files(). But it should only remove the attributes that have been successfully added. Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- fs/sysfs/group.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) Index: 2.6-mm/fs/sysfs/group.c === --- 2.6-mm.orig/fs/sysfs/group.c +++ 2.6-mm/fs/sysfs/group.c @@ -30,13 +30,19 @@ static void remove_files(struct sysfs_di static int create_files(struct sysfs_dirent *dir_sd, const struct attribute_group *grp) { - struct attribute *const* attr; - int error = 0; + int i; + int error; + + for (i = 0; grp->attrs[i]; i++) { + error = sysfs_add_file(dir_sd, grp->attrs[i], SYSFS_KOBJ_ATTR); + if (error) + goto error; + } + return 0; +error: + while (i--) + sysfs_hash_and_remove(dir_sd, grp->attrs[i]->name); - for (attr = grp->attrs; *attr && !error; attr++) - error = sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR); - if (error) - remove_files(dir_sd, grp); return error; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/10] sysdev: add error check in sysdev_register()
This patch enables to catch the errors returned by add() procedure of sysdev driver in sysdev_register. Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- drivers/base/sys.c | 48 ++-- 1 file changed, 38 insertions(+), 10 deletions(-) Index: 2.6-mm/drivers/base/sys.c === --- 2.6-mm.orig/drivers/base/sys.c +++ 2.6-mm/drivers/base/sys.c @@ -220,10 +220,13 @@ EXPORT_SYMBOL_GPL(sysdev_driver_unregist * @sysdev:device in question * */ -int sysdev_register(struct sys_device * sysdev) +int sysdev_register(struct sys_device *sysdev) { int error; - struct sysdev_class * cls = sysdev->cls; + struct sysdev_class *cls = sysdev->cls; + struct sysdev_driver *drv; + int added_sysdev = 0; + int added_cls = 0; if (!cls) return -EINVAL; @@ -244,8 +247,6 @@ int sysdev_register(struct sys_device * error = kobject_register(>kobj); if (!error) { - struct sysdev_driver * drv; - mutex_lock(_drivers_lock); /* Generic notification is implicit, because it's that * code that should have called us. @@ -253,23 +254,50 @@ int sysdev_register(struct sys_device * /* Notify global drivers */ list_for_each_entry(drv, _drivers, entry) { - if (drv->add) - drv->add(sysdev); + if (drv->add) { + error = drv->add(sysdev); + if (error) + goto error_sysdev; + } + added_sysdev++; } /* Notify class auxillary drivers */ list_for_each_entry(drv, >drivers, entry) { - if (drv->add) - drv->add(sysdev); + if (drv->add) { + error = drv->add(sysdev); + if (error) + goto error_cls; + } + added_cls++; } mutex_unlock(_drivers_lock); } return error; + +error_cls: + list_for_each_entry(drv, >drivers, entry) { + if (added_sysdev--) + break; + if (drv->add && drv->remove) + drv->remove(sysdev); + } +error_sysdev: + list_for_each_entry(drv, _drivers, entry) { + if (added_cls--) + break; + if (drv->add && drv->remove) + drv->remove(sysdev); + } + mutex_unlock(_drivers_lock); + kobject_unregister(>kobj); + + return error; } -void sysdev_unregister(struct sys_device * sysdev) +void sysdev_unregister(struct sys_device *sysdev) { - struct sysdev_driver * drv; + struct sysdev_driver *drv; mutex_lock(_drivers_lock); list_for_each_entry(drv, _drivers, entry) { - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/10] sysfs: fix kmem_cache_free(NULL)
This patch fixes out of memory error handling in sysfs_new_dirent(). kmem_cache_free() with NULL is not allowed. Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]> Signed-off-by: Akinobu Mita <[EMAIL PROTECTED]> --- fs/sysfs/dir.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) Index: 2.6-mm/fs/sysfs/dir.c === --- 2.6-mm.orig/fs/sysfs/dir.c +++ 2.6-mm/fs/sysfs/dir.c @@ -361,20 +361,20 @@ static struct dentry_operations sysfs_de struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type) { char *dup_name = NULL; - struct sysfs_dirent *sd = NULL; + struct sysfs_dirent *sd; if (type & SYSFS_COPY_NAME) { name = dup_name = kstrdup(name, GFP_KERNEL); if (!name) - goto err_out; + return NULL; } sd = kmem_cache_zalloc(sysfs_dir_cachep, GFP_KERNEL); if (!sd) - goto err_out; + goto err_out1; if (sysfs_alloc_ino(>s_ino)) - goto err_out; + goto err_out2; atomic_set(>s_count, 1); atomic_set(>s_active, 0); @@ -386,9 +386,10 @@ struct sysfs_dirent *sysfs_new_dirent(co return sd; - err_out: - kfree(dup_name); + err_out2: kmem_cache_free(sysfs_dir_cachep, sd); + err_out1: + kfree(dup_name); return NULL; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/10] CPU hotplug error handling fixes
This series of patches fixes the error handling for cpu hotplug. The problem is revealed by CPU hotplug/unplug test with fault-injection. The patch 1-3 are sysfs or driver core related error handling fixes. These are not directly related to cpu hotplug. But these are needed to pass the stress test. The patch 4 changes the behavior when one of the callbacks in notifier chain returns NOTIFY_BAD with CPU_UP_PREPARE event. This change makes cpu hotplug error handling simple. The patch 5 simplifies the cpu hotplug event handling in topology.c by the patch 4. The patch 6-10 are error handling fixes in cpu hotplug event callbacks. These fixes also depend on the change by the patch 4. Here is the test script I have confirmed with these patches. I guess we still have the similar bugs that I could not test due to no hardware. So it may be worth someone trying this script. --[ cut here ]-- #!/bin/bash FAILTYPE=failslab CPU=1 CPU_ONLINE=/sys/devices/system/cpu/cpu${CPU}/online faulty_system() { bash -c "echo 1 > /proc/self/make-it-fail && exec $*" } [ "$UID" == 0 ] || exit 1 [ -n "$FAILTYPE" -a -f /debug/$FAILTYPE/probability ] || exit 1 [ -f $CPU_ONLINE ] || exit 1 echo N > /debug/$FAILTYPE/ignore-gfp-wait echo Y > /debug/$FAILTYPE/task-filter echo 1 > /debug/$FAILTYPE/probability echo -1 > /debug/$FAILTYPE/times while true do faulty_system "echo 0 > $CPU_ONLINE" faulty_system "echo 1 > $CPU_ONLINE" done - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/10] CPU hotplug error handling fixes
This series of patches fixes the error handling for cpu hotplug. The problem is revealed by CPU hotplug/unplug test with fault-injection. The patch 1-3 are sysfs or driver core related error handling fixes. These are not directly related to cpu hotplug. But these are needed to pass the stress test. The patch 4 changes the behavior when one of the callbacks in notifier chain returns NOTIFY_BAD with CPU_UP_PREPARE event. This change makes cpu hotplug error handling simple. The patch 5 simplifies the cpu hotplug event handling in topology.c by the patch 4. The patch 6-10 are error handling fixes in cpu hotplug event callbacks. These fixes also depend on the change by the patch 4. Here is the test script I have confirmed with these patches. I guess we still have the similar bugs that I could not test due to no hardware. So it may be worth someone trying this script. --[ cut here ]-- #!/bin/bash FAILTYPE=failslab CPU=1 CPU_ONLINE=/sys/devices/system/cpu/cpu${CPU}/online faulty_system() { bash -c echo 1 /proc/self/make-it-fail exec $* } [ $UID == 0 ] || exit 1 [ -n $FAILTYPE -a -f /debug/$FAILTYPE/probability ] || exit 1 [ -f $CPU_ONLINE ] || exit 1 echo N /debug/$FAILTYPE/ignore-gfp-wait echo Y /debug/$FAILTYPE/task-filter echo 1 /debug/$FAILTYPE/probability echo -1 /debug/$FAILTYPE/times while true do faulty_system echo 0 $CPU_ONLINE faulty_system echo 1 $CPU_ONLINE done - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/10] sysfs: fix kmem_cache_free(NULL)
This patch fixes out of memory error handling in sysfs_new_dirent(). kmem_cache_free() with NULL is not allowed. Cc: Greg Kroah-Hartman [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- fs/sysfs/dir.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) Index: 2.6-mm/fs/sysfs/dir.c === --- 2.6-mm.orig/fs/sysfs/dir.c +++ 2.6-mm/fs/sysfs/dir.c @@ -361,20 +361,20 @@ static struct dentry_operations sysfs_de struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type) { char *dup_name = NULL; - struct sysfs_dirent *sd = NULL; + struct sysfs_dirent *sd; if (type SYSFS_COPY_NAME) { name = dup_name = kstrdup(name, GFP_KERNEL); if (!name) - goto err_out; + return NULL; } sd = kmem_cache_zalloc(sysfs_dir_cachep, GFP_KERNEL); if (!sd) - goto err_out; + goto err_out1; if (sysfs_alloc_ino(sd-s_ino)) - goto err_out; + goto err_out2; atomic_set(sd-s_count, 1); atomic_set(sd-s_active, 0); @@ -386,9 +386,10 @@ struct sysfs_dirent *sysfs_new_dirent(co return sd; - err_out: - kfree(dup_name); + err_out2: kmem_cache_free(sysfs_dir_cachep, sd); + err_out1: + kfree(dup_name); return NULL; } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/10] sysdev: add error check in sysdev_register()
This patch enables to catch the errors returned by add() procedure of sysdev driver in sysdev_register. Cc: Greg Kroah-Hartman [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- drivers/base/sys.c | 48 ++-- 1 file changed, 38 insertions(+), 10 deletions(-) Index: 2.6-mm/drivers/base/sys.c === --- 2.6-mm.orig/drivers/base/sys.c +++ 2.6-mm/drivers/base/sys.c @@ -220,10 +220,13 @@ EXPORT_SYMBOL_GPL(sysdev_driver_unregist * @sysdev:device in question * */ -int sysdev_register(struct sys_device * sysdev) +int sysdev_register(struct sys_device *sysdev) { int error; - struct sysdev_class * cls = sysdev-cls; + struct sysdev_class *cls = sysdev-cls; + struct sysdev_driver *drv; + int added_sysdev = 0; + int added_cls = 0; if (!cls) return -EINVAL; @@ -244,8 +247,6 @@ int sysdev_register(struct sys_device * error = kobject_register(sysdev-kobj); if (!error) { - struct sysdev_driver * drv; - mutex_lock(sysdev_drivers_lock); /* Generic notification is implicit, because it's that * code that should have called us. @@ -253,23 +254,50 @@ int sysdev_register(struct sys_device * /* Notify global drivers */ list_for_each_entry(drv, sysdev_drivers, entry) { - if (drv-add) - drv-add(sysdev); + if (drv-add) { + error = drv-add(sysdev); + if (error) + goto error_sysdev; + } + added_sysdev++; } /* Notify class auxillary drivers */ list_for_each_entry(drv, cls-drivers, entry) { - if (drv-add) - drv-add(sysdev); + if (drv-add) { + error = drv-add(sysdev); + if (error) + goto error_cls; + } + added_cls++; } mutex_unlock(sysdev_drivers_lock); } return error; + +error_cls: + list_for_each_entry(drv, cls-drivers, entry) { + if (added_sysdev--) + break; + if (drv-add drv-remove) + drv-remove(sysdev); + } +error_sysdev: + list_for_each_entry(drv, sysdev_drivers, entry) { + if (added_cls--) + break; + if (drv-add drv-remove) + drv-remove(sysdev); + } + mutex_unlock(sysdev_drivers_lock); + kobject_unregister(sysdev-kobj); + + return error; } -void sysdev_unregister(struct sys_device * sysdev) +void sysdev_unregister(struct sys_device *sysdev) { - struct sysdev_driver * drv; + struct sysdev_driver *drv; mutex_lock(sysdev_drivers_lock); list_for_each_entry(drv, sysdev_drivers, entry) { - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/10] sysfs: fix error handling in create_files()
Current error handling in create_files() attempts to remove all attributes passed by argument by remove_files(). But it should only remove the attributes that have been successfully added. Cc: Greg Kroah-Hartman [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- fs/sysfs/group.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) Index: 2.6-mm/fs/sysfs/group.c === --- 2.6-mm.orig/fs/sysfs/group.c +++ 2.6-mm/fs/sysfs/group.c @@ -30,13 +30,19 @@ static void remove_files(struct sysfs_di static int create_files(struct sysfs_dirent *dir_sd, const struct attribute_group *grp) { - struct attribute *const* attr; - int error = 0; + int i; + int error; + + for (i = 0; grp-attrs[i]; i++) { + error = sysfs_add_file(dir_sd, grp-attrs[i], SYSFS_KOBJ_ATTR); + if (error) + goto error; + } + return 0; +error: + while (i--) + sysfs_hash_and_remove(dir_sd, grp-attrs[i]-name); - for (attr = grp-attrs; *attr !error; attr++) - error = sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR); - if (error) - remove_files(dir_sd, grp); return error; } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/10] cpu: deliver CPU_UP_CANCELED only to NOTIFY_OKed callbacks with CPU_UP_PREPARE
The functions in a CPU notifier chain is called with CPU_UP_PREPARE event before making the CPU online. If one of the callback returns NOTIFY_BAD, it stops to deliver CPU_UP_PREPARE event, and CPU online operation is canceled. Then CPU_UP_CANCELED event is delivered to the functions in a CPU notifier chain again. This CPU_UP_CANCELED event is delivered to the functions which have been called with CPU_UP_PREPARE, not delivered to the functions which haven't been called with CPU_UP_PREPARE. The problem that makes existing cpu hotplug error handlings complex is that the CPU_UP_CANCELED event is delivered to the function that has returned NOTIFY_BAD, too. Usually we don't expect to call destructor function against the object that has failed to initialize. It is like: err = register_something(); if (err) { unregister_something(); // bug return err; } So it is natural to deliver CPU_UP_CANCELED event only to the functions that have returned NOTIFY_OK with CPU_UP_PREPARE event and not to call the function that have returned NOTIFY_BAD. This is what this patch is doing. Otherwise, every cpu hotplug notifiler has to track whether notifiler event is failed or not for each cpu. (drivers/base/topology.c is doing this with topology_dev_map) Similary this patch makes same thing with CPU_DOWN_PREPARE and CPU_DOWN_FAILED evnets. Cc: Rusty Russell [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- kernel/cpu.c |2 ++ 1 file changed, 2 insertions(+) Index: 2.6-mm/kernel/cpu.c === --- 2.6-mm.orig/kernel/cpu.c +++ 2.6-mm/kernel/cpu.c @@ -138,6 +138,7 @@ static int _cpu_down(unsigned int cpu, i err = __raw_notifier_call_chain(cpu_chain, CPU_DOWN_PREPARE | mod, hcpu, -1, nr_calls); if (err == NOTIFY_BAD) { + nr_calls--; __raw_notifier_call_chain(cpu_chain, CPU_DOWN_FAILED | mod, hcpu, nr_calls, NULL); printk(%s: attempt to take down CPU %u failed\n, @@ -221,6 +222,7 @@ static int __cpuinit _cpu_up(unsigned in ret = __raw_notifier_call_chain(cpu_chain, CPU_UP_PREPARE | mod, hcpu, -1, nr_calls); if (ret == NOTIFY_BAD) { + nr_calls--; printk(%s: attempt to bring up CPU %u failed\n, __FUNCTION__, cpu); ret = -EINVAL; - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/10] topology: remove topology_dev_map
By previous cpu hotplug notifier change, we don't need to track topology_dev existence for each cpu by topology_dev_map. Cc: Greg Kroah-Hartman [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- drivers/base/topology.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) Index: 2.6-mm/drivers/base/topology.c === --- 2.6-mm.orig/drivers/base/topology.c +++ 2.6-mm/drivers/base/topology.c @@ -94,27 +94,18 @@ static struct attribute_group topology_a .name = topology }; -static cpumask_t topology_dev_map = CPU_MASK_NONE; - /* Add/Remove cpu_topology interface for CPU device */ static int __cpuinit topology_add_dev(unsigned int cpu) { - int rc; struct sys_device *sys_dev = get_cpu_sysdev(cpu); - rc = sysfs_create_group(sys_dev-kobj, topology_attr_group); - if (!rc) - cpu_set(cpu, topology_dev_map); - return rc; + return sysfs_create_group(sys_dev-kobj, topology_attr_group); } static void __cpuinit topology_remove_dev(unsigned int cpu) { struct sys_device *sys_dev = get_cpu_sysdev(cpu); - if (!cpu_isset(cpu, topology_dev_map)) - return; - cpu_clear(cpu, topology_dev_map); sysfs_remove_group(sys_dev-kobj, topology_attr_group); } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 6/10] thermal_throttle: fix cpu hotplug error handling
Make thermal_throttle_add_dev() CPU_UP_PREPARE event handler instead of CPU_ONLINE event handler. Cc: Dmitriy Zavin [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- arch/i386/kernel/cpu/mcheck/therm_throt.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) Index: 2.6-mm/arch/i386/kernel/cpu/mcheck/therm_throt.c === --- 2.6-mm.orig/arch/i386/kernel/cpu/mcheck/therm_throt.c +++ 2.6-mm/arch/i386/kernel/cpu/mcheck/therm_throt.c @@ -131,23 +131,24 @@ static __cpuinit int thermal_throttle_cp { unsigned int cpu = (unsigned long)hcpu; struct sys_device *sys_dev; - int err; + int err = 0; sys_dev = get_cpu_sysdev(cpu); mutex_lock(therm_cpu_lock); switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: err = thermal_throttle_add_dev(sys_dev); - WARN_ON(err); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: thermal_throttle_remove_dev(sys_dev); break; } mutex_unlock(therm_cpu_lock); - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block thermal_throttle_cpu_notifier = - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/10] msr: fix cpu hotplug error handling
Make msr_device_create() CPU_UP_PREPARE event handler instead of CPU_ONLINE handler. Cc: H. Peter Anvin [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- arch/i386/kernel/msr.c | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) Index: 2.6-mm/arch/i386/kernel/msr.c === --- 2.6-mm.orig/arch/i386/kernel/msr.c +++ 2.6-mm/arch/i386/kernel/msr.c @@ -135,33 +135,39 @@ static const struct file_operations msr_ .open = msr_open, }; -static int msr_device_create(int i) +static int msr_device_create(int cpu) { - int err = 0; struct device *dev; - dev = device_create(msr_class, NULL, MKDEV(MSR_MAJOR, i), msr%d,i); - if (IS_ERR(dev)) - err = PTR_ERR(dev); - return err; + dev = device_create(msr_class, NULL, MKDEV(MSR_MAJOR, cpu), + msr%d, cpu); + return IS_ERR(dev) ? PTR_ERR(dev) : 0; +} + +static void msr_device_destroy(int cpu) +{ + device_destroy(msr_class, MKDEV(MSR_MAJOR, cpu)); } static int msr_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; + int err = 0; switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: - msr_device_create(cpu); + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: + err = msr_device_create(cpu); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: - device_destroy(msr_class, MKDEV(MSR_MAJOR, cpu)); + msr_device_destroy(cpu); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block __cpuinitdata msr_class_cpu_notifier = @@ -198,7 +204,7 @@ static int __init msr_init(void) out_class: i = 0; for_each_online_cpu(i) - device_destroy(msr_class, MKDEV(MSR_MAJOR, i)); + msr_device_destroy(i); class_destroy(msr_class); out_chrdev: unregister_chrdev(MSR_MAJOR, cpu/msr); @@ -210,7 +216,7 @@ static void __exit msr_exit(void) { int cpu = 0; for_each_online_cpu(cpu) - device_destroy(msr_class, MKDEV(MSR_MAJOR, cpu)); + msr_device_destroy(cpu); class_destroy(msr_class); unregister_chrdev(MSR_MAJOR, cpu/msr); unregister_hotcpu_notifier(msr_class_cpu_notifier); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 8/10] cpuid: fix cpu hotplug error handling
Make cpuid_device_create() CPU_UP_PREPARE event handler instead of CPU_ONLINE event handler. Cc: H. Peter Anvin [EMAIL PROTECTED] Signed-off-by: Akinobu Mita [EMAIL PROTECTED] --- arch/i386/kernel/cpuid.c | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) Index: 2.6-mm/arch/i386/kernel/cpuid.c === --- 2.6-mm.orig/arch/i386/kernel/cpuid.c +++ 2.6-mm/arch/i386/kernel/cpuid.c @@ -152,32 +152,38 @@ static const struct file_operations cpui .open = cpuid_open, }; -static int cpuid_device_create(int i) +static int cpuid_device_create(int cpu) { - int err = 0; struct device *dev; - dev = device_create(cpuid_class, NULL, MKDEV(CPUID_MAJOR, i), cpu%d,i); - if (IS_ERR(dev)) - err = PTR_ERR(dev); - return err; + dev = device_create(cpuid_class, NULL, MKDEV(CPUID_MAJOR, cpu), + cpu%d, cpu); + return IS_ERR(dev) ? PTR_ERR(dev) : 0; +} + +static void cpuid_device_destroy(int cpu) +{ + device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, cpu)); } static int cpuid_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; + int err = 0; switch (action) { - case CPU_ONLINE: - case CPU_ONLINE_FROZEN: - cpuid_device_create(cpu); + case CPU_UP_PREPARE: + case CPU_UP_PREPARE_FROZEN: + err = cpuid_device_create(cpu); break; + case CPU_UP_CANCELED: + case CPU_UP_CANCELED_FROZEN: case CPU_DEAD: case CPU_DEAD_FROZEN: - device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, cpu)); + cpuid_device_destroy(cpu); break; } - return NOTIFY_OK; + return err ? NOTIFY_BAD : NOTIFY_OK; } static struct notifier_block __cpuinitdata cpuid_class_cpu_notifier = @@ -214,7 +220,7 @@ static int __init cpuid_init(void) out_class: i = 0; for_each_online_cpu(i) { - device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, i)); + cpuid_device_destroy(i); } class_destroy(cpuid_class); out_chrdev: @@ -228,7 +234,7 @@ static void __exit cpuid_exit(void) int cpu = 0; for_each_online_cpu(cpu) - device_destroy(cpuid_class, MKDEV(CPUID_MAJOR, cpu)); + cpuid_device_destroy(cpu); class_destroy(cpuid_class); unregister_chrdev(CPUID_MAJOR, cpu/cpuid); unregister_hotcpu_notifier(cpuid_class_cpu_notifier); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/