Date: Wed, 29 Nov 2006 07:05:51 GMT From: Scott Long <scottl@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 110660 for review Message-ID: <200611290705.kAT75pt4020339@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=110660 Change 110660 by scottl@scottl-x64 on 2006/11/29 07:05:25 Calling xpt_action(XPT_SASYNC_CB) can wind up calling into other SIMs. This quickly becomes a locking nightmare, so decouple it from the caller via a taskqueue. Affected files ... .. //depot/projects/scottl-camlock/src/sys/cam/cam_ccb.h#10 edit .. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#44 edit Differences ... ==== //depot/projects/scottl-camlock/src/sys/cam/cam_ccb.h#10 (text+ko) ==== @@ -246,8 +246,7 @@ typedef union { void *ptr; u_long field; - u_int8_t bytes[sizeof(void *) > sizeof(u_long) - ? sizeof(void *) : sizeof(u_long)]; + u_int8_t bytes[sizeof(uintptr_t)]; } ccb_priv_entry; typedef union { @@ -277,6 +276,7 @@ u_int32_t flags; /* ccb_flags */ ccb_ppriv_area periph_priv; ccb_spriv_area sim_priv; + void *xptpriv; /* Holds a task object if needed */ u_int32_t timeout; /* Timeout value */ /* ==== //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#44 (text+ko) ==== @@ -42,6 +42,7 @@ #include <sys/md5.h> #include <sys/interrupt.h> #include <sys/sbuf.h> +#include <sys/taskqueue.h> #include <sys/lock.h> #include <sys/mutex.h> @@ -3006,6 +3007,86 @@ return(1); } +static void +xpt_action_sasync_cb(void *context, int pending) +{ + union ccb *start_ccb; + struct ccb_setasync *csa; + struct async_node *cur_entry; + struct async_list *async_head; + u_int32_t added; + int s; + + start_ccb = (union ccb *)context; + csa = &start_ccb->csa; + added = csa->event_enable; + async_head = &csa->ccb_h.path->device->asyncs; + + /* + * If there is already an entry for us, simply + * update it. + */ + s = splcam(); + mtx_lock(csa->ccb_h.path->bus->sim->mtx); + cur_entry = SLIST_FIRST(async_head); + while (cur_entry != NULL) { + if ((cur_entry->callback_arg == csa->callback_arg) + && (cur_entry->callback == csa->callback)) + break; + cur_entry = SLIST_NEXT(cur_entry, links); + } + + if (cur_entry != NULL) { + /* + * If the request has no flags set, + * remove the entry. + */ + added &= ~cur_entry->event_enable; + if (csa->event_enable == 0) { + SLIST_REMOVE(async_head, cur_entry, + async_node, links); + csa->ccb_h.path->device->refcount--; + free(cur_entry, M_CAMXPT); + } else { + cur_entry->event_enable = csa->event_enable; + } + } else { + cur_entry = malloc(sizeof(*cur_entry), M_CAMXPT, + M_NOWAIT); + if (cur_entry == NULL) { + splx(s); + goto out; + } + cur_entry->event_enable = csa->event_enable; + cur_entry->callback_arg = csa->callback_arg; + cur_entry->callback = csa->callback; + SLIST_INSERT_HEAD(async_head, cur_entry, links); + csa->ccb_h.path->device->refcount++; + } + mtx_unlock(csa->ccb_h.path->bus->sim->mtx); + + if ((added & AC_FOUND_DEVICE) != 0) { + /* + * Get this peripheral up to date with all + * the currently existing devices. + */ + xpt_for_all_devices(xptsetasyncfunc, cur_entry); + } + if ((added & AC_PATH_REGISTERED) != 0) { + /* + * Get this peripheral up to date with all + * the currently existing busses. + */ + xpt_for_all_busses(xptsetasyncbusfunc, cur_entry); + } + splx(s); + +out: + free(start_ccb->ccb_h.xptpriv, M_CAMXPT); + xpt_free_path(start_ccb->ccb_h.path); + xpt_free_ccb(start_ccb); +} + void xpt_action(union ccb *start_ccb) { @@ -3414,73 +3495,42 @@ } case XPT_SASYNC_CB: { - struct ccb_setasync *csa; - struct async_node *cur_entry; - struct async_list *async_head; - u_int32_t added; - int s; + union ccb *task_ccb; + struct task *task; - csa = &start_ccb->csa; - added = csa->event_enable; - async_head = &csa->ccb_h.path->device->asyncs; - /* - * If there is already an entry for us, simply - * update it. + * Need to decouple this operation via a taqskqueue so that + * the locking doesn't become a mess. Clone the ccb so that + * we own the memory and can free it later. */ - s = splcam(); - cur_entry = SLIST_FIRST(async_head); - while (cur_entry != NULL) { - if ((cur_entry->callback_arg == csa->callback_arg) - && (cur_entry->callback == csa->callback)) - break; - cur_entry = SLIST_NEXT(cur_entry, links); + task_ccb = malloc(sizeof(union ccb), M_CAMXPT, M_NOWAIT); + if (task_ccb == NULL) { + start_ccb->ccb_h.status = CAM_RESRC_UNAVAIL; + break; + } + bcopy(start_ccb, task_ccb, sizeof(union ccb)); + if (xpt_create_path(&task_ccb->ccb_h.path, NULL, + start_ccb->ccb_h.path_id, + start_ccb->ccb_h.target_id, + start_ccb->ccb_h.target_lun) != + CAM_REQ_CMP) { + start_ccb->ccb_h.status = CAM_RESRC_UNAVAIL; + xpt_free_ccb(task_ccb); + break; } - if (cur_entry != NULL) { - /* - * If the request has no flags set, - * remove the entry. - */ - added &= ~cur_entry->event_enable; - if (csa->event_enable == 0) { - SLIST_REMOVE(async_head, cur_entry, - async_node, links); - csa->ccb_h.path->device->refcount--; - free(cur_entry, M_CAMXPT); - } else { - cur_entry->event_enable = csa->event_enable; - } - } else { - cur_entry = malloc(sizeof(*cur_entry), M_CAMXPT, - M_NOWAIT); - if (cur_entry == NULL) { - splx(s); - csa->ccb_h.status = CAM_RESRC_UNAVAIL; - break; - } - cur_entry->event_enable = csa->event_enable; - cur_entry->callback_arg = csa->callback_arg; - cur_entry->callback = csa->callback; - SLIST_INSERT_HEAD(async_head, cur_entry, links); - csa->ccb_h.path->device->refcount++; + task = malloc(sizeof(struct task), M_CAMXPT, M_NOWAIT); + if (task == NULL) { + start_ccb->ccb_h.status = CAM_RESRC_UNAVAIL; + xpt_free_path(task_ccb->ccb_h.path); + xpt_free_ccb(task_ccb); + break; } - if ((added & AC_FOUND_DEVICE) != 0) { - /* - * Get this peripheral up to date with all - * the currently existing devices. - */ - xpt_for_all_devices(xptsetasyncfunc, cur_entry); - } - if ((added & AC_PATH_REGISTERED) != 0) { - /* - * Get this peripheral up to date with all - * the currently existing busses. - */ - xpt_for_all_busses(xptsetasyncbusfunc, cur_entry); - } - splx(s); + TASK_INIT(task, 0, xpt_action_sasync_cb, task_ccb); + task_ccb->ccb_h.xptpriv = task; + taskqueue_enqueue(taskqueue_thread, task); + start_ccb->ccb_h.status = CAM_REQ_CMP; break; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200611290705.kAT75pt4020339>