On Sunday, January 29, 2006 12:19 PM, James Bottomley wrote:
> This patch adds the transport class to the fusion mptspi. I've fixed
> the problems with sas and virtual channels (the use of virtual channel
> number is now local to mptspi). I've also extracted the GEM processor
> work arounds ... since these things are visible to all HBA's,
> this needs
> to be resurrected as a mid-layer blacklist item.
I'm not sure what is getting broke when these GEM workarounds are being
removed. These hacks were added by someone else.
>
> James
>
> [PATCH] Add SPI Transport class to mptfusion
>
> This patch adds the transport class to mptspi, makes sure RAID devices
> don't attach and exposes the underlying devices of RAID volumes on a
> virtual channel with the no_uld_attach flag set (so they can be
> addressed by sg and have DV performed on them).
>
My comments below:
> +static int mptspi_read_page0(struct scsi_target *starget,
> + struct _CONFIG_PAGE_SCSI_DEVICE_0
> *pass_pg0)
Cosmetic, but Id rather this function be called
mptspi_read_spi_device_page0(), so I know which page we
are reading.
> + if (sdev->channel) {
> + VirtDevice *vdev = sdev->hostdata;
> + sdev->no_uld_attach = 1;
> + vdev->vtarget->tflags |=
MPT_TARGET_FLAGS_RAID_COMPONENT;
> + /* The real channel for this device is zero */
> + vdev->bus_id = 0;
> + }
Id rather we being creating a virtual channel for the RAID volumes on
channel=1, and leave the hidden disks on channel=0 along with all other
real devices. That is what we are doing in mptsas.c, and I would like
to keep similar logic so we are not confusing the customer where the
raid
volumes are located. For instance, the 1068 has upto 8 ports, so we
put the virtual channel out on channel=8. I'm not sure how the
sdev->no_uld_attach flag works, e.g my concern is whether this flag
would work when we mix real exposed devices and hidden disk on
the same channel? If that works, would it be possible for you to change
this?
> +static int
> +mptscsih_quiesce_raid(MPT_SCSI_HOST *hd, int quiesce, int disk)
I would like all the raid related functions, such as this one,
and mptspi_get_resyn, mptspi_get_state, and mptspi_is_raid moved
out of the mptspi driver, and over into a generic api, such as inside
mptscsih.c. These same functions will be needed for SAS RAID Transport
Class, and I don't what to have to replicate the code over there in
mptsas.c
> +static int mptspi_write_page1(struct scsi_target *starget,
> + struct
Cosmetic, but Id rather this function be called
mptspi_write_spi_device_page0(), so I know which page we
are writing.
> /* 4. Renegotiate to all devices, if SPI
> */
> - if (ioc->bus_type == SPI) {
> - dnegoprintk(("writeSDP1: ALL_IDS USE_NVRAM\n"));
> - mptscsih_writeSDP1(hd, 0, 0, MPT_SCSICFG_ALL_IDS
| MPT_SCSICFG_USE_NVRAM);
> - }
We can't be removed !!!!
If you do host_reset - via eh_threads or IOCTL = MPTHARDRESET,
all the devices will go to asyn-narrow. The
negotiation parameters need to be updated. My suggestion is we can
hook
the reset handler chain from mptspi. This is done by over riding the
mpt_reset_register() call with local function. Then from it during
the correct "reset phase", call mptspi_dv_device.
> - pScsiReq->Function = MPI_FUNCTION_SCSI_IO_REQUEST;
> + if (vdev->vtarget->tflags & MPT_TARGET_FLAGS_RAID_COMPONENT)
> + pScsiReq->Function = >
MPI_FUNCTION_RAID_SCSI_IO_PASSTHROUGH;
> + else
> + pScsiReq->Function = MPI_FUNCTION_SCSI_IO_REQUEST;
> pScsiReq->CDBLength = SCpnt->cmd_len;
YOur only queise io when commands are issued in the context of
mptspi_dv_device,
meaning mptscsih_quiese_raid() is called over there.
But what about someone opening a /dev/sg0 handle, and sending data
across? Do you think
we should be queuiese io for all io doing RAID_PASSTHRU from
queue_command entry point,
instead of only handling the dv case. But what about the performance
hit of loading
driver when dv is going on. Any thoughts on this?
> - /* F/W not running */
> - if(!CHIPREG_READ32(&ioc->chip->Doorbell)) {
> - /* enable domain validation flags */
> - for (ii=0; ii < MPT_MAX_SCSI_DEVICES; ii++) {
> - ioc->spi_data.dvStatus[ii] |=
> MPT_SCSICFG_NEED_DV;
> - }
> - }
You have deleted the code in mpt_resume() that issues domain validation.
Are you handling power managment from transport layer? There is no
gareentee
that device were removed and added during standby/hybernation, thus
what happens when initiator is running packatized, and the device having
power cut off is running non-packatized?
>
>
>
>
Attached is a patch that apply over your patch. What this does it addes
the SAS and misc fix' patches that were missed from two weeks back. I
also:
(1) deleted the spi performance bug code "mpt_bringup_adapter", which is
not
need for generic dv
(2) changed the RAID events to KERN_DEBUG, as you requested earlier
today.
James: I would like to get this pushed thru this time. Your help is
greatly appriecated.
Eric Moore
Attachment:
dv-merged.patch
Description: dv-merged.patch
