On Tue, Feb 09, 2010 at 19:48 +0300, Mike Belopuhov wrote:
> On Tue, Feb 09, 2010 at 10:41 -0600, Marco Peereboom wrote:
> > I talked to Jim and he and I had exactly the same comments. We love
> > this minus the numbers you pull out of an orifice ;-)
> >
> > To be exact:
> > > - sc->sc_reply_post_qdepth = 128;
> > > - sc->sc_request_depth = 128;
> > > - sc->sc_num_reply_frames = 63;
> > > - sc->sc_reply_free_qdepth = 64;
> > > + sc->sc_reply_post_qdepth = 8192;
> > > + sc->sc_request_depth = 1024;
> > > + sc->sc_num_reply_frames = 1151;
> > > + sc->sc_reply_free_qdepth = 1152;
> >
> > Both choices are bad. This needs to come from the firmware and not made
> > up as we do now. Jim's values are conservative, yours are probably ok
> > for *your* chip but no telling about others. I think the port facts
> > pages has these.
> >
> > I can go ahead and commit the rest. Agree?
> >
>
> of course! calculation code above that should be used/fixed anyway.
Hi,
Calculation code is actually fine. openings was incorrect, resulting
in the dead slow performance.
With this diff i get ~130Mb/s write performace on the RAID1 w/ 15K RPM
SAS drives, which is twice i got with the request value cranked up to
the 1024.
The idea is that "openings" are treated as an amount of possible
outstanding requests in the scsi subsystem. mpii was using a value
of total maximum number of devices you can actually attach, but
that's far from the reality, which is the number of physical disks
and volumes present.
Cheers,
Mike
Index: mpii.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/mpii.c,v
retrieving revision 1.9
diff -N -u -p -u -p mpii.c
--- mpii.c 25 Feb 2010 02:05:41 -0000 1.9
+++ mpii.c 1 Mar 2010 16:42:42 -0000
@@ -1757,8 +1757,7 @@ uint32_t mpii_debug = 0
*/
#define MPII_MAX_SGL (32)
-#define MPII_MAX_REQUEST_CREDIT (500)
-#define MPII_MAX_REPLY_POST_QDEPTH (128)
+#define MPII_MAX_REQUEST_CREDIT (128)
struct mpii_dmamem {
bus_dmamap_t mdm_map;
@@ -2375,7 +2374,8 @@ mpii_attach(struct mpii_softc *sc)
sc->sc_link.adapter_softc = sc;
sc->sc_link.adapter_target = sc->sc_target;
sc->sc_link.adapter_buswidth = sc->sc_max_devices;
- sc->sc_link.openings = sc->sc_request_depth / sc->sc_max_devices;
+ sc->sc_link.openings = sc->sc_request_depth / (sc->sc_pd_count +
+ sc->sc_vd_count);
bzero(&saa, sizeof(saa));
saa.saa_sc_link = &sc->sc_link;
@@ -3037,23 +3037,11 @@ mpii_iocfacts(struct mpii_softc *sc)
sc->sc_reply_post_qdepth = sc->sc_request_depth +
sc->sc_num_reply_frames + 1;
- if (sc->sc_reply_post_qdepth > MPII_MAX_REPLY_POST_QDEPTH)
- sc->sc_reply_post_qdepth = MPII_MAX_REPLY_POST_QDEPTH;
-
if (sc->sc_reply_post_qdepth >
ifp.max_reply_descriptor_post_queue_depth)
sc->sc_reply_post_qdepth =
ifp.max_reply_descriptor_post_queue_depth;
- /* XXX JPG temporary override of calculated values.
- * need to think this through as the specs
- * and other existing drivers contradict
- */
- sc->sc_reply_post_qdepth = 128;
- sc->sc_request_depth = 128;
- sc->sc_num_reply_frames = 63;
- sc->sc_reply_free_qdepth = 64;
-
DNPRINTF(MPII_D_MISC, "%s: sc_request_depth: %d "
"sc_num_reply_frames: %d sc_reply_free_qdepth: %d "
"sc_reply_post_qdepth: %d\n", DEVNAME(sc), sc->sc_request_depth,
