LSI - MR-Fusion controller driver <mrsas> patch and man page

Doug Ambrisko ambrisko at cisco.com
Tue Jan 7 18:15:20 UTC 2014


On Tue, Jan 07, 2014 at 05:37:17AM +0000, Desai, Kashyap wrote:
| Doug:
| 
| I have replied to your mail at very high level and not going for inline 
| reply to your respond to capture key issue and possible resolution. 

We might want to take this discussion into our meeting on the 20th.
However, I'll give you my view on your concerns.  I understand LSI
concerns and LSI needs to understand FreeBSD's concerns since we deal
with with them a lot.
 
| Hi All,
| 
| There are two biggest challenge for <mrsas> drivers.
| 
| 1. <mrsas> conflict with <mfi> for Thunderbolt, Invader and Fury controllers. 
| As we discussed, we have resolve this giving priority to <mfi> controller 
| and only when customer wants use <mrsas> they need proper hint support.
| Currently, there are customer who is using <mrsas> posted from LSI 
| external site and able to configure using storcli and driver is working 
| well for their need.
| Definitely, we are doing lots of enhancement and defect fixes in those 
| areas, but note that before we work on actual issue, lots of customer 
| complain that <mrsas> is not loaded due to <mfi> and <mrsas> PnP ID 
| conflict. Please understand that, this is bit critical for customers who 
| want to use <mrsas>.

LSI also have a lot of customers/machines using mfi on Thunderbolt
cards that depend on various features.  We have found in the past
that transitioning to a new an improved infrastructure impacts a lot
of users so based on our last meeting I thought it was decided that
we'd work adding support to make that transition as smooth as possible
and FreeBSD people would help LSI with that effort.  This work is a win
for all since then there is a clear migration path.
 
| To this  issue, we need svn commits in <mfi> driver (from submitted 
| patch see only <mfi> related changes) to understand device.hints at 
| least, so that customer can use <mrsas> driver from lsi external site.

Yes, we can probably make the minimal change to mfi to allow mrsas to
optionally take over.  That can probably be done the quickest.
 
| Anything related to <mfi>/<mrsas> interopt can be considered a different 
| design goal and requirement.  
| E.a  "We should be fine not have alias of /dev/daX to /dev/mfidX for 
| initial drop.  Probably we should document this constraint for initial 
| commit of mrsas"

We have found this to be a problem in the past.  People will flip things
to test the waters so adding this fairly simple feature will address that.

| E.a  "Please consider that customer is not using <mfiutils> to configure 
| when <mrsas> driver is used." 
| Till now customer is not having any issue using <strocli64> with  <mrsas>. 
| We may go for good amount of interopt development to support configuration 
| utils as <mfiutils>, but that may need time.

As a long time user of LSI MegaRAID SAS cards when debugging issues we had
to run Linux binaries supplied by our system vendor to get support.  Being
able to run MegaCli/storcli is only part of what people depend on.
 
| 2. <mrsas> is only available via LSI external site.  We need <mrsas>xi
| upstream, so that customer should not wait for LSI external site downloads 
| for more generic use case.
| This can be done in different phases, as LSI is doing for <mps> driver.  
| There are good comment posted by Doug and we are working on those.
| We do not wanted to change code without posting for internal Q/A testing. 
| We should take advantage of LSI internal Q/A testing to fix most of the 
| defect internally.

That is the goal we are working towards and we had agreed on things to
do to get mrsas into FreeBSD.  Getting mrsas into FreeBSD will also
give LSI additional real work testing and bug fixes from the wild.  So that
is a win/win.
 
| To resolve above issue, we should find middle way to post the <mrsas> 
| check-in in mainline.  I mean, keep few know limitation as documented and 
| do not wait for gathering of all feature in single drop. 
| E.a  If we start changing code to meet style(9) coding standard, we may 
| add some manual mistakes in code.  We can take each action item as 
| Enhancement request (Including style(9) change as well) for LSI and we 
| will be doing all those step by step.( As we are doing for <mps> driver).

