Skip site navigation (1)Skip section navigation (2)
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: <http://docs.FreeBSD.org/cgi/mid.cgi?1362672425.84238.14.camel>