Hi Jonathan Matthew,
From: YASUOKA Masahiko <yasu...@openbsd.org>
Subject: Re: mfi(4): Make "bioctl -R" work after hot swapping
Date: Tue, 24 Apr 2018 20:20:40 +0900 (JST)
> On Thu, 29 Jun 2017 17:14:41 +0900 (JST)
> FUKAUMI Naoki <fuka...@soum.co.jp> wrote:
>> Currently "bioctl -R" works only if disk state is "Offline" (set by
>> "bioctl -O") and it doesn't work for "Failed" disk.
>>
>> To make it work with hot swapped disk, report unused ("unconfigured" in
>> MegaRAID) disk to userland, and handle it properly when rebuilding.
>
> Let me update the diff.
>
> ok? comments?
here is updated patch which includes following fixes from mfii(4),
* don't return on error while updating sensors
* fix BBU detection
* fix RAID level of spanned logical disk
* report cachecade disk in ioctl/sensor
could you review it?
diff --git a/sys/dev/ic/mfi.c b/sys/dev/ic/mfi.c
index 57fe533..af39b52 100644
--- a/sys/dev/ic/mfi.c
+++ b/sys/dev/ic/mfi.c
@@ -121,7 +121,7 @@ int mfi_bio_hs(struct mfi_softc *, int, int, void
*);
#ifndef SMALL_KERNEL
int mfi_create_sensors(struct mfi_softc *);
void mfi_refresh_sensors(void *);
-int mfi_bbu(struct mfi_softc *);
+void mfi_bbu(struct mfi_softc *);
#endif /* SMALL_KERNEL */
#endif /* NBIO > 0 */
@@ -1582,7 +1582,8 @@ mfi_ioctl(struct device *dev, u_long cmd, caddr_t addr)
int
mfi_bio_getitall(struct mfi_softc *sc)
{
- int i, d, size, rv = EINVAL;
+ int i, d, rv = EINVAL;
+ size_t size;
union mfi_mbox mbox;
struct mfi_conf *cfg = NULL;
struct mfi_ld_details *ld_det = NULL;
@@ -1716,8 +1717,13 @@ mfi_ioctl_vol(struct mfi_softc *sc, struct bioc_vol *bv)
i = bv->bv_volid;
link = scsi_get_link(sc->sc_scsibus, i, 0);
- if (link != NULL && link->device_softc != NULL) {
+ if (link == NULL) {
+ strlcpy(bv->bv_dev, "cache", sizeof(bv->bv_dev));
+ } else {
dev = link->device_softc;
+ if (dev == NULL)
+ goto done;
+
strlcpy(bv->bv_dev, dev->dv_xname, sizeof(bv->bv_dev));
}
@@ -1769,8 +1775,7 @@ mfi_ioctl_vol(struct mfi_softc *sc, struct bioc_vol *bv)
* a subset that is valid for the MFI controller.
*/
bv->bv_level = sc->sc_ld_details[i].mld_cfg.mlc_parm.mpa_pri_raid;
- if (sc->sc_ld_details[i].mld_cfg.mlc_parm.mpa_sec_raid ==
- MFI_DDF_SRL_SPANNED)
+ if (sc->sc_ld_details[i].mld_cfg.mlc_parm.mpa_span_depth > 1)
bv->bv_level *= 10;
bv->bv_nodisk =
sc->sc_ld_details[i].mld_cfg.mlc_parm.mpa_no_drv_per_span *
@@ -1790,11 +1795,12 @@ mfi_ioctl_disk(struct mfi_softc *sc, struct bioc_disk
*bd)
struct mfi_array *ar;
struct mfi_ld_cfg *ld;
struct mfi_pd_details *pd;
+ struct mfi_pd_list *pl;
struct mfi_pd_progress *mfp;
struct mfi_progress *mp;
struct scsi_inquiry_data *inqbuf;
char vend[8+16+4+1], *vendp;
- int rv = EINVAL;
+ int i, rv = EINVAL;
int arr, vol, disk, span;
union mfi_mbox mbox;
@@ -1810,6 +1816,7 @@ mfi_ioctl_disk(struct mfi_softc *sc, struct bioc_disk *bd)
cfg = sc->sc_cfg;
pd = malloc(sizeof *pd, M_DEVBUF, M_WAITOK);
+ pl = malloc(sizeof *pl, M_DEVBUF, M_WAITOK);
ar = cfg->mfc_array;
vol = bd->bd_volid;
@@ -1833,13 +1840,53 @@ mfi_ioctl_disk(struct mfi_softc *sc, struct bioc_disk
*bd)
/* offset disk into pd list */
disk = bd->bd_diskid % ld[vol].mlc_parm.mpa_no_drv_per_span;
- bd->bd_target = ar[arr].pd[disk].mar_enc_slot;
+
+ if (ar[arr].pd[disk].mar_pd.mfp_id == 0xffffU) {
+ /* disk is missing but succeed command */
+ bd->bd_status = BIOC_SDFAILED;
+ rv = 0;
+
+ /* try to find an unused disk for the target to rebuild */
+ if (mfi_mgmt(sc, MR_DCMD_PD_GET_LIST, MFI_DATA_IN,
+ sizeof *pl, pl, NULL))
+ goto freeme;
+
+ for (i = 0; i < pl->mpl_no_pd; i++) {
+ if (pl->mpl_address[i].mpa_scsi_type != 0)
+ continue;
+
+ memset(&mbox, 0, sizeof(mbox));
+ mbox.s[0] = pl->mpl_address[i].mpa_pd_id;
+ if (mfi_mgmt(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN,
+ sizeof *pd, pd, &mbox))
+ continue;
+
+ if (pd->mpd_fw_state == MFI_PD_UNCONFIG_GOOD ||
+ pd->mpd_fw_state == MFI_PD_UNCONFIG_BAD)
+ break;
+ }
+
+ if (i == pl->mpl_no_pd)
+ goto freeme;
+ } else {
+ memset(&mbox, 0, sizeof(mbox));
+ mbox.s[0] = ar[arr].pd[disk].mar_pd.mfp_id;
+ if (mfi_mgmt(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN,
+ sizeof *pd, pd, &mbox)) {
+ bd->bd_status = BIOC_SDINVALID;
+ goto freeme;
+ }
+ }
+
+ /* get the remaining fields */
+ bd->bd_channel = pd->mpd_enc_idx;
+ bd->bd_target = pd->mpd_enc_slot;
/* get status */
- switch (ar[arr].pd[disk].mar_pd_state){
+ switch (pd->mpd_fw_state){
case MFI_PD_UNCONFIG_GOOD:
- case MFI_PD_FAILED:
- bd->bd_status = BIOC_SDFAILED;
+ case MFI_PD_UNCONFIG_BAD:
+ bd->bd_status = BIOC_SDUNUSED;
break;
case MFI_PD_HOTSPARE: /* XXX dedicated hotspare part of array? */
@@ -1850,6 +1897,10 @@ mfi_ioctl_disk(struct mfi_softc *sc, struct bioc_disk
*bd)
bd->bd_status = BIOC_SDOFFLINE;
break;
+ case MFI_PD_FAILED:
+ bd->bd_status = BIOC_SDFAILED;
+ break;
+
case MFI_PD_REBUILD:
bd->bd_status = BIOC_SDREBUILD;
break;
@@ -1858,27 +1909,15 @@ mfi_ioctl_disk(struct mfi_softc *sc, struct bioc_disk
*bd)
bd->bd_status = BIOC_SDONLINE;
break;
- case MFI_PD_UNCONFIG_BAD: /* XXX define new state in bio */
+ case MFI_PD_COPYBACK:
+ case MFI_PD_SYSTEM:
default:
bd->bd_status = BIOC_SDINVALID;
break;
}
- /* get the remaining fields */
- memset(&mbox, 0, sizeof(mbox));
- mbox.s[0] = ar[arr].pd[disk].mar_pd.mfp_id;
- if (mfi_mgmt(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN,
- sizeof *pd, pd, &mbox)) {
- /* disk is missing but succeed command */
- rv = 0;
- goto freeme;
- }
-
bd->bd_size = pd->mpd_size * 512; /* bytes per block */
- /* if pd->mpd_enc_idx is 0 then it is not in an enclosure */
- bd->bd_channel = pd->mpd_enc_idx;
-
inqbuf = (struct scsi_inquiry_data *)&pd->mpd_inq_data;
vendp = inqbuf->vendor;
memcpy(vend, vendp, sizeof vend - 1);
@@ -1898,6 +1937,7 @@ mfi_ioctl_disk(struct mfi_softc *sc, struct bioc_disk *bd)
rv = 0;
freeme:
free(pd, M_DEVBUF, sizeof *pd);
+ free(pl, M_DEVBUF, sizeof *pl);
return (rv);
}
@@ -2008,27 +2048,133 @@ done:
return (rv);
}
+static int
+mfi_makegood(struct mfi_softc *sc, uint16_t pd_id)
+{
+ struct mfii_foreign_scan_info *fsi;
+ struct mfi_pd_details *pd;
+ union mfi_mbox mbox;
+ int rv;
+
+ fsi = malloc(sizeof *fsi, M_DEVBUF, M_WAITOK);
+ pd = malloc(sizeof *pd, M_DEVBUF, M_WAITOK);
+
+ memset(&mbox, 0, sizeof mbox);
+ mbox.s[0] = pd_id;
+ rv = mfi_mgmt(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN, sizeof(*pd), pd,
+ &mbox);
+ if (rv != 0)
+ goto done;
+
+ if (pd->mpd_fw_state == MFI_PD_UNCONFIG_BAD) {
+ mbox.s[0] = pd_id;
+ mbox.s[1] = pd->mpd_pd.mfp_seq;
+ mbox.b[4] = MFI_PD_UNCONFIG_GOOD;
+ rv = mfi_mgmt(sc, MR_DCMD_PD_SET_STATE, MFI_DATA_NONE, 0,
+ NULL, &mbox);
+ if (rv != 0)
+ goto done;
+ }
+
+ memset(&mbox, 0, sizeof mbox);
+ mbox.s[0] = pd_id;
+ rv = mfi_mgmt(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN, sizeof(*pd), pd,
+ &mbox);
+ if (rv != 0)
+ goto done;
+
+ if (pd->mpd_ddf_state & MFI_DDF_FOREIGN) {
+ rv = mfi_mgmt(sc, MR_DCMD_CFG_FOREIGN_SCAN, MFI_DATA_IN,
+ sizeof(*fsi), fsi, NULL);
+ if (rv != 0)
+ goto done;
+
+ if (fsi->count > 0) {
+ rv = mfi_mgmt(sc, MR_DCMD_CFG_FOREIGN_CLEAR,
+ MFI_DATA_NONE, 0, NULL, NULL);
+ if (rv != 0)
+ goto done;
+ }
+ }
+
+ memset(&mbox, 0, sizeof mbox);
+ mbox.s[0] = pd_id;
+ rv = mfi_mgmt(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN, sizeof *pd, pd,
+ &mbox);
+ if (rv != 0)
+ goto done;
+
+ if (pd->mpd_fw_state != MFI_PD_UNCONFIG_GOOD ||
+ pd->mpd_ddf_state & MFI_DDF_FOREIGN)
+ rv = ENXIO;
+
+done:
+ free(fsi, M_DEVBUF, sizeof *fsi);
+ free(pd, M_DEVBUF, sizeof *pd);
+
+ return (rv);
+}
+
+static int
+mfi_makespare(struct mfi_softc *sc, uint16_t pd_id)
+{
+ struct mfi_hotspare *hs;
+ struct mfi_pd_details *pd;
+ union mfi_mbox mbox;
+ size_t size;
+ int rv = EINVAL;
+
+ /* we really could skip and expect that inq took care of it */
+ if (mfi_bio_getitall(sc)) {
+ DNPRINTF(MFI_D_IOCTL, "%s: mfi_bio_getitall failed\n",
+ DEVNAME(sc));
+ return (rv);
+ }
+ size = sizeof *hs + sizeof(uint16_t) * sc->sc_cfg->mfc_no_array;
+
+ hs = malloc(size, M_DEVBUF, M_WAITOK);
+ pd = malloc(sizeof *pd, M_DEVBUF, M_WAITOK);
+
+ memset(&mbox, 0, sizeof mbox);
+ mbox.s[0] = pd_id;
+ rv = mfi_mgmt(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN, sizeof *pd, pd,
+ &mbox);
+ if (rv != 0)
+ goto done;
+
+ memset(hs, 0, size);
+ hs->mhs_pd.mfp_id = pd->mpd_pd.mfp_id;
+ hs->mhs_pd.mfp_seq = pd->mpd_pd.mfp_seq;
+ rv = mfi_mgmt(sc, MR_DCMD_CFG_MAKE_SPARE, MFI_DATA_OUT, size, hs, NULL);
+
+done:
+ free(hs, M_DEVBUF, size);
+ free(pd, M_DEVBUF, sizeof *pd);
+
+ return (rv);
+}
+
int
mfi_ioctl_setstate(struct mfi_softc *sc, struct bioc_setstate *bs)
{
- struct mfi_pd_list *pd;
- struct mfi_pd_details *info;
+ struct mfi_pd_details *pd;
+ struct mfi_pd_list *pl;
int i, found, rv = EINVAL;
union mfi_mbox mbox;
DNPRINTF(MFI_D_IOCTL, "%s: mfi_ioctl_setstate %x\n", DEVNAME(sc),
bs->bs_status);
- pd = malloc(sizeof(*pd), M_DEVBUF, M_WAITOK);
- info = malloc(sizeof *info, M_DEVBUF, M_WAITOK);
+ pd = malloc(sizeof *pd, M_DEVBUF, M_WAITOK);
+ pl = malloc(sizeof *pl, M_DEVBUF, M_WAITOK);
if (mfi_mgmt(sc, MR_DCMD_PD_GET_LIST, MFI_DATA_IN,
- sizeof(*pd), pd, NULL))
+ sizeof *pl, pl, NULL))
goto done;
- for (i = 0, found = 0; i < pd->mpl_no_pd; i++)
- if (bs->bs_channel == pd->mpl_address[i].mpa_enc_index &&
- bs->bs_target == pd->mpl_address[i].mpa_enc_slot) {
+ for (i = 0, found = 0; i < pl->mpl_no_pd; i++)
+ if (bs->bs_channel == pl->mpl_address[i].mpa_enc_index &&
+ bs->bs_target == pl->mpl_address[i].mpa_enc_slot) {
found = 1;
break;
}
@@ -2037,14 +2183,14 @@ mfi_ioctl_setstate(struct mfi_softc *sc, struct
bioc_setstate *bs)
goto done;
memset(&mbox, 0, sizeof(mbox));
- mbox.s[0] = pd->mpl_address[i].mpa_pd_id;
+ mbox.s[0] = pl->mpl_address[i].mpa_pd_id;
if (mfi_mgmt(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN,
- sizeof *info, info, &mbox))
+ sizeof *pd, pd, &mbox))
goto done;
- mbox.s[0] = pd->mpl_address[i].mpa_pd_id;
- mbox.s[1] = info->mpd_pd.mfp_seq;
+ mbox.s[0] = pl->mpl_address[i].mpa_pd_id;
+ mbox.s[1] = pd->mpd_pd.mfp_seq;
switch (bs->bs_status) {
case BIOC_SSONLINE:
@@ -2060,6 +2206,31 @@ mfi_ioctl_setstate(struct mfi_softc *sc, struct
bioc_setstate *bs)
break;
case BIOC_SSREBUILD:
+ if (pd->mpd_fw_state != MFI_PD_OFFLINE) {
+ if ((rv = mfi_makegood(sc,
+ pl->mpl_address[i].mpa_pd_id)))
+ goto done;
+
+ if ((rv = mfi_makespare(sc,
+ pl->mpl_address[i].mpa_pd_id)))
+ goto done;
+
+ memset(&mbox, 0, sizeof(mbox));
+ mbox.s[0] = pl->mpl_address[i].mpa_pd_id;
+ rv = mfi_mgmt(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN,
+ sizeof *pd, pd, &mbox);
+ if (rv != 0)
+ goto done;
+
+ /* rebuilding might be started by mfi_makespare() */
+ if (pd->mpd_fw_state == MFI_PD_REBUILD) {
+ rv = 0;
+ goto done;
+ }
+
+ mbox.s[0] = pl->mpl_address[i].mpa_pd_id;
+ mbox.s[1] = pd->mpd_pd.mfp_seq;
+ }
mbox.b[4] = MFI_PD_REBUILD;
break;
@@ -2070,14 +2241,10 @@ mfi_ioctl_setstate(struct mfi_softc *sc, struct
bioc_setstate *bs)
}
- if ((rv = mfi_mgmt(sc, MR_DCMD_PD_SET_STATE, MFI_DATA_NONE, 0, NULL,
- &mbox)))
- goto done;
-
- rv = 0;
+ rv = mfi_mgmt(sc, MR_DCMD_PD_SET_STATE, MFI_DATA_NONE, 0, NULL, &mbox);
done:
free(pd, M_DEVBUF, sizeof *pd);
- free(info, M_DEVBUF, sizeof *info);
+ free(pl, M_DEVBUF, sizeof *pl);
return (rv);
}
@@ -2335,7 +2502,7 @@ static const char *mfi_bbu_indicators[] = {
#define MFI_BBU_SENSORS 4
-int
+void
mfi_bbu(struct mfi_softc *sc)
{
struct mfi_bbu_status bbu;
@@ -2354,7 +2521,7 @@ mfi_bbu(struct mfi_softc *sc)
sc->sc_bbu_status[i].value = 0;
sc->sc_bbu_status[i].status = SENSOR_S_UNKNOWN;
}
- return (-1);
+ return;
}
switch (bbu.battery_type) {
@@ -2379,7 +2546,7 @@ mfi_bbu(struct mfi_softc *sc)
sc->sc_bbu_status[i].value = 0;
sc->sc_bbu_status[i].status = SENSOR_S_UNKNOWN;
}
- return (0);
+ return;
}
status = letoh32(bbu.fw_status);
@@ -2398,8 +2565,6 @@ mfi_bbu(struct mfi_softc *sc)
sc->sc_bbu_status[i].value = (status & (1 << i)) ? 1 : 0;
sc->sc_bbu_status[i].status = SENSOR_S_UNSPEC;
}
-
- return (0);
}
int
@@ -2412,7 +2577,7 @@ mfi_create_sensors(struct mfi_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);
@@ -2455,17 +2620,21 @@ mfi_create_sensors(struct mfi_softc *sc)
for (i = 0; i < sc->sc_ld_cnt; i++) {
link = scsi_get_link(sc->sc_scsibus, i, 0);
- if (link == NULL)
- goto bad;
- dev = link->device_softc;
+ if (link == NULL) {
+ strlcpy(sc->sc_sensors[i].desc, "cache",
+ sizeof(sc->sc_sensors[i].desc));
+ } else {
+ dev = link->device_softc;
+ if (dev == NULL)
+ continue;
+
+ strlcpy(sc->sc_sensors[i].desc, dev->dv_xname,
+ sizeof(sc->sc_sensors[i].desc));
+ }
sc->sc_sensors[i].type = SENSOR_DRIVE;
sc->sc_sensors[i].status = SENSOR_S_UNKNOWN;
-
- strlcpy(sc->sc_sensors[i].desc, dev->dv_xname,
- sizeof(sc->sc_sensors[i].desc));
-
sensor_attach(&sc->sc_sensordev, &sc->sc_sensors[i]);
}
@@ -2490,8 +2659,8 @@ mfi_refresh_sensors(void *arg)
int i, rv;
struct bioc_vol bv;
- if (sc->sc_bbu != NULL && mfi_bbu(sc) != 0)
- return;
+ if (sc->sc_bbu != NULL)
+ mfi_bbu(sc);
for (i = 0; i < sc->sc_ld_cnt; i++) {
bzero(&bv, sizeof(bv));
@@ -2501,8 +2670,11 @@ mfi_refresh_sensors(void *arg)
rv = mfi_ioctl_vol(sc, &bv);
rw_exit_write(&sc->sc_lock);
- if (rv != 0)
- return;
+ if (rv != 0) {
+ sc->sc_sensors[i].value = 0; /* unknown */
+ sc->sc_sensors[i].status = SENSOR_S_UNKNOWN;
+ continue;
+ }
switch(bv.bv_status) {
case BIOC_SVOFFLINE:
--
2.7.4