The problem with ignoring style causes a bunch of issues.  The white space
style issues should be delt with right away.  That doesn't change the code
and the resulting code can be checked to make sure the resulting code
doesn't change.  Once people start using ioctl's and what are in the
ioctl's then changing style in headers later becomes a problem.  Going out
on a limb a bit here, I'm as concerned about things that can be changed
in the future and have no impact outside the /sys/dev/mrsas area (again,
ioctl's are outside that that is a problem).  However, once code gets in
then sometimes style issues don't get fixed.
 
| I am not aware of project method to do svn check, since I worked with 
| Ken D Merry earlier to do actual commit in FreeBSD head repo. I thought 
| it is going to be same way for <mrsas>.

I know we have GSoC (Google Summer of Code) projects checked in by folks
that were not committers but I'm not sure if we can do that with subversion.
I need to check or someone else can chime in.  It would be nice if LSI
could directly check into a project and then we can have people test that
and merge that into head.

I think things are more complicated for mrsas just due to the requirements
people have for LSI MegaRAID SAS.  As it we can't use mrsas for any of our
products since we have an infrastructure depending on features that mfi
has.  Making the transition smoother is a win for everyone so that if
some say mfi can do this and mrsas can do it, it just makes things easier.
For me I know I did something pretty well if people use things I've done
but never ask me questions about it.
 
| Sorry, but doing lots of discussion in single mail thread is prone to 
| divert actual agenda and we may hang forever without any conclusion.
| So, let's open defect in <mrsas> as soon as it get upstream, LSI will 
| work for all the defect against <mrsas>. 
| At least immediate check in <mfi> diver to understand device.hints, 
| will allow customer to use <mrsas> from LSI external site.

Again, this isn't typically how it works unless you can find someone that
will take the responsibility of all of the issues and make it right.  I for
one do not want to check it in and take the back lash.  I try to work
within the FreeBSD guidelines for code and style is pretty important.
I don't want to rehash old issues but working better with FreeBSD from
the start could have avoided a bunch of issues.  LSI did a bunch of good
work but things could have been done more optimal so mrsas could have been
in FreeBSD sooner.

Of course I'm only one person in FreeBSD so other people might have other
views and if they see an easier way to help LSI then I'm all for that.
I'm just stating what I see needs to be done before I check it in.

So to me gating issues to check it in are:
     - 	performance
     -	style
     -	mfiutil ioctl support
     -	alias support
     -	Scott's code review
     -	man page review by doc's team
     -	testing

I know this is additional work on everyone's side but it really does make
things better.  I have WIP code that I'd love to check into FreeBSD but
it is not ready for FreeBSD standards.  People have given me suggestions
of which I've gone back to make things better.  So that is a good thing
the only problem I have is limited time to do the changes and test the
changes.  So I understand the pain.

Having the driver available in a SCM repository would be great
since various people can help make simple changes.  Maybe putting it
up in git might help if we can't make a subversion project work?
 
Thanks,

Doug A.
 
| > -----Original Message-----
| > From: Doug Ambrisko [mailto:ambrisko at cisco.com]
| > Sent: Tuesday, January 07, 2014 12:00 AM
| > To: Desai, Kashyap
| > Cc: freebsd-scsi at freebsd.org; scottl at netflix.com; sean_bruno at yahoo.com;
| > dwhite at ixsystems.com; jpaetzel at freebsd.org; Maloy, Joe; Mankani,
| > Krishnaraddi; McConnell, Stephen; Saxena, Sumit; Radford, Adam; Kenneth
| > D. Merry
| > Subject: Re: LSI - MR-Fusion controller driver <mrsas> patch and man page
| > 
| > On Mon, Jan 06, 2014 at 07:43:27AM +0000, Desai, Kashyap wrote:
| > | Doug:
| > |
| > | I have replied inline for your comment. For most of the comment you
| > | posted below can be work out and we can prioritize completion of those
| > | based on nature of issue.
| > | I will work on READ/WRITE comparison w.r.t <mfi> and <mrsas> as a
| > | priority item.
| > 
| > IO performance is the highest priority.
| > 
| > | Please consider that it will be really helpful for further
| > | development, if we have common code check-in in Upstream FreeBSD. We
| > | will actively work on each comment posted for <mrsas> and address
| > | those before we have <mrsas> in-box for any FreeBSD release.
| > 
| > What we have done in the past is to create a project branch to do this and it
| > has worked well.  It would be nice if you could check in directly to a this
| > project branch so we could use svn to resolve differences.
| > 
| > I poked around at adding the ioctl compatibility support and noticed that
| > mrsas seems to lack a general method to send a dcmd to the controller.
| > What Scott did in mfi was to create a generic function to send dcmd's with a
| > simple data buffer.  That data buffer was then put into the sg list in the
| > generic function.  This made it easier to translate the various ioctl's since we
| > could convert the sg lists passed down from the user land into a simple
| > malloced data base and then covert back out.
| > In the mfituil ioctl method it just passes down the data buffer and then
| > let's the driver convert it to a sg list.   To make ioctl emulation
| > easier in these cases:
| > 	32 bit FreeBSD LSI ioctl on 64 bit
| > 	32 bit Linux LSI ioctl on 32 bit
| > 	32 bit Linux LSI ioctl on 64 bit
| > 	32 bit mfiutil ioctl on 32 bit
| > 	32 bit mfiutil ioctl on 64 bit
| > 
| > Of high priority is style, please make it style(9) compliant.  There are a trailing
| > spaces, spaces instead of tabs, I think lack of tab aligned indent.  I noticed
| > functions being externally defined in .c files instead of .h.  There should be .h
| > files that can be included in user land applications for ioctls and there can be
| > .h files just for the kernel.
| > These files will need to be installed as appropriate.  I found this with the ioctl
| > function.
| > 
| > | > -----Original Message-----
| > | > From: Doug Ambrisko [mailto:ambrisko at cisco.com]
| > | > Sent: Saturday, January 04, 2014 2:45 AM
| > | > To: Desai, Kashyap
| > | > Cc: freebsd-scsi at freebsd.org; scottl at netflix.com;
| > | > sean_bruno at yahoo.com; dwhite at ixsystems.com;
| > jpaetzel at freebsd.org;
| > | > Maloy, Joe; Mankani, Krishnaraddi; McConnell, Stephen; Saxena,
| > | > Sumit; Radford, Adam; Kenneth D. Merry
| > | > Subject: Re: LSI - MR-Fusion controller driver <mrsas> patch and man
| > | > page
| > | >
| > | > On Fri, Dec 06, 2013 at 02:37:21PM +0000, Desai, Kashyap wrote:
| > | > | Hi,
| > | > |
| > | > | Please consider attached patch for FreeBSD upstream check-in.
| > | > |
| > | > | Please find attached patch for <mrsas> driver for LSI's MegaRaid
| > | > Controllers. This driver supports Thunderbolt onwards Device IDs of
| > | > MR controllers.
| > | > | Currently it supports 0x005B and 0x005D Device IDs.
| > | > |
| > | > | NOTE : This driver will not eliminate or by pass any functionality
| > | > | of <mfi>
| > | > driver which already support above to Device IDs to keep existing
| > | > user experience unchanged.
| > | > |
| > | > | <mfi> Driver will be always given priority over <mrsas> driver and
| > | > | only if customer/user wants to use/migrate from <mfi> to <mrsas>,
| > | > | it will hook up into kernel via device.hint rules. (Attached is
| > | > | mrsas man page for more info.)
| > | > |
| > | > | LSI will continue to update <mrsas> driver in future in timely bases.
| > | > | We have another set of patch in pipeline to be submitted for
| > | > | <mrsas>, but need first go ahead for attached patch and later we
| > | > | will continue to keep <mrsas> up-to-date (In sync with LSI
| > | > | released driver which is available at lsi's external site)
| > | > |
| > | > | Apply patch with "patch -p0 < patchname.patch" from head directory.
| > | > |
| > | > | -- Few notes for user--
| > | > | LSI recommends using fusion_force bit In hint settings at start of
| > | > | the day, if
| > | > they want to use <mrsas>. ( <mfi> will be a default choice for
| > | > MR-Fusion HBA), if will be changed only with fusion_force hint
| > | > settings. (See mrsas man
| > | > page) Changing any default behavior is well tested for most of the
| > condition.
| > | > | Switching from <mfi> to <mrsas> for MR-Fusion options is designed
| > | > | to
| > | > allow user as one time choice, though multiple time switch from
| > | > <mfi> to <mrsas> is possible, it is not recommended. So, user needs
| > | > to decide from start of the day, which driver they want to use for MR-
| > Fusion  card.
| > | > |
| > | > | -- Implementation details --
| > | > | To support this feature, we have modify <mfi> code to change
| > | > | default
| > | > return type from probe. Currently <mfi> driver return
| > | > "BUS_PROBE_DEFAULT". <mfi> driver has been be changed to return
| > | > "BUS_PROBE_LOW_PRIORITY" if fusion_force hint from device.hints  is
| > set.
| > | > | Please notice, above mentioned implementation in <mfi> driver is
| > | > | only
| > | > applicable in case of  MR-Fusion controller detection. For any other
| > | > controllers, supported by <mfi> driver, the behavior of probe return
| > | > will remain same as before.
| > | > |
| > | > |
| > | > | -- High level feature list of <mrsas> -- 1. Supports Fast Path
| > | > | feature of LSI controllers.
| > | > | 2. Supports 4K sector Drives.
| > | > | 3. CAM layer based interface. All VDs will be attached to CAM
| > | > | layer (Expected storage will be visible in "camcontroll") 4.
| > | > | Complete support of Online Controller Reset. (OCR) 5.  OCR on
| > | > | Fimrware fault and IO
| > | > timeout case.
| > | > | 6. Work well with <storcli> management application which is
| > | > | generic
| > | > application provided by LSI for all other Operating system.
| > | > | 7. Supporst DIF enabled VDs (Same support as provided in Linux and
| > | > | other OSes in FreeBSD) 8. Fast Path Load balance support.
| > | > |
| > | > | - In summary, this driver is in part with Linux based MR drivers
| > | > | and all other features will be available to <mrsas> as planned
| > | > | activity from LSI
| > | > |
| > | > | This code is well tested by LSI Q/A team on 32 bit and 64 bit
| > | > | FreeBSD
| > | > Released OSes.
| > | >
| > | > Sorry it is has taken so long to start playing with this.  I found
| > | > one critical issue with read performance.  I added a printf to the
| > | > driver to make sure the fast path is not being used and it claims it isn't.
| > | > Doing either a dd or using the
| > | > dt(http://www.scsifaq.org/RMiller_Tools/dt.html)
| > | > I see good write performance with both mrsas and mfi but on read
| > | > mrsas is backing up and appears to be single threaded.  With mrsas
| > | > doing a dd of=/dev/zero if=/dev/da0 bs=1m I'm seeing 45685 kBps and
| > | > with mfi seeing
| > | > 597072 kBps via gstat.  Using dt via:
| > | > 	dt of=/dev/da0 bs=64k limit=1g aios=32 I see writes at 538977
| > | > Kbytes/sec and reads at bouncing around between
| > | > 20000 - 8000 Kbytes/sec, mfi reports 244744 Kbytes/sec.  The
| > | > interesting piece is seeing the L(q) via gstat being around 30 when
| > | > doing mrsas reads, writes are 1 or less.  With mfi both read and writes are
| > 1 or less.
| > | >
| > | Thanks for for providing feedback. This is very important observation.
| > | We will work on this and get back to you.  Meanwhile, if we have high
| > | level acceptance(considering few defect fixes and optimization as a
| > | follow up activity) of current <mrsas> code, can have <mrsas> commit
| > | in current FreeBSD driver. This will definitely help us to manage
| > | source code from single destination. Currently there are multiple
| > | flavors/version of <mrsas> driver code is floated due to Upstream
| > | commit is pending. We can work for each observation/optimization
| > | proposed from upstream community on priority bases.
| > 
| > Yes a project branch can help with this.
| > 
| > | > This is with a GENERIC kernel on -current/amd64.  I updated my
| > | > source to
| > | > 260231 and see the same.  Witness, etc. are all on.  I don't recall
| > | > seeing this when I tested mrsas a long time ago.
| > |
| > | Thanks for additional info.
| > |
| > | >
| > | > The card I'm using is:
| > | >   LSI MegaRAID SAS 9266-8i
| > | >   Firmware BIOS 5.38.00_4.12.05.00_0x05180000
| > | >   Firmware BCON 6.1-49-e_49-Rel
| > | >   Firmware PCLI 05.05-03:#%00011
| > | >   Firmware APP  3.220.75-2196
| > | >   Firmware NVDT 2.1209.03-0117
| > | >   Firmware BTBL 2.05.00.00-0010
| > | >   Firmware BOOT 07.26.13.219
| > | >
| > | > Some other issues I ran into when I kldunload mrsas I got a:
| > | > 	sysctl_unregister_oid: failed to unregister sysctl and then when I
| > |
| > | I do not recall if we have seen this issue, but will try this on our setup.
| > 
| > It think this and the panic might have been an artifact of my inition testing.  I
| > followed the instruction of kldload mrsas from /boot/loader.conf and saw
| > this.  However, when I started working the emulation I modified my
| > GERNERIC config to not have mfi in it so I could switch between mfi and
| > mrsas via kldload/kldunload to test ioctl emulation.  When I did this I noticed
| > that the patch included mrsas in the GENERIC configuration.
| > That might have caused this issue having both the module and static driver in
| > the same system.  So this is probably a bad bug report.
| > Doing kldload and kldunload without mrsas and mfi static in the kernel I didn't
| > run into this again.
| > 
| > | > kldload mrsas I got a panic:
| > | > mrsas0: <LSI Thunderbolt SAS Controller> port 0x7000-0x70ff mem
| > | > 0xdf060000-0xdf0 63fff,0xdf000000-0xdf03ffff irq 32 at device 0.0 on
| > | > pci4
| > #########################################################
| > | > <mrsas> Driver has detected MR-Fusion Controller.If you wish to
| > | > switch to <mfi> driver for MR-Fusion,read manual of mrsas. Once you
| > | > chose to use <mrsas> Driverf or MR-Fusion Controllers, you should
| > | > not switch to <mfi>.LSI recommend to use <m
| > | > rsas> driver for MR-Fusion.Use <mrsas> if you are not sure about
| > | > rsas> choice.Quick he
| > | > lp to use <mfi> for MR-Fustion Card:
| > | >  Remove hint.mrsas.0.fusion_force=1 from /boot/device.hints.
| > | >
| > #########################################################
| > | > mrsas0: Waiting for FW to come to ready state
| > | >
| > | > <panic message from malloc>
| > | >
| > | > db> tr
| > | > Tracing pid 534 tid 100245 td 0xfffff80015035490
| > | > malloc() at malloc+0x1c8/frame 0xfffffe03fbab3200
| > | > cpuctl_devs() at cpuctl_devs+0x61df/frame 0xfffffe03fbab3290
| > | > cpuctl_devs() at cpuctl_devs+0x6189/frame 0xfffffe03fbab32f0
| > | > cpuctl_devs() at cpuctl_devs+0x990a/frame 0xfffffe03fbab34b0
| > | > device_attach() at device_attach+0x3a2/frame 0xfffffe03fbab3510
| > | > pci_driver_added() at pci_driver_added+0xfa/frame 0xfffffe03fbab3550
| > | > devclass_driver_added() at devclass_driver_added+0x7d/frame
| > | > 0xfffffe03fbab3590
| > | > devclass_add_driver() at devclass_add_driver+0x127/frame
| > | > 0xfffffe03fbab35d0
| > | > module_register_init() at module_register_init+0xb0/frame
| > | > 0xfffffe03fbab3600
| > | > linker_load_module() at linker_load_module+0xbda/frame
| > | > 0xfffffe03fbab3930
| > | > kern_kldload() at kern_kldload+0xad/frame 0xfffffe03fbab3970
| > | > sys_kldload() at sys_kldload+0x5b/frame 0xfffffe03fbab39a0
| > | > amd64_syscall() at amd64_syscall+0x265/frame 0xfffffe03fbab3ab0
| > | > Xfast_syscall() at Xfast_syscall+0xfb/frame 0xfffffe03fbab3ab0
| > | >
| > | > I played with storcli/storcli64 and MegaCli/MegaCli64 from LSI's web site.
| > | > I noticed that the 32 bit versions are built for FreeBSD 8.1 and the
| > | > 64 bit FreeBSD 7.4.  It might not be a critical problem but some
| > | > companies might still be on earlier releases.
| > | >
| > | > I need to look at adding the ioctl translation support for mfiutil
| > | > and Linux to mrsas.  This means we should have a 32 bit to 64 bit
| > | > ioctl translator in the driver since our Linux emulation is 32 bit.
| > | We can pick this work as next action item.
| > 
| > I started working on this and mentioned some things that would make this
| > easier.
| > 
| > | > It's also handy to run 32 bit tools on the 64 bit kernel.
| > | >
| > | > It would be good to add alias support for /dev/mfid* such as was
| > | > done via ATA CAM sys/cam/ata/ata_da.c.  Search for
| > | > ada_legacy_aliases and legacy_id.  The helped the migration from
| > /dev/ad* to /dev/ada*.
| > | >
| > | Let me check this. You can help me to understand what is expected here
| > | and we can provide solution for this.
| > | Again, we can mark this as next to-do for mrsas and will provide patch
| > | as an when we have code ready.
| > 
| > The idea is that when /dev/da<x> node is created and alias of /dev/mfi<y> is
| > created point to /dev/da<x>.  This way existing fstab that reference
| > /dev/mfid0 for example will continue to work.  It just makes it easier for
| > migration.
| > 
| > A trickier part will be the feature to allow both LD and PD of an LD to be
| > available in a system.  It is a feature that people use.  With mfi it is easy to see
| > RAID controlled devices via /dev/mfid<x> and mfisyspd<x>.
| > The physical device access come up as /dev/da<x>.  So people should know
| > be very carefull when accessing /dev/da<x> since it could upset the RAID
| > controller, such as flashing firmware and when the disk goes offline it can kick
| > out the disk from the RAID.
| > 
| > I haven't looked at how mrsas deals with deleting and recreating RAID on the
| > fly.  mfi can do it with the proper sysctl/tunables set.
| > 
| > | > I'm not sure hint.mrsas.<x>.fusion_force="1" is a good toggle to
| > | > enable/disable mrsas.  I can see why you did that but it might be
| > | > more obvious to make hint.mfi.<x>.disabled="1" to work since that
| > | > follows more of what people are used to.
| > |
| > | Either way is fine, since hint.mfi.<X>.disabled may miss lead that we
| > | are going to disable <mfi> completely.
| > | We may have <mfi> driver working for Falcon and other old MFI based
| > | controllers.
| > 
| > Since it is a hint.mfi.<x>.disabled, the x refers to card x, ie.
| > hint.mfi.1.disabled would disable the 2nd card in the system what would be
| > the ThunderBolt based can and leave the others to be mfi.  We should get
| > more input from FreeBSD folks.  fusion_force seems pretty non-obvious to
| > me.  I'd suggest force_mrsas to be more obvious and it should probably be a
| > generic tunabled and device instance specific.
| > 
| > Adding printing events to the kernel messages seems to be missing.
| > This needs to be controlled by sysctl/tunable but I might not have seen this
| > yet.
| > 
| > I'd say the highest priority is on performance and style.
| > 
| > Getting a review from Scott would be good but I'm not sure he has had time
| > to do that.  It might be good to look at mfi to see how things are laid out.
| > 
| > FYI, my biggest problem working on FreeBSD is time.  My day job priorities
| > end up eating into time to work on "less critical" issues.  Unfortunately a
| > bunch of FreeBSD things fall into that catagory.  I need to really resume on
| > my mount path changes.  I know what I need to do, I just need to carve out
| > some time to work on it.
| > 
| > Thanks,
| > 
| > Doug A.


More information about the freebsd-scsi mailing list