Commit 88187605 authored by Max Asbock's avatar Max Asbock Committed by Linus Torvalds

[PATCH] ibmasm driver: fix race in command refcount logic

This patch fixes a race in the command reference counting logic by putting
spinlocks around kobject_put() in the command_put function.

- Also added debug messages.

- Changed a memcpy to memcpy_fromio since we are reading from io space.
Signed-off-by: default avatarMax Asbock <masbock@us.ibm.com>
Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent 278d72ae
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
*/ */
#include "ibmasm.h" #include "ibmasm.h"
#include "lowlevel.h"
static void exec_next_command(struct service_processor *sp); static void exec_next_command(struct service_processor *sp);
static void free_command(struct kobject *kobj); static void free_command(struct kobject *kobj);
...@@ -31,8 +32,9 @@ static struct kobj_type ibmasm_cmd_kobj_type = { ...@@ -31,8 +32,9 @@ static struct kobj_type ibmasm_cmd_kobj_type = {
.release = free_command, .release = free_command,
}; };
static atomic_t command_count = ATOMIC_INIT(0);
struct command *ibmasm_new_command(size_t buffer_size) struct command *ibmasm_new_command(struct service_processor *sp, size_t buffer_size)
{ {
struct command *cmd; struct command *cmd;
...@@ -55,11 +57,15 @@ struct command *ibmasm_new_command(size_t buffer_size) ...@@ -55,11 +57,15 @@ struct command *ibmasm_new_command(size_t buffer_size)
kobject_init(&cmd->kobj); kobject_init(&cmd->kobj);
cmd->kobj.ktype = &ibmasm_cmd_kobj_type; cmd->kobj.ktype = &ibmasm_cmd_kobj_type;
cmd->lock = &sp->lock;
cmd->status = IBMASM_CMD_PENDING; cmd->status = IBMASM_CMD_PENDING;
init_waitqueue_head(&cmd->wait); init_waitqueue_head(&cmd->wait);
INIT_LIST_HEAD(&cmd->queue_node); INIT_LIST_HEAD(&cmd->queue_node);
atomic_inc(&command_count);
dbg("command count: %d\n", atomic_read(&command_count));
return cmd; return cmd;
} }
...@@ -68,6 +74,8 @@ static void free_command(struct kobject *kobj) ...@@ -68,6 +74,8 @@ static void free_command(struct kobject *kobj)
struct command *cmd = to_command(kobj); struct command *cmd = to_command(kobj);
list_del(&cmd->queue_node); list_del(&cmd->queue_node);
atomic_dec(&command_count);
dbg("command count: %d\n", atomic_read(&command_count));
kfree(cmd->buffer); kfree(cmd->buffer);
kfree(cmd); kfree(cmd);
} }
...@@ -94,8 +102,14 @@ static struct command *dequeue_command(struct service_processor *sp) ...@@ -94,8 +102,14 @@ static struct command *dequeue_command(struct service_processor *sp)
static inline void do_exec_command(struct service_processor *sp) static inline void do_exec_command(struct service_processor *sp)
{ {
char tsbuf[32];
dbg("%s:%d at %s\n", __FUNCTION__, __LINE__, get_timestamp(tsbuf));
if (ibmasm_send_i2o_message(sp)) { if (ibmasm_send_i2o_message(sp)) {
sp->current_command->status = IBMASM_CMD_FAILED; sp->current_command->status = IBMASM_CMD_FAILED;
wake_up(&sp->current_command->wait);
command_put(sp->current_command);
exec_next_command(sp); exec_next_command(sp);
} }
} }
...@@ -111,14 +125,16 @@ static inline void do_exec_command(struct service_processor *sp) ...@@ -111,14 +125,16 @@ static inline void do_exec_command(struct service_processor *sp)
void ibmasm_exec_command(struct service_processor *sp, struct command *cmd) void ibmasm_exec_command(struct service_processor *sp, struct command *cmd)
{ {
unsigned long flags; unsigned long flags;
char tsbuf[32];
dbg("%s:%d at %s\n", __FUNCTION__, __LINE__, get_timestamp(tsbuf));
spin_lock_irqsave(&sp->lock, flags); spin_lock_irqsave(&sp->lock, flags);
if (!sp->current_command) { if (!sp->current_command) {
command_get(cmd);
sp->current_command = cmd; sp->current_command = cmd;
command_get(sp->current_command);
spin_unlock_irqrestore(&sp->lock, flags); spin_unlock_irqrestore(&sp->lock, flags);
do_exec_command(sp); do_exec_command(sp);
} else { } else {
enqueue_command(sp, cmd); enqueue_command(sp, cmd);
...@@ -129,9 +145,9 @@ void ibmasm_exec_command(struct service_processor *sp, struct command *cmd) ...@@ -129,9 +145,9 @@ void ibmasm_exec_command(struct service_processor *sp, struct command *cmd)
static void exec_next_command(struct service_processor *sp) static void exec_next_command(struct service_processor *sp)
{ {
unsigned long flags; unsigned long flags;
char tsbuf[32];
wake_up(&sp->current_command->wait); dbg("%s:%d at %s\n", __FUNCTION__, __LINE__, get_timestamp(tsbuf));
command_put(sp->current_command);
spin_lock_irqsave(&sp->lock, flags); spin_lock_irqsave(&sp->lock, flags);
sp->current_command = dequeue_command(sp); sp->current_command = dequeue_command(sp);
...@@ -169,7 +185,9 @@ void ibmasm_receive_command_response(struct service_processor *sp, void *respons ...@@ -169,7 +185,9 @@ void ibmasm_receive_command_response(struct service_processor *sp, void *respons
if (!sp->current_command) if (!sp->current_command)
return; return;
memcpy(cmd->buffer, response, min(size, cmd->buffer_size)); memcpy_fromio(cmd->buffer, response, min(size, cmd->buffer_size));
cmd->status = IBMASM_CMD_COMPLETE; cmd->status = IBMASM_CMD_COMPLETE;
wake_up(&sp->current_command->wait);
command_put(sp->current_command);
exec_next_command(sp); exec_next_command(sp);
} }
...@@ -33,7 +33,13 @@ void ibmasm_receive_message(struct service_processor *sp, void *message, int mes ...@@ -33,7 +33,13 @@ void ibmasm_receive_message(struct service_processor *sp, void *message, int mes
u32 size; u32 size;
struct dot_command_header *header = (struct dot_command_header *)message; struct dot_command_header *header = (struct dot_command_header *)message;
if (message_size == 0)
return;
size = get_dot_command_size(message); size = get_dot_command_size(message);
if (size == 0)
return;
if (size > message_size) if (size > message_size)
size = message_size; size = message_size;
...@@ -67,7 +73,7 @@ int ibmasm_send_driver_vpd(struct service_processor *sp) ...@@ -67,7 +73,7 @@ int ibmasm_send_driver_vpd(struct service_processor *sp)
u8 *vpd_data; u8 *vpd_data;
int result = 0; int result = 0;
command = ibmasm_new_command(INIT_BUFFER_SIZE); command = ibmasm_new_command(sp, INIT_BUFFER_SIZE);
if (command == NULL) if (command == NULL)
return -ENOMEM; return -ENOMEM;
...@@ -121,7 +127,7 @@ int ibmasm_send_os_state(struct service_processor *sp, int os_state) ...@@ -121,7 +127,7 @@ int ibmasm_send_os_state(struct service_processor *sp, int os_state)
struct os_state_command *os_state_cmd; struct os_state_command *os_state_cmd;
int result = 0; int result = 0;
cmd = ibmasm_new_command(sizeof(struct os_state_command)); cmd = ibmasm_new_command(sp, sizeof(struct os_state_command));
if (cmd == NULL) if (cmd == NULL)
return -ENOMEM; return -ENOMEM;
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include <linux/notifier.h> #include <linux/notifier.h>
#include "ibmasm.h" #include "ibmasm.h"
#include "dot_command.h" #include "dot_command.h"
#include "lowlevel.h"
static int suspend_heartbeats = 0; static int suspend_heartbeats = 0;
...@@ -62,7 +63,7 @@ void ibmasm_unregister_panic_notifier(void) ...@@ -62,7 +63,7 @@ void ibmasm_unregister_panic_notifier(void)
int ibmasm_heartbeat_init(struct service_processor *sp) int ibmasm_heartbeat_init(struct service_processor *sp)
{ {
sp->heartbeat = ibmasm_new_command(HEARTBEAT_BUFFER_SIZE); sp->heartbeat = ibmasm_new_command(sp, HEARTBEAT_BUFFER_SIZE);
if (sp->heartbeat == NULL) if (sp->heartbeat == NULL)
return -ENOMEM; return -ENOMEM;
...@@ -71,6 +72,12 @@ int ibmasm_heartbeat_init(struct service_processor *sp) ...@@ -71,6 +72,12 @@ int ibmasm_heartbeat_init(struct service_processor *sp)
void ibmasm_heartbeat_exit(struct service_processor *sp) void ibmasm_heartbeat_exit(struct service_processor *sp)
{ {
char tsbuf[32];
dbg("%s:%d at %s\n", __FUNCTION__, __LINE__, get_timestamp(tsbuf));
ibmasm_wait_for_response(sp->heartbeat, IBMASM_CMD_TIMEOUT_NORMAL);
dbg("%s:%d at %s\n", __FUNCTION__, __LINE__, get_timestamp(tsbuf));
suspend_heartbeats = 1;
command_put(sp->heartbeat); command_put(sp->heartbeat);
} }
...@@ -78,14 +85,16 @@ void ibmasm_receive_heartbeat(struct service_processor *sp, void *message, size ...@@ -78,14 +85,16 @@ void ibmasm_receive_heartbeat(struct service_processor *sp, void *message, size
{ {
struct command *cmd = sp->heartbeat; struct command *cmd = sp->heartbeat;
struct dot_command_header *header = (struct dot_command_header *)cmd->buffer; struct dot_command_header *header = (struct dot_command_header *)cmd->buffer;
char tsbuf[32];
dbg("%s:%d at %s\n", __FUNCTION__, __LINE__, get_timestamp(tsbuf));
if (suspend_heartbeats) if (suspend_heartbeats)
return; return;
/* return the received dot command to sender */ /* return the received dot command to sender */
cmd->status = IBMASM_CMD_PENDING; cmd->status = IBMASM_CMD_PENDING;
size = min(size, cmd->buffer_size); size = min(size, cmd->buffer_size);
memcpy(cmd->buffer, message, size); memcpy_fromio(cmd->buffer, message, size);
header->type = sp_write; header->type = sp_write;
ibmasm_exec_command(sp, cmd); ibmasm_exec_command(sp, cmd);
} }
...@@ -95,12 +95,17 @@ struct command { ...@@ -95,12 +95,17 @@ struct command {
size_t buffer_size; size_t buffer_size;
int status; int status;
struct kobject kobj; struct kobject kobj;
spinlock_t *lock;
}; };
#define to_command(c) container_of(c, struct command, kobj) #define to_command(c) container_of(c, struct command, kobj)
static inline void command_put(struct command *cmd) static inline void command_put(struct command *cmd)
{ {
unsigned long flags;
spin_lock_irqsave(cmd->lock, flags);
kobject_put(&cmd->kobj); kobject_put(&cmd->kobj);
spin_unlock_irqrestore(cmd->lock, flags);
} }
static inline void command_get(struct command *cmd) static inline void command_get(struct command *cmd)
...@@ -159,7 +164,7 @@ struct service_processor { ...@@ -159,7 +164,7 @@ struct service_processor {
}; };
/* command processing */ /* command processing */
extern struct command *ibmasm_new_command(size_t buffer_size); extern struct command *ibmasm_new_command(struct service_processor *sp, size_t buffer_size);
extern void ibmasm_exec_command(struct service_processor *sp, struct command *cmd); extern void ibmasm_exec_command(struct service_processor *sp, struct command *cmd);
extern void ibmasm_wait_for_response(struct command *cmd, int timeout); extern void ibmasm_wait_for_response(struct command *cmd, int timeout);
extern void ibmasm_receive_command_response(struct service_processor *sp, void *response, size_t size); extern void ibmasm_receive_command_response(struct service_processor *sp, void *response, size_t size);
......
...@@ -321,7 +321,7 @@ static ssize_t command_file_write(struct file *file, const char __user *ubuff, s ...@@ -321,7 +321,7 @@ static ssize_t command_file_write(struct file *file, const char __user *ubuff, s
if (command_data->command) if (command_data->command)
return -EAGAIN; return -EAGAIN;
cmd = ibmasm_new_command(count); cmd = ibmasm_new_command(command_data->sp, count);
if (!cmd) if (!cmd)
return -ENOMEM; return -ENOMEM;
......
...@@ -63,7 +63,7 @@ int ibmasm_start_reverse_heartbeat(struct service_processor *sp, struct reverse_ ...@@ -63,7 +63,7 @@ int ibmasm_start_reverse_heartbeat(struct service_processor *sp, struct reverse_
int times_failed = 0; int times_failed = 0;
int result = 1; int result = 1;
cmd = ibmasm_new_command(sizeof rhb_dot_cmd); cmd = ibmasm_new_command(sp, sizeof rhb_dot_cmd);
if (!cmd) if (!cmd)
return -ENOMEM; return -ENOMEM;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment