d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Eliminate xpt_copy_path.

Browse Source 
It's used in exactly one place. In that place it's used so we can hold the lock
on the device associated with the path (since we do a xpt_path_lock and unlock
pair around the callback). Instead, inline taking and dropping the reference to
the device so we can ensure we can unlock the mutex after the callback finishes
if the path in the ccb that's queued to be processed by xpt_scanner_thread is
destroyed while being processed. We don't actually need the path itself for
anything other than dereferencing it to get the device to do the lock and
unlock.

This also makes the locking / use model for cam_path a little cleaner by
eliminating a case where we needlessly copy the object.

Reviewed by: chuck, chs, ken
Differential Revision:	https://reviews.freebsd.org/D24008
This commit is contained in:
Warner Losh 2020-03-10 23:59:58 +00:00
parent 9ac30e0b66
commit 3dfb13c1cd
2 changed files with 18 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
sys/cam/cam_xpt.c
View File

@ -803,7 +803,8 @@ static void
xpt_scanner_thread(void *dummy)
{
union ccb *ccb;
struct cam_path path;
struct mtx *mtx;
struct cam_ed *device;
xpt_lock_buses();
for (;;) {
@ -815,15 +816,22 @@ xpt_scanner_thread(void *dummy)
xpt_unlock_buses();
/*
* Since lock can be dropped inside and path freed
* by completion callback even before return here,
* take our own path copy for reference.
* We need to lock the device's mutex which we use as
* the path mutex. We can't do it directly because the
* cam_path in the ccb may wind up going away because
* the path lock may be dropped and the path retired in
* the completion callback. We do this directly to keep
* the reference counts in cam_path sane. We also have
* to copy the device pointer because ccb_h.path may
* be freed in the callback.
*/
xpt_copy_path(&path, ccb->ccb_h.path);
xpt_path_lock(&path);
mtx = xpt_path_mtx(ccb->ccb_h.path);
device = ccb->ccb_h.path->device;
xpt_acquire_device(device);
mtx_lock(mtx);
xpt_action(ccb);
xpt_path_unlock(&path);
xpt_release_path(&path);
mtx_unlock(mtx);
xpt_release_device(device);
xpt_lock_buses();
}
@ -3686,15 +3694,6 @@ xpt_clone_path(struct cam_path **new_path_ptr, struct cam_path *path)
new_path = (struct cam_path *)malloc(sizeof(*path), M_CAMPATH, M_NOWAIT);
if (new_path == NULL)
return(CAM_RESRC_UNAVAIL);
xpt_copy_path(new_path, path);
*new_path_ptr = new_path;
return (CAM_REQ_CMP);
}
void
xpt_copy_path(struct cam_path *new_path, struct cam_path *path)
{
*new_path = *path;
if (path->bus != NULL)
xpt_acquire_bus(path->bus);
@ -3702,6 +3701,8 @@ xpt_copy_path(struct cam_path *new_path, struct cam_path *path)
xpt_acquire_target(path->target);
if (path->device != NULL)
xpt_acquire_device(path->device);
*new_path_ptr = new_path;
return (CAM_REQ_CMP);
}
void

2
sys/cam/cam_xpt.h
View File

@ -140,8 +140,6 @@ cam_status xpt_compile_path(struct cam_path *new_path,
lun_id_t lun_id);
cam_status xpt_clone_path(struct cam_path **new_path,
struct cam_path *path);
void xpt_copy_path(struct cam_path *new_path,
struct cam_path *path);
void xpt_release_path(struct cam_path *path);