Page MenuHomeFreeBSD

cam/scsi: Use xpt_path_inq() instead of hand-rolled expansion
ClosedPublic

Authored by imp on Jul 2 2025, 3:43 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 15, 12:10 AM
Unknown Object (File)
Wed, Oct 15, 12:10 AM
Unknown Object (File)
Tue, Oct 14, 3:27 PM
Unknown Object (File)
Tue, Oct 14, 3:27 PM
Unknown Object (File)
Tue, Oct 14, 2:05 AM
Unknown Object (File)
Wed, Sep 24, 3:14 AM
Unknown Object (File)
Sat, Sep 20, 1:33 PM
Unknown Object (File)
Sep 17 2025, 7:49 AM
Subscribers

Details

Summary

Use xpt_path_inq() for all XPT_PATH_INQ requests. They all should be
CAM_PRIORITY_NONE since XPT_PATH_INQ is an unqueued command, so the
monor changes here from _NORMAL to _NONE don't matter. And the one place
we preseve the priority doesn't matter either: It's an allocated CCB,
true, but it only ever stores the CPI from XPT_PATH_INQ.

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp requested review of this revision.Jul 2 2025, 3:43 AM
jhb added a subscriber: jhb.

Maybe as a followup update the comment for xpt_path_inq() in cam_xpt.h to remove the comment about bzero may not be necessary.

This revision is now accepted and ready to land.Jul 2 2025, 1:32 PM

So what neither of us noticed, until I spotted it just before pushing the commit, is that this replaces a few instances of PRIORITY_NONE with PRIORITY_NORMAL. I wonder if that matters.

If this were a queued operation, it would matter. Since xpt_schedule() orders the queue by priority for what to do next it could matter.

However, since XPT_PATH_INQ isn't queued, xpt_action() will call xpt_default_action() always ({scsi,ata,nvme,mmc}_action all call xpt_default_action() for this). Priority isn't even looked at: there's a direct call to sim->sim_action(). All the SIMs I looked at handle XPT_PATH_INQ directly and call xpt_done() at the end. So does prio matter here? And if it doesn't matter, there's one more than can be done, but it carefully preserves the priority for reasons that seem silly now:

xpt_setup_ccb(&work_ccb->ccb_h, request_ccb->ccb_h.path,
              request_ccb->ccb_h.pinfo.priority);
work_ccb->ccb_h.func_code = XPT_PATH_INQ;
xpt_action(work_ccb);
if (work_ccb->ccb_h.status != CAM_REQ_CMP) {
        request_ccb->ccb_h.status = work_ccb->ccb_h.status;
        xpt_free_ccb(work_ccb);
        xpt_done(request_ccb);
        return;
}

so why bother? Adding ken and mav since they might know.

Bonus question: would it be more proper to set priority to CAM_PRIORITY_NONE in xpt_path_inq() since it's ignored?

priority doesn't matter

This revision now requires review to proceed.Jul 5 2025, 6:13 PM

I'm not quite sure this should be PRIOIRTY_NONE to flag that the priority isn't used. Another commit will fix xpt_path_inq() to use that.

s/monor/minor/ in commit log

This revision is now accepted and ready to land.Jul 7 2025, 4:58 PM