svn commit: r196777 - head/sys/dev/ahci

[Ivan Voras]

2009/9/3 Scott Long <scottl at samsco.org>:

> The problem is lack of kernel address space, not lack of RAM, but that's

Yes.

> just semantics in this discussion.  I've tested with increasing MAXPHYS in
> increments to 1M.  Performance increases logrithmically, and effectively
> hits a max at 512K for the variety of controllers that I tested.  The gain
> from 64K to 128K is huge, the gain from 128K to 256K is ok, the gain from
> 256k to 512k is measurable but less significant, and the gain from 512k to
> 1m is almost not measurable.

>From what I've seen with iostat, all machines I can currently test
(SATA, various SCSI-like RAID, QLogic FC HBA), 64 k is the maximum
transfer size, which I assume is DFLTPHYS. From your tests it's
apparent it is suboptimal...

> I have simple patches to increase MAXPHYS.  The introduction of the the
> maxio paramter in the CAM SIM interface is there in preparation for this.
> However, a _LOT_LOT_LOT_ of drivers in the tree falsely assume that MAXPHYS
> and DEFLTPHYS are 128k and 64k respectively, and size their data structures
> accordingly.  Changing these values will cause the drivers to fail in bad
> ways.  So an audit needs to be done.  Also, MAXPHYS is abused by the swapper
> in the struct-buf, so that needs to be reviewed as well.

Since GEOM is ok with it, couldn't a migration path be to introduce a
new tunable, something like LARGE_MAXPHYS (or to rename it completely
to MAX_IO) or whatever and use it (after reviewing cases of abuses) in
new and better maintained drivers? If the new size is larger than the
old one, maybe the maximum damage would be a little bit of wasted
space now and then?

I can test mostly ciss and mfi.

I see mfi uses it in one place that looks kind of logical:

 334         /*
 335          * Get information needed for sizing the contiguous memory for the
 336          * frame pool.  Size down the sgl parameter since we know that
 337          * we will never need more than what's required for MAXPHYS.
 338          * It would be nice if these constants were available at runtime
 339          * instead of compile time.
 340          */
 341         status = sc->mfi_read_fw_status(sc);
 342         sc->mfi_max_fw_cmds = status & MFI_FWSTATE_MAXCMD_MASK;
 343         max_fw_sge = (status & MFI_FWSTATE_MAXSGL_MASK) >> 16;
 344         sc->mfi_max_sge = min(max_fw_sge, ((MAXPHYS / PAGE_SIZE) + 1));

But ciss doesn't reference it at all so either it deviously assumes it
or is independent of it.

> Even though kernel address space is less restricted on 64bit platforms, it's
> still not free and limitless.  Large I/O's requires more work in the VM to
> assign address space, and in turn causes more lock contention.  I haven't
> done any practical measurements of this on common workloads, but I can
> anecdotally say that I see increased lock contention from it in locking
> profiles.  If FreeBSD wants to seriously increase MAXPHYS, this needs to be
> looked at and either proven to not be important, or fixed.

I think the major benefits would be for the server-side crowd using
RAID modes with some kind of striping.

-- 
f+rEnSIBITAhITAhLR1nM9F4cIs5KJrhbcsVtUIt7K1MhWJy1A==