Date: Thu, 07 Mar 2013 16:07:05 +0000 From: Sean Bruno <seanwbruno@gmail.com> To: Mark Johnston <markj@freebsd.org> Cc: freebsd-scsi@freebsd.org Subject: Re: adding BBU relearn support to mfiutil Message-ID: <1362672425.84238.14.camel@localhost> In-Reply-To: <20130304033836.GA33631@oddish> References: <20130304033836.GA33631@oddish>
next in thread | previous in thread | raw e-mail | index | archive | help
Hey, I saw this. :-)
Trying to get through the pile of things to take a look at it.
Sean
On Sun, 2013-03-03 at 22:38 -0500, Mark Johnston wrote:
> Hi Everyone,
>
> I recently needed to add a couple of features to mfiutil related to BBU
> relearning. I've pasted a patch below which
>
> 1. adds extra fields to the output of "mfiutil show battery" showing BBU
> properties. This is essentially the output of
>
> # MegaCli -AdpBbuInfo -GetBbuProperties -aLL
>
> and consists of info about battery learning: the learn period, the
> time at which the controller will start the next relearn, and the BBU
> mode (which indicates whether the battery supports transparent
> relearning).
>
> 2. adds a couple of subcommands under "mfiutil bbu" which lets users set
> the BBU properties which can be set by MegaCli.
>
> 3. adds a command "mfiutil start learn" which immediately kicks off a
> battery relearn.
>
> These changes grew out of concern about the fact that the controller
> write cache is set to write-through mode during a relearn period (which
> usually lasts for several hours). This ended up causing some mysterious
> and intermittent performance issues, so I needed a way of getting more
> info about what was going on (using MegaCli isn't really an option for
> several reasons). Some BBUs support transparent relearning, which
> basically means that the controller write cache doesn't get turned off
> during a relearn. However, LSI's default config doesn't enable it, and
> now mfiutil can be used to do that (through "mfiutil bbu bbu-mode").
>
> I was hoping someone would be able to review the patch. If anyone's able
> and willing to test it, I'd very much appreciate feedback from that.
>
> Thanks!
> -Mark
>
> diff --git a/sys/dev/mfi/mfi_debug.c b/sys/dev/mfi/mfi_debug.c
> index 4aec4f7..be42ec0 100644
> --- a/sys/dev/mfi/mfi_debug.c
> +++ b/sys/dev/mfi/mfi_debug.c
> @@ -168,6 +168,15 @@ mfi_print_dcmd(struct mfi_softc *sc, device_t dev, struct mfi_command *cm)
> case MFI_DCMD_LD_MAP_GET_INFO:
> opcode = "LD_MAP_GET_INFO";
> break;
> + case MFI_DCMD_BBU_START_LEARN:
> + opcode = "BBU_START_LEARN";
> + break;
> + case MFI_DCMD_BBU_GET_PROP:
> + opcode = "BBU_GET_PROP";
> + break;
> + case MFI_DCMD_BBU_SET_PROP:
> + opcode = "BBU_SET_PROP";
> + break;
> default:
> opcode = "UNKNOWN";
> break;
> diff --git a/sys/dev/mfi/mfireg.h b/sys/dev/mfi/mfireg.h
> index 52ddafe..8f88f10 100644
> --- a/sys/dev/mfi/mfireg.h
> +++ b/sys/dev/mfi/mfireg.h
> @@ -234,6 +234,9 @@ typedef enum {
> MFI_DCMD_BBU_GET_STATUS = 0x05010000,
> MFI_DCMD_BBU_GET_CAPACITY_INFO =0x05020000,
> MFI_DCMD_BBU_GET_DESIGN_INFO = 0x05030000,
> + MFI_DCMD_BBU_START_LEARN = 0x05040000,
> + MFI_DCMD_BBU_GET_PROP = 0x05050100,
> + MFI_DCMD_BBU_SET_PROP = 0x05050200,
> MFI_DCMD_CLUSTER = 0x08000000,
> MFI_DCMD_CLUSTER_RESET_ALL = 0x08010100,
> MFI_DCMD_CLUSTER_RESET_LD = 0x08010200
> @@ -1367,6 +1370,15 @@ struct mfi_bbu_state {
> uint8_t reserved[21];
> } __packed;
>
> +struct mfi_bbu_properties {
> + uint32_t auto_learn_period;
> + uint32_t next_learn_time;
> + uint8_t learn_delay_interval;
> + uint8_t auto_learn_mode;
> + uint8_t bbu_mode;
> + uint8_t reserved[21];
> +} __packed;
> +
> union mfi_bbu_status_detail {
> struct mfi_ibbu_state ibbu;
> struct mfi_bbu_state bbu;
> diff --git a/usr.sbin/mfiutil/Makefile b/usr.sbin/mfiutil/Makefile
> index e100358..c460b3f 100644
> --- a/usr.sbin/mfiutil/Makefile
> +++ b/usr.sbin/mfiutil/Makefile
> @@ -1,8 +1,8 @@
> # $FreeBSD$
> PROG= mfiutil
>
> -SRCS= mfiutil.c mfi_cmd.c mfi_config.c mfi_drive.c mfi_evt.c mfi_flash.c \
> - mfi_patrol.c mfi_show.c mfi_volume.c
> +SRCS= mfiutil.c mfi_bbu.c mfi_cmd.c mfi_config.c mfi_drive.c mfi_evt.c \
> + mfi_flash.c mfi_patrol.c mfi_show.c mfi_volume.c
> MAN8= mfiutil.8
>
> CFLAGS+= -fno-builtin-strftime
> diff --git a/usr.sbin/mfiutil/mfi_bbu.c b/usr.sbin/mfiutil/mfi_bbu.c
> new file mode 100644
> index 0000000..86a9722
> --- /dev/null
> +++ b/usr.sbin/mfiutil/mfi_bbu.c
> @@ -0,0 +1,252 @@
> +/*-
> + * Copyright (c) 2013 Sandvine Inc.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. The names of the authors may not be used to endorse or promote
> + * products derived from this software without specific prior written
> + * permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + *
> + * $FreeBSD$
> + */
> +
> +#include <sys/param.h>
> +#include <sys/errno.h>
> +#include <sys/stat.h>
> +#include <err.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <time.h>
> +#include <unistd.h>
> +#include "mfiutil.h"
> +
> +/* The autolearn period is given in seconds. */
> +void
> +mfi_autolearn_period(uint32_t period, char *buf, size_t sz)
> +{
> + unsigned int d, h;
> + char *tmp;
> +
> + d = period / (24 * 3600);
> + h = (period % (24 * 3600)) / 3600;
> +
> + tmp = buf;
> + if (d != 0) {
> + tmp += snprintf(buf, sz, "%u day%s", d, d == 1 ? "" : "s");
> + sz -= tmp - buf;
> + if (h != 0) {
> + tmp += snprintf(tmp, sz, ", ");
> + sz -= 2;
> + }
> + }
> + if (h != 0)
> + snprintf(tmp, sz, "%u hour%s", h, h == 1 ? "" : "s");
> +
> + if (d == 0 && h == 0)
> + snprintf(tmp, sz, "less than 1 hour");
> +}
> +
> +/* The time to the next relearn is given in seconds since 1/1/2000. */
> +void
> +mfi_next_learn_time(uint32_t time, char *buf, size_t sz)
> +{
> + time_t basetime;
> + struct tm tm;
> + size_t len;
> +
> + memset(&tm, 0, sizeof(tm));
> + tm.tm_year = 100;
> + basetime = timegm(&tm);
> + basetime += (time_t)time;
> + len = snprintf(buf, sz, "%s", ctime(&basetime));
> + if (len > 0)
> + /* Get rid of the newline added by ctime(3). */
> + buf[len - 1] = '\0';
> +}
> +
> +void
> +mfi_autolearn_mode(uint8_t mode, char *buf, size_t sz)
> +{
> +
> + switch (mode) {
> + case 0:
> + snprintf(buf, sz, "enabled");
> + break;
> + case 1:
> + snprintf(buf, sz, "disabled");
> + break;
> + case 2:
> + snprintf(buf, sz, "warn via event");
> + break;
> + default:
> + snprintf(buf, sz, "mode 0x%02x", mode);
> + break;
> + }
> +}
> +
> +int
> +mfi_bbu_get_props(int fd, struct mfi_bbu_properties *props, uint8_t *statusp)
> +{
> +
> + return (mfi_dcmd_command(fd, MFI_DCMD_BBU_GET_PROP, props,
> + sizeof(*props), NULL, 0, statusp));
> +}
> +
> +int
> +mfi_bbu_set_props(int fd, struct mfi_bbu_properties *props, uint8_t *statusp)
> +{
> +
> + return (mfi_dcmd_command(fd, MFI_DCMD_BBU_SET_PROP, props,
> + sizeof(*props), NULL, 0, statusp));
> +}
> +
> +static int
> +start_bbu_learn(int ac, char **av __unused)
> +{
> + uint8_t status;
> + int error, fd;
> +
> + status = MFI_STAT_OK;
> + error = 0;
> +
> + if (ac != 1) {
> + warnx("start learn: unexpected arguments");
> + return (EINVAL);
> + }
> +
> + fd = mfi_open(mfi_unit, O_RDWR);
> + if (fd < 0) {
> + error = errno;
> + warn("mfi_open");
> + return (error);
> + }
> +
> + if (mfi_dcmd_command(fd, MFI_DCMD_BBU_START_LEARN, NULL, 0, NULL, 0,
> + &status) < 0) {
> + error = errno;
> + warn("Failed to start BBU learn");
> + } else if (status != MFI_STAT_OK) {
> + warnx("Failed to start BBU learn: %s", mfi_status(status));
> + error = EIO;
> + }
> +
> + return (error);
> +}
> +MFI_COMMAND(start, learn, start_bbu_learn);
> +
> +static int
> +update_bbu_props(int ac, char **av)
> +{
> + struct mfi_bbu_properties props;
> + unsigned long delay;
> + uint8_t status;
> + int error, fd;
> + char *mode, *endptr;
> +
> + status = MFI_STAT_OK;
> + error = 0;
> +
> + if (ac != 3) {
> + warnx("bbu: property and value required");
> + return (EINVAL);
> + }
> +
> + fd = mfi_open(mfi_unit, O_RDWR);
> + if (fd < 0) {
> + error = errno;
> + warn("mfi_open");
> + return (error);
> + }
> +
> + if (mfi_bbu_get_props(fd, &props, &status) < 0) {
> + error = errno;
> + warn("Failed to get BBU properties");
> + goto done;
> + } else if (status != MFI_STAT_OK) {
> + warnx("Failed to get BBU properties: %s", mfi_status(status));
> + error = EIO;
> + goto done;
> + }
> +
> + if (strcmp(av[1], "learn-delay") == 0) {
> + delay = strtoul(av[2], &endptr, 10);
> + if (strlen(av[2]) == 0 || *endptr != '\0' || delay > 255) {
> + warnx("Invalid learn delay '%s'", av[2]);
> + error = EINVAL;
> + goto done;
> + }
> +
> + props.learn_delay_interval = delay;
> + } else if (strcmp(av[1], "autolearn-mode") == 0) {
> + mode = av[2];
> +
> + if (strcmp(av[2], "enable") == 0)
> + props.auto_learn_mode = 0;
> + else if (strcmp(av[2], "disable") == 0)
> + props.auto_learn_mode = 1;
> + else if (mode[0] >= '0' && mode[0] <= '2' && mode[1] == '\0')
> + props.auto_learn_mode = mode[0] - '0';
> + else {
> + warnx("Invalid mode '%s'", mode);
> + error = EINVAL;
> + goto done;
> + }
> + } else if (strcmp(av[1], "bbu-mode") == 0) {
> + if (props.bbu_mode == 0) {
> + warnx("This BBU does not implement different modes");
> + error = EINVAL;
> + goto done;
> + }
> +
> + /* The mode must be an integer between 1 and 5. */
> + mode = av[2];
> + if (mode[0] < '1' || mode[0] > '5' || mode[1] != '\0') {
> + warnx("Invalid mode '%s'", mode);
> + error = EINVAL;
> + goto done;
> + }
> +
> + props.bbu_mode = mode[0] - '0';
> + } else {
> + warnx("bbu: Invalid command '%s'", av[1]);
> + error = EINVAL;
> + goto done;
> + }
> +
> + if (mfi_bbu_set_props(fd, &props, &status) < 0) {
> + error = errno;
> + warn("Failed to set BBU properties");
> + goto done;
> + } else if (status != MFI_STAT_OK) {
> + warnx("Failed to set BBU properties: %s", mfi_status(status));
> + error = EIO;
> + goto done;
> + }
> +
> +done:
> + close(fd);
> +
> + return (error);
> +}
> +MFI_COMMAND(top, bbu, update_bbu_props);
> diff --git a/usr.sbin/mfiutil/mfi_show.c b/usr.sbin/mfiutil/mfi_show.c
> index be395bb..06721c2 100644
> --- a/usr.sbin/mfiutil/mfi_show.c
> +++ b/usr.sbin/mfiutil/mfi_show.c
> @@ -140,9 +140,11 @@ show_battery(int ac, char **av __unused)
> {
> struct mfi_bbu_capacity_info cap;
> struct mfi_bbu_design_info design;
> + struct mfi_bbu_properties props;
> struct mfi_bbu_status stat;
> uint8_t status;
> - int comma, error, fd, show_capacity;
> + int comma, error, fd, show_capacity, show_props;
> + char buf[32];
>
> if (ac != 1) {
> warnx("show battery: extra arguments");
> @@ -186,6 +188,14 @@ show_battery(int ac, char **av __unused)
> return (error);
> }
>
> + if (mfi_bbu_get_props(fd, &props, &status) < 0) {
> + error = errno;
> + warn("Failed to get properties");
> + close(fd);
> + return (error);
> + }
> + show_props = (status == MFI_STAT_OK);
> +
> printf("mfi%d: Battery State:\n", mfi_unit);
> printf(" Manufacture Date: %d/%d/%d\n", design.mfg_date >> 5 & 0x0f,
> design.mfg_date & 0x1f, design.mfg_date >> 9 & 0xffff);
> @@ -205,6 +215,23 @@ show_battery(int ac, char **av __unused)
> printf(" Design Voltage: %d mV\n", design.design_voltage);
> printf(" Current Voltage: %d mV\n", stat.voltage);
> printf(" Temperature: %d C\n", stat.temperature);
> + if (show_props) {
> + mfi_autolearn_period(props.auto_learn_period, buf, sizeof(buf));
> + printf(" Autolearn period: %s\n", buf);
> + if (props.auto_learn_mode != 0)
> + snprintf(buf, sizeof(buf), "never");
> + else
> + mfi_next_learn_time(props.next_learn_time, buf,
> + sizeof(buf));
> + printf(" Next learn time: %s\n", buf);
> + printf(" Learn delay interval: %u hour%s\n",
> + props.learn_delay_interval,
> + props.learn_delay_interval != 1 ? "s" : "");
> + mfi_autolearn_mode(props.auto_learn_mode, buf, sizeof(buf));
> + printf(" Autolearn mode: %s\n", buf);
> + if (props.bbu_mode != 0)
> + printf(" BBU Mode: %d\n", props.bbu_mode);
> + }
> printf(" Status:");
> comma = 0;
> if (stat.fw_status & MFI_BBU_STATE_PACK_MISSING) {
> diff --git a/usr.sbin/mfiutil/mfi_volume.c b/usr.sbin/mfiutil/mfi_volume.c
> index 49417d0..2306256 100644
> --- a/usr.sbin/mfiutil/mfi_volume.c
> +++ b/usr.sbin/mfiutil/mfi_volume.c
> @@ -363,7 +363,8 @@ volume_cache(int ac, char **av)
> break;
> }
> if (props.default_cache_policy != props.current_cache_policy)
> - printf("Cache Disabled Due to Dead Battery\n");
> + printf(
> + "Cache disabled due to dead battery or ongoing battery relearn\n");
> error = 0;
> } else {
> new = props;
> diff --git a/usr.sbin/mfiutil/mfiutil.8 b/usr.sbin/mfiutil/mfiutil.8
> index b1be5d9..cb160fc 100644
> --- a/usr.sbin/mfiutil/mfiutil.8
> +++ b/usr.sbin/mfiutil/mfiutil.8
> @@ -141,6 +141,12 @@
> .Nm
> .Op Fl u Ar unit
> .Cm flash Ar file
> +.Nm
> +.Op Fl u Ar unit
> +.Cm start learn
> +.Nm
> +.Op Fl u Ar unit
> +.Cm bbu Ar setting Ar value
> .Sh DESCRIPTION
> The
> .Nm
> @@ -565,6 +571,32 @@ Stop a currently running patrol read operation.
> Updates the flash on the controller with the firmware stored in
> .Ar file .
> A reboot is required for the new firmware to take effect.
> +.It Cm start learn
> +Start a battery relearn.
> +.It Cm bbu Ar setting Ar value
> +Update battery backup unit (BBU) properties related to battery relearning.
> +The following settings are configurable:
> +.Bl -tag -width indent
> +.It Cm learn-delay
> +Add a delay to the next scheduled battery relearn event. This setting is
> +given in hours and must lie in the range of 0 to 255.
> +.It Cm autolearn-mode
> +Enable or disable automatic periodic battery relearning.
> +The setting may be set to
> +.Dq enable
> +or
> +.Dq disable
> +to respectively enable or disable the relearn cycle.
> +Alternatively, a mode of 0, 1 or 2 may be given.
> +Mode 0 enables periodic relearning, mode 1 disables it, and mode 2 disables
> +it and logs a warning to the event log when it detects that a battery relearn
> +should be performed.
> +.It Cm bbu-mode
> +Set the BBU's mode of operation. This setting is not supported by all BBUs.
> +Where it is supported, the possible values are the integers between 1 and 5
> +inclusive.
> +Modes 1, 2 and 3 enable a transparent learn cycle, whereas modes 4 and 5 do not.
> +The BBU's data retention time is greater when transparent learning is not used.
> .El
> .Sh EXAMPLES
> Configure the cache for volume mfid0 to cache only writes:
> diff --git a/usr.sbin/mfiutil/mfiutil.c b/usr.sbin/mfiutil/mfiutil.c
> index 79c6a4b..7488e24 100644
> --- a/usr.sbin/mfiutil/mfiutil.c
> +++ b/usr.sbin/mfiutil/mfiutil.c
> @@ -84,6 +84,8 @@ usage(void)
> fprintf(stderr, " start patrol - start a patrol read\n");
> fprintf(stderr, " stop patrol - stop a patrol read\n");
> fprintf(stderr, " flash <firmware>\n");
> + fprintf(stderr, " start learn - start a BBU relearn\n");
> + fprintf(stderr, " bbu <setting> <value> - set BBU properties\n");
> #ifdef DEBUG
> fprintf(stderr, " debug - debug 'show config'\n");
> fprintf(stderr, " dump - display 'saved' config\n");
> diff --git a/usr.sbin/mfiutil/mfiutil.h b/usr.sbin/mfiutil/mfiutil.h
> index 687bdd9..8a544c2 100644
> --- a/usr.sbin/mfiutil/mfiutil.h
> +++ b/usr.sbin/mfiutil/mfiutil.h
> @@ -152,6 +152,13 @@ int mfi_reconfig_supported(void);
> const char *mfi_status(u_int status_code);
> const char *mfi_drive_name(struct mfi_pd_info *pinfo, uint16_t device_id,
> uint32_t def);
> +int mfi_bbu_get_props(int fd, struct mfi_bbu_properties *props,
> + uint8_t *statusp);
> +int mfi_bbu_set_props(int fd, struct mfi_bbu_properties *props,
> + uint8_t *statusp);
> +void mfi_autolearn_period(uint32_t, char *, size_t);
> +void mfi_next_learn_time(uint32_t, char *, size_t);
> +void mfi_autolearn_mode(uint8_t, char *, size_t);
>
> void scan_firmware(struct mfi_info_component *comp);
> void display_firmware(struct mfi_info_component *comp, const char *tag);
> _______________________________________________
> freebsd-scsi@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-scsi
> To unsubscribe, send any mail to "freebsd-scsi-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1362672425.84238.14.camel>