On Thu, Mar 08, 2018 at 06:43:51PM +0900, Naoki Fukaumi wrote:
> Hi,
>
> thank you for your feedback!
>
> From: Jonathan Matthew <jonat...@d14n.org>
> Subject: Re: mfii(4): add bio(4) support
> Date: Thu, 8 Mar 2018 19:18:28 +1000
>
> > On Mon, Mar 05, 2018 at 09:49:16PM +0900, Naoki Fukaumi wrote:
> >> Hi tech@,
> >>
> >> This patch adds bio(4) support for mfii(4).
> >> # with "mfii(4): use MFII_FUNCTION_PASSTHRU_IO for MFI commands"
> >>
> >> most parts are taken from mfi(4), plus fix for rebuilding (bioctl -R).
> >
> > Thanks for working on this, it'd be great to have this feature.
> >
> > I tried this out on a dell server with a perc H710 (SAS2208) with an SSD
> > cache
> > in front of the disks (dell calls this cachecade) which shows up as a
> > logical
> > disk but doesn't answer scsi commands. As a result, the sensor attach code
> > fails because it can't find the scsi_link for the cache disk:
> >
> >
> >> + for (i = 0; i < sc->sc_ld_cnt; i++) {
> >> + link = scsi_get_link(sc->sc_scsibus, i, 0);
> >> + if (link == NULL)
> >> + goto bad;
> >> +
> >
> > I think this is the only case where we'll have NULL there, so we could just
> > put
> > in 'cache' as the sensor description.
>
> I didn't know about 'cachecade'. for now, I'm not sure it is better to
> expose as 'cache', or ignore it.
>
> is cachecade disk reported in bioctl(8)? (e.g. bioctl mfii0)
Yes, bioctl mfii0 shows it:
mfii0 2 Online 198910672896 RAID0 WB
0 Online 100030242816 1:14.0 noencl <ATA MZ-5EA1000-0D3
7D3Q>
1 Online 100030242816 1:15.0 noencl <ATA MZ-5EA1000-0D3
7D3Q>
If one of the cache SSDs fails, I definitely want to know about it, so I think
it
should be exposed through sensors too.
>
> > I also don't get a sensor for the battery. I haven't looked into this yet.
> > Any idea why that would happen?
>
> could you try following (only 1st hunk, only 2nd hunk, or both) patch?
> # I'm not sure this is right fix...
The 2nd hunk makes the battery sensors show up here.
>
> --- sys/dev/pci/mfii.c
> +++ sys/dev/pci/mfii.c
> @@ -3628,28 +3628,15 @@ mfii_bbu(struct mfii_softc *sc)
> }
>
> switch (bbu.battery_type) {
> - case MFI_BBU_TYPE_IBBU:
> - mask = MFI_BBU_STATE_BAD_IBBU;
> - soh_bad = 0;
> - break;
> case MFI_BBU_TYPE_BBU:
> mask = MFI_BBU_STATE_BAD_BBU;
> soh_bad = (bbu.detail.bbu.is_SOH_good == 0);
> break;
> -
> case MFI_BBU_TYPE_NONE:
> + case MFI_BBU_TYPE_IBBU:
> default:
> - sc->sc_bbu[0].value = 0;
> - sc->sc_bbu[0].status = SENSOR_S_CRIT;
> - for (i = 1; i < MFI_BBU_SENSORS; i++) {
> - sc->sc_bbu[i].value = 0;
> - sc->sc_bbu[i].status = SENSOR_S_UNKNOWN;
> - }
> - for (i = 0; i < nitems(mfi_bbu_indicators); i++) {
> - sc->sc_bbu_status[i].value = 0;
> - sc->sc_bbu_status[i].status = SENSOR_S_UNKNOWN;
> - }
> - return (0);
> + mask = MFI_BBU_STATE_BAD_IBBU;
> + soh_bad = 0;
> }
>
> status = letoh32(bbu.fw_status);
> @@ -3682,7 +3669,7 @@ mfii_create_sensors(struct mfii_softc *sc)
> strlcpy(sc->sc_sensordev.xname, DEVNAME(sc),
> sizeof(sc->sc_sensordev.xname));
>
> - if (ISSET(letoh32(sc->sc_info.mci_adapter_ops ), MFI_INFO_AOPS_BBU)) {
> + if (ISSET(letoh32(sc->sc_info.mci_hw_present), MFI_INFO_HW_BBU)) {
> sc->sc_bbu = mallocarray(4, sizeof(*sc->sc_bbu),
> M_DEVBUF, M_WAITOK | M_ZERO);
>
>
> --
> FUKAUMI Naoki
