svn commit: r228939 - head/sys/dev/mps

[John Baldwin]

On Monday, January 09, 2012 2:04:23 pm Garrett Cooper wrote:
> 2012/1/9 Alexander Motin <mav at freebsd.org>:
> > On 09.01.2012 20:54, Maksim Yevmenkin wrote:
> >>
> >> On Wed, Dec 28, 2011 at 2:49 PM, Alexander Motin<mav at freebsd.org>  wrote:
> >>
> >>> Author: mav
> >>> Date: Wed Dec 28 22:49:28 2011
> >>> New Revision: 228939
> >>> URL: http://svn.freebsd.org/changeset/base/228939
> >>>
> >>> Log:
> >>>  Set maximum I/O size for mps(4) to MAXPHYS. Looking into the code, I 
see
> >>>  no reason why it should be limited to 64K of DFLTPHYS. DMA data tag is
> >>> any
> >>>  way set to allow MAXPHYS, S/G lists (chain elements) are sufficient and
> >>>  overflows are also handled. On my tests even 1MB I/Os are working fine.
> >>>
> >>>  Reviewed by:  ken@
> >>>
> >>> Modified:
> >>>  head/sys/dev/mps/mps_sas.c
> >>>
> >>> Modified: head/sys/dev/mps/mps_sas.c
> >>>
> >>> 
==============================================================================
> >>> --- head/sys/dev/mps/mps_sas.c  Wed Dec 28 22:18:53 2011       
 (r228938)
> >>> +++ head/sys/dev/mps/mps_sas.c  Wed Dec 28 22:49:28 2011       
 (r228939)
> >>> @@ -937,6 +937,7 @@ mpssas_action(struct cam_sim *sim, union
> >>>                cpi->transport_version = 0;
> >>>                cpi->protocol = PROTO_SCSI;
> >>>                cpi->protocol_version = SCSI_REV_SPC;
> >>> +               cpi->maxio = MAXPHYS;
> >>>                cpi->ccb_h.status = CAM_REQ_CMP;
> >>>                break;
> >>>        }
> >>
> >>
> >> sorry for the late reply, but can we make in into tunable please? i
> >> have in local tree
> >>
> >> --- mps_sas.c.orig      2011-11-17 02:05:04.000000000 -0800
> >> +++ mps_sas.c   2011-12-28 16:05:10.000000000 -0800
> >> @@ -121,6 +121,11 @@
> >>
> >>  MALLOC_DEFINE(M_MPSSAS, "MPSSAS", "MPS SAS memory");
> >>
> >> +int mps_maxio = MAXPHYS;
> >> +TUNABLE_INT("hw.mps.maxio",&mps_maxio);
> >> +SYSCTL_INT(_hw_mps, OID_AUTO, maxio, CTLFLAG_RD,&mps_maxio, 0,
> >> +       "CAM maxio override\n");
> >> +
> >>  static __inline int mpssas_set_lun(uint8_t *lun, u_int ccblun);
> >>  static struct mpssas_target * mpssas_alloc_target(struct mpssas_softc *,
> >>      struct mpssas_target *);
> >> @@ -938,6 +943,7 @@
> >>
> >>                cpi->protocol = PROTO_SCSI;
> >>                cpi->protocol_version = SCSI_REV_SPC;
> >>                cpi->ccb_h.status = CAM_REQ_CMP;
> >> +               cpi->maxio = mps_maxio;
> >>                break;
> >>        }
> >>        case XPT_GET_TRAN_SETTINGS:
> >
> >
> > We can. but could you explain why? Have you found any problems this 
change?
> 
>     It would make it nice when dealing with different controllers -- a
> similar example is that mfi(4) has a tunable called hw.mfi.max_cmds
> which controls the I/O command queue size as not all MegaRAID cards
> have the same I/O queue size capabilities.

Yeah, but that's more a debugging aid.  mfi(4) has a firmware interface
that tells you how many command slots it supports dynamically.  mfi also
uses a better way to compute maxio:

        sc->ld_disk->d_maxsize = min(sc->ld_controller->mfi_max_io * secsize,
            (sc->ld_controller->mfi_max_sge - 1) * PAGE_SIZE);

It would be nice if mps could use a similar formula that was actually
limited by the device's capabilities.  The upper layers will cap transfers
to MAXPHYS elsewhere.

-- 
John Baldwin