Both the receiving thread and the bs worker threads access the hash of lists at it_nexus->cmd_hash_list. It may be the case that this is causing list corruption. Lock each individual list in the hash with a mutex. Using a recursive mutex, since abort_task_set both needs to hold the mutex, and __cmd_done (called from abort_cmd) also is calling cmd_hlist_remove, which also takes the mutex. Signed-off-by: Andy Grover <agrover at redhat.com> --- Hi Kiefer, does this make a difference? Thanks -- Andy usr/scsi_cmnd.h | 3 +++ usr/target.c | 46 ++++++++++++++++++++++++++++++++++++++-------- usr/target.h | 7 ++++++- 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/usr/scsi_cmnd.h b/usr/scsi_cmnd.h index c764c4e..cd9272e 100644 --- a/usr/scsi_cmnd.h +++ b/usr/scsi_cmnd.h @@ -49,6 +49,9 @@ struct scsi_cmd { struct it_nexus *it_nexus; struct it_nexus_lu_info *itn_lu_info; + + /* backptr to what cmd_hash_entry we're on so we can lock properly */ + struct cmd_hash_entry *cur_hash_entry; }; #define scsi_cmnd_accessor(field, type) \ diff --git a/usr/target.c b/usr/target.c index 42443d0..ac26a99 100644 --- a/usr/target.c +++ b/usr/target.c @@ -29,6 +29,7 @@ #include <unistd.h> #include <sys/socket.h> #include <sys/time.h> +#include <pthread.h> #include "list.h" #include "util.h" @@ -269,6 +270,7 @@ int it_nexus_create(int tid, uint64_t itn_id, int host_no, char *info) struct scsi_lu *lu; struct it_nexus_lu_info *itn_lu; struct timeval tv; + pthread_mutexattr_t mutex_attr; dprintf("%d %" PRIu64 " %d\n", tid, itn_id, host_no); /* for reserve/release code */ @@ -308,8 +310,15 @@ int it_nexus_create(int tid, uint64_t itn_id, int host_no, char *info) &itn->it_nexus_lu_info_list); } - for (i = 0; i < ARRAY_SIZE(itn->cmd_hash_list); i++) - INIT_LIST_HEAD(&itn->cmd_hash_list[i]); + pthread_mutexattr_init(&mutex_attr); + pthread_mutexattr_settype(&mutex_attr, PTHREAD_MUTEX_RECURSIVE); + + for (i = 0; i < ARRAY_SIZE(itn->cmd_hash_list); i++) { + INIT_LIST_HEAD(&itn->cmd_hash_list[i].head); + pthread_mutex_init(&itn->cmd_hash_list[i].lock, &mutex_attr); + } + + pthread_mutexattr_destroy(&mutex_attr); list_add_tail(&itn->nexus_siblings, &target->it_nexus_list); @@ -324,6 +333,7 @@ int it_nexus_destroy(int tid, uint64_t itn_id) int i; struct it_nexus *itn; struct scsi_lu *lu; + struct cmd_hash_entry *entry; dprintf("%d %" PRIu64 "\n", tid, itn_id); @@ -331,9 +341,18 @@ int it_nexus_destroy(int tid, uint64_t itn_id) if (!itn) return -ENOENT; - for (i = 0; i < ARRAY_SIZE(itn->cmd_hash_list); i++) - if (!list_empty(&itn->cmd_hash_list[i])) + for (i = 0; i < ARRAY_SIZE(itn->cmd_hash_list); i++) { + entry = &itn->cmd_hash_list[i]; + pthread_mutex_lock(&entry->lock); + if (!list_empty(&entry->head)) { + pthread_mutex_unlock(&entry->lock); return -EBUSY; + } + pthread_mutex_unlock(&entry->lock); + } + + for (i = 0; i < ARRAY_SIZE(itn->cmd_hash_list); i++) + pthread_mutex_destroy(&itn->cmd_hash_list[i].lock); list_for_each_entry(lu, &itn->nexus_target->device_list, device_siblings) device_release(tid, itn_id, lu->lun, 0); @@ -357,13 +376,22 @@ static struct scsi_lu *device_lookup(struct target *target, uint64_t lun) static void cmd_hlist_insert(struct it_nexus *itn, struct scsi_cmd *cmd) { - struct list_head *list = &itn->cmd_hash_list[hashfn(cmd->tag)]; - list_add(&cmd->c_hlist, list); + struct cmd_hash_entry *entry = &itn->cmd_hash_list[hashfn(cmd->tag)]; + + pthread_mutex_lock(&entry->lock); + cmd->cur_hash_entry = entry; + list_add(&cmd->c_hlist, &entry->head); + pthread_mutex_unlock(&entry->lock); } static void cmd_hlist_remove(struct scsi_cmd *cmd) { + struct cmd_hash_entry *entry = cmd->cur_hash_entry; + + pthread_mutex_lock(&entry->lock); + cmd->cur_hash_entry = NULL; list_del(&cmd->c_hlist); + pthread_mutex_unlock(&entry->lock); } static void tgt_cmd_queue_init(struct tgt_cmd_queue *q) @@ -1116,8 +1144,9 @@ static int abort_task_set(struct mgmt_req *mreq, struct target* target, list_for_each_entry(itn, &target->it_nexus_list, nexus_siblings) { for (i = 0; i < ARRAY_SIZE(itn->cmd_hash_list); i++) { - struct list_head *list = &itn->cmd_hash_list[i]; - list_for_each_entry_safe(cmd, tmp, list, c_hlist) { + struct cmd_hash_entry *entry = &itn->cmd_hash_list[i]; + pthread_mutex_lock(&entry->lock); + list_for_each_entry_safe(cmd, tmp, &entry->head, c_hlist) { if ((all && itn->itn_id == itn_id) || (cmd->tag == tag && itn->itn_id == itn_id) || (lun && !memcmp(cmd->lun, lun, sizeof(cmd->lun)))) { @@ -1127,6 +1156,7 @@ static int abort_task_set(struct mgmt_req *mreq, struct target* target, count++; } } + pthread_mutex_unlock(&entry->lock); } } return count; diff --git a/usr/target.h b/usr/target.h index f1e51f3..23e149d 100644 --- a/usr/target.h +++ b/usr/target.h @@ -48,11 +48,16 @@ struct target { struct tgt_account account; }; +struct cmd_hash_entry { + struct list_head head; + pthread_mutex_t lock; +}; + struct it_nexus { uint64_t itn_id; long ctime; - struct list_head cmd_hash_list[1 << HASH_ORDER]; + struct cmd_hash_entry cmd_hash_list[1 << HASH_ORDER]; struct target *nexus_target; -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe stgt" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html |