Page MenuHomeFreeBSD
Differential D30298 Diff 92567 sys/cam/scsi/scsi_targ_bh.c

Changeset View

Standalone View

View Options

sys/cam/scsi/scsi_targ_bh.c

Show First 20 Lines • Show All 235 Lines • ▼ Show 20 Linestargbhenlun(struct cam_periph *periph)
cam_status status; cam_status status;
int i; int i;
softc = (struct targbh_softc *)periph->softc; softc = (struct targbh_softc *)periph->softc;
if ((softc->flags & TARGBH_FLAG_LUN_ENABLED) != 0) if ((softc->flags & TARGBH_FLAG_LUN_ENABLED) != 0)
return (CAM_REQ_CMP); return (CAM_REQ_CMP);
memset(&immed_ccb, 0, sizeof(immed_ccb));
xpt_setup_ccb(&immed_ccb.ccb_h, periph->path, CAM_PRIORITY_NORMAL); xpt_setup_ccb(&immed_ccb.ccb_h, periph->path, CAM_PRIORITY_NORMAL);
immed_ccb.ccb_h.func_code = XPT_EN_LUN; immed_ccb.ccb_h.func_code = XPT_EN_LUN;
/* Don't need support for any vendor specific commands */ /* Don't need support for any vendor specific commands */
immed_ccb.cel.grp6_len = 0; immed_ccb.cel.grp6_len = 0;
immed_ccb.cel.grp7_len = 0; immed_ccb.cel.grp7_len = 0;
immed_ccb.cel.enable = 1; immed_ccb.cel.enable = 1;
xpt_action(&immed_ccb); xpt_action(&immed_ccb);
status = immed_ccb.ccb_h.status; status = immed_ccb.ccb_h.status;
if (status != CAM_REQ_CMP) { if (status != CAM_REQ_CMP) {
xpt_print(periph->path, xpt_print(periph->path,
"targbhenlun - Enable Lun Rejected with status 0x%x\n", "targbhenlun - Enable Lun Rejected with status 0x%x\n",
status); status);
return (status); return (status);
} }
softc->flags |= TARGBH_FLAG_LUN_ENABLED; softc->flags |= TARGBH_FLAG_LUN_ENABLED;
/* /*
* Build up a buffer of accept target I/O * Build up a buffer of accept target I/O
* operations for incoming selections. * operations for incoming selections.
*/ */
for (i = 0; i < MAX_ACCEPT; i++) { for (i = 0; i < MAX_ACCEPT; i++) {
struct ccb_accept_tio *atio; struct ccb_accept_tio *atio;
impUnsubmitted
Not Done
Inline Actions

we should just use this on the stack, not malloc it. we're not freeing it anyway.

imp: we should just use this on the stack, not malloc it. we're not freeing it anyway.
scottlUnsubmitted
Not Done
Inline Actions

Absolutely disagree. The usage and completion happens in a different context.

scottl: Absolutely disagree. The usage and completion happens in a different context.
atio = (struct ccb_accept_tio*)malloc(sizeof(*atio), M_SCSIBH, atio = (struct ccb_accept_tio*)malloc(sizeof(*atio), M_SCSIBH,
impUnsubmitted
Not Done
Inline Actions

This is leaked

imp: This is leaked
mavUnsubmitted
Not Done
Inline Actions

It is not leaked (at least here), but queued into HBA. It return back to be processed by targbhdone() after request arrival or if aborted. And obviously it should not be on stack for that to work.

mav: It is not leaked (at least here), but queued into HBA. It return back to be processed by…
M_NOWAIT); M_ZERO | M_NOWAIT);
if (atio == NULL) { if (atio == NULL) {
status = CAM_RESRC_UNAVAIL; status = CAM_RESRC_UNAVAIL;
break; break;
} }
atio->ccb_h.ccb_descr = targbhallocdescr(); atio->ccb_h.ccb_descr = targbhallocdescr();
if (atio->ccb_h.ccb_descr == NULL) { if (atio->ccb_h.ccb_descr == NULL) {
Show All 22 Lines if (i == 0) {
return (CAM_REQ_CMP_ERR); return (CAM_REQ_CMP_ERR);
} }
/* /*
* Build up a buffer of immediate notify CCBs * Build up a buffer of immediate notify CCBs
* so the SIM can tell us of asynchronous target mode events. * so the SIM can tell us of asynchronous target mode events.
*/ */
for (i = 0; i < MAX_ACCEPT; i++) { for (i = 0; i < MAX_ACCEPT; i++) {
struct ccb_immediate_notify *inot; struct ccb_immediate_notify *inot;
impUnsubmitted
Not Done
Inline Actions

we shouldn't malloc this, but just use it on the stack imho.

imp: we shouldn't malloc this, but just use it on the stack imho.
scottlUnsubmitted
Not Done
Inline Actions

Again, this is incorrect advice.

scottl: Again, this is incorrect advice.
inot = (struct ccb_immediate_notify*)malloc(sizeof(*inot), inot = (struct ccb_immediate_notify*)malloc(sizeof(*inot),
impUnsubmitted
Not Done
Inline Actions

this is leaked

imp: this is leaked
mavUnsubmitted
Not Done
Inline Actions

this is leaked

mav: > this is leaked
mavUnsubmitted
Not Done
Inline Actions

Same as above, not leaked but queued.

mav: Same as above, not leaked but queued.
M_SCSIBH, M_NOWAIT); M_SCSIBH, M_ZERO | M_NOWAIT);
if (inot == NULL) { if (inot == NULL) {
status = CAM_RESRC_UNAVAIL; status = CAM_RESRC_UNAVAIL;
break; break;
} }
xpt_setup_ccb(&inot->ccb_h, periph->path, CAM_PRIORITY_NORMAL); xpt_setup_ccb(&inot->ccb_h, periph->path, CAM_PRIORITY_NORMAL);
inot->ccb_h.func_code = XPT_IMMEDIATE_NOTIFY; inot->ccb_h.func_code = XPT_IMMEDIATE_NOTIFY;
inot->ccb_h.cbfcnp = targbhdone; inot->ccb_h.cbfcnp = targbhdone;
SLIST_INSERT_HEAD(&softc->immed_notify_slist, &inot->ccb_h, SLIST_INSERT_HEAD(&softc->immed_notify_slist, &inot->ccb_h,
periph_links.sle); periph_links.sle);
xpt_action((union ccb *)inot); xpt_action((union ccb *)inot);
status = inot->ccb_h.status; status = inot->ccb_h.status;
impUnsubmitted
Not Done
Inline Actions

well, if not leaked and the xpt_action(), but I don't see the couple of routines I checked do that.

imp: well, if not leaked and the xpt_action(), but I don't see the couple of routines I checked do…
mavUnsubmitted
Not Done
Inline Actions

targbhdone() or something behind should free it.

mav: targbhdone() or something behind should free it.
impUnsubmitted
Not Done
Inline Actions

Ah, yes. that is true. I'd bogusly thought these were all sync CCBs, and since they are async, my observations (leaked + put on stack) are equally bogus, here and elsewhere... they should be ignored.

imp: Ah, yes. that is true. I'd bogusly thought these were all sync CCBs, and since they are async…
if (status != CAM_REQ_INPROG) if (status != CAM_REQ_INPROG)
break; break;
} }
if (i == 0) { if (i == 0) {
xpt_print(periph->path, xpt_print(periph->path,
"targbhenlun - Could not allocate immediate notify " "targbhenlun - Could not allocate immediate notify "
"CCBs: status = 0x%x\n", status); "CCBs: status = 0x%x\n", status);
Show All 10 Linestargbhdislun(struct cam_periph *periph)
union ccb ccb; union ccb ccb;
struct targbh_softc *softc; struct targbh_softc *softc;
struct ccb_accept_tio* atio; struct ccb_accept_tio* atio;
struct ccb_hdr *ccb_h; struct ccb_hdr *ccb_h;
softc = (struct targbh_softc *)periph->softc; softc = (struct targbh_softc *)periph->softc;
if ((softc->flags & TARGBH_FLAG_LUN_ENABLED) == 0) if ((softc->flags & TARGBH_FLAG_LUN_ENABLED) == 0)
return CAM_REQ_CMP; return CAM_REQ_CMP;
memset(&ccb, 0, sizeof(ccb));
/* XXX Block for Continue I/O completion */ /* XXX Block for Continue I/O completion */
/* Kill off all ACCECPT and IMMEDIATE CCBs */ /* Kill off all ACCECPT and IMMEDIATE CCBs */
while ((atio = softc->accept_tio_list) != NULL) { while ((atio = softc->accept_tio_list) != NULL) {
softc->accept_tio_list = softc->accept_tio_list =
((struct targbh_cmd_desc*)atio->ccb_h.ccb_descr)->atio_link; ((struct targbh_cmd_desc*)atio->ccb_h.ccb_descr)->atio_link;
▲ Show 20 Lines • Show All 406 Lines • Show Last 20 Lines