Date: Mon, 5 Feb 2018 23:21:29 +0100 From: Oliver Pinter <oliver.pinter@hardenedbsd.org> To: Alexander Motin <mav@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-stable@freebsd.org" <svn-src-stable@freebsd.org>, "svn-src-stable-11@freebsd.org" <svn-src-stable-11@freebsd.org> Subject: Re: svn commit: r328820 - in stable/11/sys/cam: . ata nvme scsi Message-ID: <CAPQ4fftsZJqc9fHMWqaeOSqL_AWtnY=ovooOz2yfp=s0Pecx6Q@mail.gmail.com> In-Reply-To: <201802022322.w12NMwnq074609@repo.freebsd.org> References: <201802022322.w12NMwnq074609@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Saturday, February 3, 2018, Alexander Motin <mav@freebsd.org> wrote: > Author: mav > Date: Fri Feb 2 23:22:58 2018 > New Revision: 328820 > URL: https://svnweb.freebsd.org/changeset/base/328820 > > Log: > MFC r303468 (by imp): > Move protocol specific stuff into a linker set object that's > per-protocol. This reduces the number scsi symbols references by > cam_xpt significantly, and eliminates all ata / nvme symbols. There's > still some NVME / ATA specific code for dealing with XPT_NVME_IO and > XPT_ATA_IO respectively, and a bunch of scsi-specific code, but this > is progress. > > Hi! Once you finished with the cherry-picking of nvme related commits, could you please bump the __FreeBSD_version and add them to release notes? > Modified: > stable/11/sys/cam/ata/ata_xpt.c > stable/11/sys/cam/cam_xpt.c > stable/11/sys/cam/cam_xpt_internal.h > stable/11/sys/cam/nvme/nvme_xpt.c > stable/11/sys/cam/scsi/scsi_xpt.c > Directory Properties: > stable/11/ (props changed) > > Modified: stable/11/sys/cam/ata/ata_xpt.c > ============================================================ > ================== > --- stable/11/sys/cam/ata/ata_xpt.c Fri Feb 2 23:19:20 2018 > (r328819) > +++ stable/11/sys/cam/ata/ata_xpt.c Fri Feb 2 23:22:58 2018 > (r328820) > @@ -189,6 +189,11 @@ static void ata_dev_async(u_int32_t > async_code, > void *async_arg); > static void ata_action(union ccb *start_ccb); > static void ata_announce_periph(struct cam_periph *periph); > +static void ata_proto_announce(struct cam_ed *device); > +static void ata_proto_denounce(struct cam_ed *device); > +static void ata_proto_debug_out(union ccb *ccb); > +static void semb_proto_announce(struct cam_ed *device); > +static void semb_proto_denounce(struct cam_ed *device); > > static int ata_dma = 1; > static int atapi_dma = 1; > @@ -215,6 +220,43 @@ ATA_XPT_XPORT(sata, SATA); > > #undef ATA_XPORT_XPORT > > +static struct xpt_proto_ops ata_proto_ops_ata = { > + .announce = ata_proto_announce, > + .denounce = ata_proto_denounce, > + .debug_out = ata_proto_debug_out, > +}; > +static struct xpt_proto ata_proto_ata = { > + .proto = PROTO_ATA, > + .name = "ata", > + .ops = &ata_proto_ops_ata, > +}; > + > +static struct xpt_proto_ops ata_proto_ops_satapm = { > + .announce = ata_proto_announce, > + .denounce = ata_proto_denounce, > + .debug_out = ata_proto_debug_out, > +}; > +static struct xpt_proto ata_proto_satapm = { > + .proto = PROTO_SATAPM, > + .name = "satapm", > + .ops = &ata_proto_ops_satapm, > +}; > + > +static struct xpt_proto_ops ata_proto_ops_semb = { > + .announce = semb_proto_announce, > + .denounce = semb_proto_denounce, > + .debug_out = ata_proto_debug_out, > +}; > +static struct xpt_proto ata_proto_semb = { > + .proto = PROTO_SEMB, > + .name = "semb", > + .ops = &ata_proto_ops_semb, > +}; > + > +CAM_XPT_PROTO(ata_proto_ata); > +CAM_XPT_PROTO(ata_proto_satapm); > +CAM_XPT_PROTO(ata_proto_semb); > + > static void > probe_periph_init() > { > @@ -2116,4 +2158,41 @@ ata_announce_periph(struct cam_periph *periph) > printf(")"); > } > printf("\n"); > +} > + > +static void > +ata_proto_announce(struct cam_ed *device) > +{ > + ata_print_ident(&device->ident_data); > +} > + > +static void > +ata_proto_denounce(struct cam_ed *device) > +{ > + ata_print_ident_short(&device->ident_data); > +} > + > +static void > +semb_proto_announce(struct cam_ed *device) > +{ > + semb_print_ident((struct sep_identify_data *)&device->ident_data); > +} > + > +static void > +semb_proto_denounce(struct cam_ed *device) > +{ > + semb_print_ident_short((struct sep_identify_data > *)&device->ident_data); > +} > + > +static void > +ata_proto_debug_out(union ccb *ccb) > +{ > + char cdb_str[(sizeof(struct ata_cmd) * 3) + 1]; > + > + if (ccb->ccb_h.func_code != XPT_ATA_IO) > + return; > + > + CAM_DEBUG(ccb->ccb_h.path, > + CAM_DEBUG_CDB,("%s. ACB: %s\n", ata_op_string(&ccb->ataio.cmd) > , > + ata_cmd_string(&ccb->ataio.cmd, cdb_str, > sizeof(cdb_str)))); > } > > Modified: stable/11/sys/cam/cam_xpt.c > ============================================================ > ================== > --- stable/11/sys/cam/cam_xpt.c Fri Feb 2 23:19:20 2018 (r328819) > +++ stable/11/sys/cam/cam_xpt.c Fri Feb 2 23:22:58 2018 (r328820) > @@ -747,6 +747,19 @@ cam_module_event_handler(module_t mod, int what, void > return 0; > } > > +static struct xpt_proto * > +xpt_proto_find(cam_proto proto) > +{ > + struct xpt_proto **pp; > + > + SET_FOREACH(pp, cam_xpt_proto_set) { > + if ((*pp)->proto == proto) > + return *pp; > + } > + > + return NULL; > +} > + > static void > xpt_rescan_done(struct cam_periph *periph, union ccb *done_ccb) > { > @@ -1013,6 +1026,7 @@ void > xpt_announce_periph(struct cam_periph *periph, char *announce_string) > { > struct cam_path *path = periph->path; > + struct xpt_proto *proto; > > cam_periph_assert(periph, MA_OWNED); > periph->flags |= CAM_PERIPH_ANNOUNCED; > @@ -1026,25 +1040,20 @@ xpt_announce_periph(struct cam_periph *periph, > char *a > path->target->target_id, > (uintmax_t)path->device->lun_id); > printf("%s%d: ", periph->periph_name, periph->unit_number); > - if (path->device->protocol == PROTO_SCSI) > - scsi_print_inquiry(&path->device->inq_data); > - else if (path->device->protocol == PROTO_ATA || > - path->device->protocol == PROTO_SATAPM) > - ata_print_ident(&path->device->ident_data); > - else if (path->device->protocol == PROTO_SEMB) > - semb_print_ident( > - (struct sep_identify_data *)&path->device->ident_data); > - else if (path->device->protocol == PROTO_NVME) > - nvme_print_ident(path->device->nvme_cdata, > path->device->nvme_data); > + proto = xpt_proto_find(path->device->protocol); > + if (proto) > + proto->ops->announce(path->device); > else > - printf("Unknown protocol device\n"); > + printf("%s%d: Unknown protocol device %d\n", > + periph->periph_name, periph->unit_number, > + path->device->protocol); > if (path->device->serial_num_len > 0) { > /* Don't wrap the screen - print only the first 60 chars > */ > printf("%s%d: Serial Number %.60s\n", periph->periph_name, > periph->unit_number, path->device->serial_num); > } > /* Announce transport details. */ > - (*(path->bus->xport->ops->announce))(periph); > + path->bus->xport->ops->announce(periph); > /* Announce command queueing. */ > if (path->device->inq_flags & SID_CmdQue > || path->device->flags & CAM_DEV_TAG_AFTER_COUNT) { > @@ -1070,6 +1079,7 @@ void > xpt_denounce_periph(struct cam_periph *periph) > { > struct cam_path *path = periph->path; > + struct xpt_proto *proto; > > cam_periph_assert(periph, MA_OWNED); > printf("%s%d at %s%d bus %d scbus%d target %d lun %jx\n", > @@ -1081,18 +1091,13 @@ xpt_denounce_periph(struct cam_periph *periph) > path->target->target_id, > (uintmax_t)path->device->lun_id); > printf("%s%d: ", periph->periph_name, periph->unit_number); > - if (path->device->protocol == PROTO_SCSI) > - scsi_print_inquiry_short(&path->device->inq_data); > - else if (path->device->protocol == PROTO_ATA || > - path->device->protocol == PROTO_SATAPM) > - ata_print_ident_short(&path->device->ident_data); > - else if (path->device->protocol == PROTO_SEMB) > - semb_print_ident_short( > - (struct sep_identify_data *)&path->device->ident_data); > - else if (path->device->protocol == PROTO_NVME) > - nvme_print_ident(path->device->nvme_cdata, > path->device->nvme_data); > + proto = xpt_proto_find(path->device->protocol); > + if (proto) > + proto->ops->denounce(path->device); > else > - printf("Unknown protocol device"); > + printf("%s%d: Unknown protocol device %d\n", > + periph->periph_name, periph->unit_number, > + path->device->protocol); > if (path->device->serial_num_len > 0) > printf(" s/n %.60s", path->device->serial_num); > printf(" detached\n"); > @@ -3257,7 +3262,6 @@ restart: > static void > xpt_run_devq(struct cam_devq *devq) > { > - char cdb_str[(SCSI_MAX_CDBLEN * 3) + 1]; > struct mtx *mtx; > > CAM_DEBUG_PRINT(CAM_DEBUG_XPT, ("xpt_run_devq\n")); > @@ -3269,6 +3273,7 @@ xpt_run_devq(struct cam_devq *devq) > struct cam_ed *device; > union ccb *work_ccb; > struct cam_sim *sim; > + struct xpt_proto *proto; > > device = (struct cam_ed *)camq_remove(&devq->send_queue, > CAMQ_HEAD); > @@ -3334,32 +3339,12 @@ xpt_run_devq(struct cam_devq *devq) > work_ccb->ccb_h.flags &= > ~CAM_TAG_ACTION_VALID; > } > > - switch (work_ccb->ccb_h.func_code) { > - case XPT_SCSI_IO: > - CAM_DEBUG(work_ccb->ccb_h.path, > - CAM_DEBUG_CDB,("%s. CDB: %s\n", > - scsi_op_desc(work_ccb->csio. > cdb_io.cdb_bytes[0], > - &device->inq_data), > - scsi_cdb_string(work_ccb-> > csio.cdb_io.cdb_bytes, > - cdb_str, sizeof(cdb_str)))); > - break; > - case XPT_ATA_IO: > - CAM_DEBUG(work_ccb->ccb_h.path, > - CAM_DEBUG_CDB,("%s. ACB: %s\n", > - ata_op_string(&work_ccb->ataio.cmd), > - ata_cmd_string(&work_ccb->ataio.cmd, > - cdb_str, sizeof(cdb_str)))); > - break; > - case XPT_NVME_IO: > - CAM_DEBUG(work_ccb->ccb_h.path, > - CAM_DEBUG_CDB,("%s. NCB: %s\n", > - nvme_op_string(&work_ccb->nvmeio.cmd), > - nvme_cmd_string(&work_ccb->nvmeio.cmd, > - cdb_str, sizeof(cdb_str)))); > - break; > - default: > - break; > - } > + KASSERT(device == work_ccb->ccb_h.path->device, > + ("device (%p) / path->device (%p) mismatch", > + device, work_ccb->ccb_h.path->device)); > + proto = xpt_proto_find(device->protocol); > + if (proto && proto->ops->debug_out) > + proto->ops->debug_out(work_ccb); > > /* > * Device queues can be shared among multiple SIM instances > > Modified: stable/11/sys/cam/cam_xpt_internal.h > ============================================================ > ================== > --- stable/11/sys/cam/cam_xpt_internal.h Fri Feb 2 23:19:20 2018 > (r328819) > +++ stable/11/sys/cam/cam_xpt_internal.h Fri Feb 2 23:22:58 2018 > (r328820) > @@ -66,6 +66,26 @@ SET_DECLARE(cam_xpt_xport_set, struct xpt_xport); > #define CAM_XPT_XPORT(data) \ > DATA_SET(cam_xpt_xport_set, data) > > +typedef void (*xpt_proto_announce_func)(struct cam_ed *); > +typedef void (*xpt_proto_debug_out_func)(union ccb *); > + > +struct xpt_proto_ops { > + xpt_proto_announce_func announce; > + xpt_proto_announce_func denounce; > + xpt_proto_debug_out_func debug_out; > +}; > + > +struct xpt_proto { > + cam_proto proto; > + const char *name; > + struct xpt_proto_ops *ops; > +}; > + > +SET_DECLARE(cam_xpt_proto_set, struct xpt_proto); > +#define CAM_XPT_PROTO(data) \ > + DATA_SET(cam_xpt_proto_set, data) > + > + > /* > * The CAM EDT (Existing Device Table) contains the device information for > * all devices for all busses in the system. The table contains a > > Modified: stable/11/sys/cam/nvme/nvme_xpt.c > ============================================================ > ================== > --- stable/11/sys/cam/nvme/nvme_xpt.c Fri Feb 2 23:19:20 2018 > (r328819) > +++ stable/11/sys/cam/nvme/nvme_xpt.c Fri Feb 2 23:22:58 2018 > (r328820) > @@ -152,6 +152,9 @@ static void nvme_dev_async(u_int32_t async_code, > void *async_arg); > static void nvme_action(union ccb *start_ccb); > static void nvme_announce_periph(struct cam_periph *periph); > +static void nvme_proto_announce(struct cam_ed *device); > +static void nvme_proto_denounce(struct cam_ed *device); > +static void nvme_proto_debug_out(union ccb *ccb); > > static struct xpt_xport_ops nvme_xport_ops = { > .alloc_device = nvme_alloc_device, > @@ -171,6 +174,18 @@ NVME_XPT_XPORT(nvme, NVME); > > #undef NVME_XPT_XPORT > > +static struct xpt_proto_ops nvme_proto_ops = { > + .announce = nvme_proto_announce, > + .denounce = nvme_proto_denounce, > + .debug_out = nvme_proto_debug_out, > +}; > +static struct xpt_proto nvme_proto = { > + .proto = PROTO_NVME, > + .name = "nvme", > + .ops = &nvme_proto_ops, > +}; > +CAM_XPT_PROTO(nvme_proto); > + > static void > nvme_probe_periph_init() > { > @@ -620,3 +635,29 @@ nvme_announce_periph(struct cam_periph *periph) > /* XXX NVME STUFF HERE */ > printf("\n"); > } > + > +static void > +nvme_proto_announce(struct cam_ed *device) > +{ > + nvme_print_ident(device->nvme_cdata, device->nvme_data); > +} > + > +static void > +nvme_proto_denounce(struct cam_ed *device) > +{ > + nvme_print_ident(device->nvme_cdata, device->nvme_data); > +} > + > +static void > +nvme_proto_debug_out(union ccb *ccb) > +{ > + char cdb_str[(sizeof(struct nvme_command) * 3) + 1]; > + > + if (ccb->ccb_h.func_code != XPT_NVME_IO) > + return; > + > + CAM_DEBUG(ccb->ccb_h.path, > + CAM_DEBUG_CDB,("%s. NCB: %s\n", nvme_op_string(&ccb->nvmeio. > cmd), > + nvme_cmd_string(&ccb->nvmeio.cmd, cdb_str, > sizeof(cdb_str)))); > +} > + > > Modified: stable/11/sys/cam/scsi/scsi_xpt.c > ============================================================ > ================== > --- stable/11/sys/cam/scsi/scsi_xpt.c Fri Feb 2 23:19:20 2018 > (r328819) > +++ stable/11/sys/cam/scsi/scsi_xpt.c Fri Feb 2 23:22:58 2018 > (r328820) > @@ -589,6 +589,9 @@ static void scsi_dev_async(u_int32_t async_code, > void *async_arg); > static void scsi_action(union ccb *start_ccb); > static void scsi_announce_periph(struct cam_periph *periph); > +static void scsi_proto_announce(struct cam_ed *device); > +static void scsi_proto_denounce(struct cam_ed *device); > +static void scsi_proto_debug_out(union ccb *ccb); > > static struct xpt_xport_ops scsi_xport_ops = { > .alloc_device = scsi_alloc_device, > @@ -614,6 +617,18 @@ SCSI_XPT_XPORT(ppb, PPB); > > #undef SCSI_XPORT_XPORT > > +static struct xpt_proto_ops scsi_proto_ops = { > + .announce = scsi_proto_announce, > + .denounce = scsi_proto_denounce, > + .debug_out = scsi_proto_debug_out, > +}; > +static struct xpt_proto scsi_proto = { > + .proto = PROTO_SCSI, > + .name = "scsi", > + .ops = &scsi_proto_ops, > +}; > +CAM_XPT_PROTO(scsi_proto); > + > static void > probe_periph_init() > { > @@ -3102,3 +3117,30 @@ scsi_announce_periph(struct cam_periph *periph) > printf("\n"); > } > > +static void > +scsi_proto_announce(struct cam_ed *device) > +{ > + scsi_print_inquiry(&device->inq_data); > +} > + > +static void > +scsi_proto_denounce(struct cam_ed *device) > +{ > + scsi_print_inquiry_short(&device->inq_data); > +} > + > +static void > +scsi_proto_debug_out(union ccb *ccb) > +{ > + char cdb_str[(SCSI_MAX_CDBLEN * 3) + 1]; > + struct cam_ed *device; > + > + if (ccb->ccb_h.func_code != XPT_SCSI_IO) > + return; > + > + device = ccb->ccb_h.path->device; > + CAM_DEBUG(ccb->ccb_h.path, > + CAM_DEBUG_CDB,("%s. CDB: %s\n", > + scsi_op_desc(ccb->csio.cdb_io.cdb_bytes[0], > &device->inq_data), > + scsi_cdb_string(ccb->csio.cdb_io.cdb_bytes, cdb_str, > sizeof(cdb_str)))); > +} > _______________________________________________ > svn-src-stable-11@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/svn-src-stable-11 > To unsubscribe, send any mail to " > svn-src-stable-11-unsubscribe@freebsd.org" >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPQ4fftsZJqc9fHMWqaeOSqL_AWtnY=ovooOz2yfp=s0Pecx6Q>