Skip site navigation (1)Skip section navigation (2)
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>