Overall, looks good. Just a few minor style nits.
Thanks,
Brian
On 11/18/2015 10:05 AM, Gabriel Krisman Bertazi wrote:
> Sorry, I screwed up when sending the lastest version of this patch.
> Forgot to add the v2. Resending...
>
> -- >8 --
>
> New ssd-report command allow users to inquiry some info on their
> read-intensive SSDs, like life expectancy and usage stats, so they can
> know when their SSD will wear out.
>
> Changes since v1:
> - Comply to the latest SAS Spec.
> - Fix negative life remaining values.
> - Trigger PFA Trip if other parameters exist in the log page.
> - Implement common interface in iprlib to search specific parameters in
> a log page.
>
> Signed-off-by: Gabriel Krisman Bertazi <kris...@linux.vnet.ibm.com>
> ---
> iprconfig.8 | 4 ++
> iprconfig.c | 165
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> iprlib.c | 45 +++++++++++++++++
> iprlib.h | 131 +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 345 insertions(+)
>
> diff --git a/iprconfig.8 b/iprconfig.8
> index 9f25902..6c8c3dd 100644
> --- a/iprconfig.8
> +++ b/iprconfig.8
> @@ -337,6 +337,10 @@ Show the microcode level that is currently loaded on the
> specified device.
> Note: The device specified may be the sg device associated with an IOA,
> in which case the IOA's microcode level will be shown.
> .TP
> +.B ssd-report [device]
> +.br
> +Display information about Read Intensive SSD devices in the system.
> +.TP
> .B show-ucode-levels
> .br
> Show the microcode level that is currently loaded for every device and
> diff --git a/iprconfig.c b/iprconfig.c
> index c4499a6..af055da 100644
> --- a/iprconfig.c
> +++ b/iprconfig.c
> @@ -18708,6 +18708,170 @@ static int dump (char **args, int num_args)
> return 0;
> }
>
> +static char *print_ssd_report(struct ipr_dev *dev, char *body)
> +{
> + struct ipr_inquiry_page0 page0_inq;
> + struct ipr_dasd_inquiry_page3 page3_inq;
> + struct ipr_sas_std_inq_data std_inq;
> + struct ipr_sas_inquiry_pageC4 pageC4_inq;
> + struct ipr_sas_inquiry_pageC7 pageC7_inq;
> + struct ipr_sas_log_page page34_log;
> + struct ipr_sas_log_page page2F_log;
> + struct ipr_sas_log_page page02_log;
> +
> + struct ipr_sas_log_smart_attr *smart_attr;
> + struct ipr_sas_log_inf_except_attr *inf_except_attr;
> + struct ipr_sas_log_write_err_cnt_attr *bytes_counter;
> +
> + char buffer[BUFSIZ];
> + int rc, len, nentries = 0;
> + uint64_t aux;
> + uint32_t uptime = 0;
> + uint32_t life_remain = 0;
> + uint64_t total_gb_writ = 0;
Can we change these to use u32 / u64 types to be consistent?
> + int pfa_trip = 0;
> +
> + memset(&std_inq, 0, sizeof(std_inq));
> + memset(&pageC4_inq, 0, sizeof(pageC4_inq));
> + memset(&pageC7_inq, 0, sizeof(pageC7_inq));
> + memset(&page0_inq, 0, sizeof(page0_inq));
> + memset(&page3_inq, 0, sizeof(page3_inq));
> + memset(&page34_log, 0, sizeof(page34_log));
> + memset(&page2F_log, 0, sizeof(page2F_log));
> + memset(&page02_log, 0, sizeof(page02_log));
> +
> + rc = ipr_inquiry(dev, IPR_STD_INQUIRY, &std_inq, sizeof(std_inq));
> + if (rc)
> + return NULL;
> +
> + if (!std_inq.is_ssd) {
> + scsi_err(dev, "%s is not a SSD.\n", dev->gen_name);
> + return NULL;
> + }
> +
> + rc = ipr_inquiry(dev, 0xC4, &pageC4_inq, sizeof(pageC4_inq));
> + if (rc)
> + return NULL;
> +
> + if (pageC4_inq.endurance != IPR_SAS_ENDURANCE_LOW_EZ &&
> + pageC4_inq.endurance != IPR_SAS_ENDURANCE_LOW3 &&
> + pageC4_inq.endurance != IPR_SAS_ENDURANCE_LOW1) {
> + scsi_err(dev, "%s is not a Read Intensive SSD.\n",
> + dev->gen_name);
> + return NULL;
> + }
> +
> + rc = ipr_inquiry(dev, 0x3, &page3_inq, sizeof(page3_inq));
> + if (rc)
> + return NULL;
> +
> + rc = ipr_inquiry(dev, 0xC7, &pageC7_inq, sizeof(pageC7_inq));
> + if (rc)
> + return NULL;
> +
> + rc = ipr_log_sense(dev, 0x34, &page34_log, sizeof(page34_log));
> + if (rc)
> + return NULL;
> +
> + smart_attr = ipr_sas_log_get_param(&page34_log, 0xE7, NULL);
> + if (smart_attr && smart_attr->norm_worst_val > 0)
> + life_remain = smart_attr->norm_worst_val;
> +
> + smart_attr = ipr_sas_log_get_param(&page34_log, 0x09, NULL);
> + if (smart_attr)
> + uptime = ntohl(*((uint32_t *) smart_attr->raw_data));
> +
> + rc = ipr_log_sense(dev, 0x02, &page02_log, sizeof(page02_log));
> + if (rc)
> + return NULL;
> +
> + bytes_counter = ipr_sas_log_get_param(&page02_log, 0x05, NULL);
> + if (bytes_counter)
> + total_gb_writ = be64toh(*(uint64_t*)
> + &bytes_counter->counter) >> 14;
> +
> +
> + rc = ipr_log_sense(dev, 0x2F, &page2F_log, sizeof(page2F_log));
> + if (rc)
> + return NULL;
> +
> + inf_except_attr = ipr_sas_log_get_param(&page2F_log, 0x0, &nentries);
> + if (inf_except_attr)
> + pfa_trip = inf_except_attr->inf_except_add_sense_code;
> + if (nentries > 1)
> + pfa_trip = 1;
> +
> + body = add_line_to_body(body, "", NULL);
> + /* FRU Number */
> + ipr_strncpy_0(buffer, std_inq.asm_part_num,
> + IPR_SAS_STD_INQ_ASM_PART_NUM);
> + body = add_line_to_body(body, _("FRU Number"), buffer);
> +
> + /* Serial number */
> + ipr_strncpy_0(buffer, (char *)std_inq.std_inq_data.serial_num,
> + IPR_SERIAL_NUM_LEN);
> + body = add_line_to_body(body, _("Serial Number"), buffer);
> +
> + /* FW level */
> + len = sprintf(buffer, "%X%X%X%X", page3_inq.release_level[0],
> + page3_inq.release_level[1], page3_inq.release_level[2],
> + page3_inq.release_level[3]);
> +
> + if (isalnum(page3_inq.release_level[0]) &&
> + isalnum(page3_inq.release_level[1]) &&
> + isalnum(page3_inq.release_level[2]) &&
> + isalnum(page3_inq.release_level[3]))
> + sprintf(buffer + len, " (%c%c%c%c)", page3_inq.release_level[0],
> + page3_inq.release_level[1], page3_inq.release_level[2],
> + page3_inq.release_level[3]);
> +
> + body = add_line_to_body(body, _("Firmware Version"), buffer);
> +
> + /* Bytes written */
> + snprintf(buffer, BUFSIZ, "%ld GB", total_gb_writ);
> + body = add_line_to_body(body, _("Total Bytes Written"), buffer);
> +
> + /* Max bytes. */
> + aux = ntohl(*((uint32_t *) pageC7_inq.total_bytes_warranty)) >> 8;
> + snprintf(buffer, BUFSIZ, "%ld GB", aux * 1024L);
> + body = add_line_to_body(body, _("Number of Bytes reported by Warranty"),
> + buffer);
> +
> + /* Life remaining */
> + snprintf(buffer, BUFSIZ, "%hhd %%", life_remain);
> + body = add_line_to_body(body, _("Life Remaining Gauge"), buffer);
> +
> + /* PFA Trip */
> + body = add_line_to_body(body, _("PFA Trip"), pfa_trip? "yes": "no");
> +
> + /* Power-on days */
> + snprintf (buffer, 4, "%d", uptime / 24);
> + body = add_line_to_body(body, _("Power-on Days"), buffer);
> +
> + return body;
> +}
> +
> +static int ssd_report(char **args, int num_args)
> +{
> + struct ipr_dev *dev = find_dev(args[0]);
> + char *body;
> +
> + if (!dev) {
> + fprintf(stderr, "Cannot find %s\n", args[0]);
> + return -EINVAL;
> + }
> +
> + body = print_ssd_report(dev, NULL);
> + if (!body) {
> + scsi_err(dev, "Device inquiry failed.\n");
> + return 1;
Would -EIO be better here perhaps?
> + }
> +
> + printf("%s", body);
> +
> + return 0;
> +}
> +
> static const struct {
> char *cmd;
> int min_args;
> @@ -18811,6 +18975,7 @@ static const struct {
> { "set-log-level", 2, 0, 2, set_log_level_cmd,
> "sg5 2" },
> { "set-write-cache-policy", 2, 0, 2,
> set_device_write_cache_policy, "sg5 [writeback|writethrough]" },
> { "query-write-cache-policy", 1, 0, 1,
> query_device_write_cache_policy, "sg5" },
> + { "ssd-report", 1, 0, 1, ssd_report, "sg5" },
> { "identify-disk", 2, 0, 2, identify_disk, "sda 1"
> },
> { "identify-slot", 2, 0, 2, identify_slot,
> "U5886.001.P915059-P1-D1 1" },
> { "remove-disk", 2, 0, 2, remove_disk, "sda 1" },
> diff --git a/iprlib.c b/iprlib.c
> index 6e5a487..b0ff857 100644
> --- a/iprlib.c
> +++ b/iprlib.c
> @@ -3038,6 +3038,51 @@ int ipr_is_log_page_supported(struct ipr_dev *dev, u8
> page)
> return 0;
> }
>
> +/** ipr_sas_log_get_param - Fetch parameter from log page.
> + *
> + * Iterate over an already fetched log page pointed by page and return a
> + * pointer to the beginning of the parameter.
> + *
> + * @page: An already fetched log page.
> + * @param: to be fetched
> + *
> + * Returns: @dst: A pointer to the original page area starting at the
> + * parameter. NULL if parameter was not found.
> + * @entries_cnt: output parameter, the number of entries found in the
> + * page.
> + **/
> +void *ipr_sas_log_get_param(const struct ipr_sas_log_page *page,
> + uint32_t param_code, int *entries_cntr)
Can you change this uint32_t to a u32 to be consistent with the rest of the
code?
> +{
> + int page_length;
> + int i;
> + const u8 *raw_data = page->raw_data;
> + uint32_t cur_code;
> + struct log_parameter_hdr *hdr;
> + void *ret = NULL;
> +
> + if (entries_cntr)
> + *entries_cntr = 0;
> +
> + page_length = (page->page_length[0] << 8) | page->page_length[1];
> +
> + for(i = 0; i < page_length && i < IPR_SAS_LOG_MAX_ENTRIES;
> + i += hdr->length + sizeof(*hdr)) {
> + hdr = (struct log_parameter_hdr *) &raw_data[i];
> + cur_code = (hdr->parameter_code[0] << 8) |
> hdr->parameter_code[1];
> +
> + if (cur_code == param_code) {
> + ret = hdr;
> + if (!entries_cntr)
> + break;
> + }
> +
> + if (entries_cntr)
> + *entries_cntr += 1;
> + }
> + return ret;
> +}
> +
> /**
> * ipr_get_blk_size - return the block size for the given device
> * @dev: ipr dev struct
> diff --git a/iprlib.h b/iprlib.h
> index cf6bf79..8d6c363 100644
> --- a/iprlib.h
> +++ b/iprlib.h
> @@ -236,6 +236,18 @@ typedef uint64_t u64;
> #define IPR_VSET_VIRTUAL_BUS 0x2
> #define IPR_IOAFP_VIRTUAL_BUS 0x3
>
> +#define IPR_SAS_STD_INQ_UCODE_ID 12
> +#define IPR_SAS_STD_INQ_VENDOR_UNIQ 40
> +#define IPR_SAS_STD_INQ_PLANT_MAN 4
> +#define IPR_SAS_STD_INQ_DATE_MAN 5
> +#define IPR_SAS_STD_INQ_FRU_COUNT 4
> +#define IPR_SAS_STD_INQ_FRU_FIELD_LEN 2
> +#define IPR_SAS_STD_INQ_FRU_PN 12
> +#define IPR_SAS_STD_INQ_ASM_EC_LVL 10
> +#define IPR_SAS_STD_INQ_ASM_PART_NUM 12
> +#define IPR_SAS_STD_INQ_FRU_ASM_EC 10
> +#define IPR_SAS_INQ_BYTES_WARRANTY_LEN 3
> +
> /* Device write cache policies. */
> enum {IPR_DEV_CACHE_WRITE_THROUGH = 0, IPR_DEV_CACHE_WRITE_BACK};
>
> @@ -1202,6 +1214,36 @@ struct ipr_std_inq_data_long {
> u8 z6_term[IPR_STD_INQ_Z6_TERM_LEN];
> };
>
> +struct ipr_sas_std_inq_data {
> + struct ipr_std_inq_data std_inq_data;
> + u8 microcode_id[IPR_SAS_STD_INQ_UCODE_ID];
> + u8 reserved1;
> + u8 vendor_unique[IPR_SAS_STD_INQ_VENDOR_UNIQ];
> +
> +#if defined (__BIG_ENDIAN_BITFIELD)
> + u8 reserved2:5;
> + u8 is_ssd:1;
> + u8 near_line:1;
> + u8 unlock:1;
> +#elif defined (__LITTLE_ENDIAN_BITFIELD)
> + u8 unlock:1;
> + u8 near_line:1;
> + u8 is_ssd:1;
> + u8 reserved2:5;
> +#endif
> +
> + u8 plant_manufacture[IPR_SAS_STD_INQ_PLANT_MAN];
> + u8 date_manufacture[IPR_SAS_STD_INQ_DATE_MAN];
> + u8 vendor_unique_pn;
> + u8 fru_count[IPR_SAS_STD_INQ_FRU_COUNT];
> + u8 fru_field_len[IPR_SAS_STD_INQ_FRU_FIELD_LEN];
> + u8 fru_pn[IPR_SAS_STD_INQ_FRU_PN];
> + u8 asm_ec_level[IPR_SAS_STD_INQ_ASM_EC_LVL];
> + u8 asm_part_num[IPR_SAS_STD_INQ_ASM_PART_NUM];
> + u8 fru_asm_ec[IPR_SAS_STD_INQ_FRU_ASM_EC];
> + u8 reserved3[6];
> +};
> +
> struct ipr_mode_page_28_scsi_dev_bus_attr {
> struct ipr_res_addr res_addr;
>
> @@ -2216,6 +2258,65 @@ struct ipr_global_cache_params_term {
> u8 reserved3;
> };
>
> +struct log_parameter_hdr {
> + u8 parameter_code[2];
> +
> +#if defined (__BIG_ENDIAN_BITFIELD)
> + u8 du:1;
> + u8 reserved1:1;
> + u8 tsd:1;
> + u8 etc:1;
> + u8 tmc:2;
> + u8 lbin:1;
> + u8 lp:1;
> +#elif defined (__LITTLE_ENDIAN_BITFIELD)
> + u8 lp:1;
> + u8 lbin:1;
> + u8 tmc:2;
> + u8 etc:1;
> + u8 tsd:1;
> + u8 reserved1:1;
> + u8 du:1;
> +#endif
> + u8 length;
> +};
> +
> +/* Log Parameter: Log Page = 0x34. Attribute template */
> +struct ipr_sas_log_smart_attr {
> + struct log_parameter_hdr hdr;
> + u8 norm_threshold_val;
> + int8_t norm_worst_val;
> + u8 raw_data[10];
> +};
> +
> +/* Log Parameter: Log Page = 0x2F. Attribute Code = 0x0 */
> +struct ipr_sas_log_inf_except_attr {
> + struct log_parameter_hdr param_hdr;
> + u8 inf_except_add_sense_code;
> + u8 inf_except_add_sense_code_qual;
> + u8 last_temp_read;
> +};
> +
> +struct ipr_sas_log_write_err_cnt_attr {
> + struct log_parameter_hdr param_hdr;
> + u8 counter;
> +};
> +
> +struct ipr_sas_log_page {
> +#if defined (__BIG_ENDIAN_BITFIELD)
> + u8 reserved1:2;
> + u8 page_code:6;
> +#elif defined (__LITTLE_ENDIAN_BITFIELD)
> + u8 page_code:6;
> + u8 reserved1:2;
> +#endif
> + u8 reserved2;
> + u8 page_length[2];
> +
> +#define IPR_SAS_LOG_MAX_ENTRIES 256
> + u8 raw_data[IPR_SAS_LOG_MAX_ENTRIES];
> +};
> +
> struct ipr_query_ioa_caching_info {
> u16 len;
> u8 reserved[2];
> @@ -2540,6 +2641,34 @@ struct ipr_ses_inquiry_pageC3 {
> u8 reserved2;
> };
>
> +struct ipr_sas_inquiry_pageC4 {
> + u8 perif_type;
Can you rename this to peri_qual_dev_type to be similar to the other pages?
> + u8 page_code;
> + u8 reserved1;
> + u8 page_length;
> +#define IPR_SAS_ENDURANCE_HIGH_HDD 0x20
> +#define IPR_SAS_ENDURANCE_LOW_EZ 0x31
> +#define IPR_SAS_ENDURANCE_LOW3 0x32
> +#define IPR_SAS_ENDURANCE_LOW1 0x33
> + u8 endurance;
> + u8 reserved[6];
> + u8 revision;
> + u8 serial_num_11s[8];
> + u8 serial_num_supplier[8];
> + u8 master_drive_part[12];
> +};
> +
> +struct ipr_sas_inquiry_pageC7 {
> + u8 perif_type;
Can you rename this to peri_qual_dev_type to be similar to the other pages?
> + u8 page_code;
> + u8 reserved1;
> + u8 page_length;
> + u8 ascii_len;
> + u8 reserved2[109];
> + u8 total_bytes_warranty[IPR_SAS_INQ_BYTES_WARRANTY_LEN];
> + u8 reserved3[43];
> +};
> +
> static inline int ipr_elem_offset(struct ipr_ses_config_pg *ses_cfg, u8 type)
> {
> int i, off;
> @@ -2646,6 +2775,8 @@ int ipr_mode_sense(struct ipr_dev *, u8, void *);
> int ipr_mode_select(struct ipr_dev *, void *, int);
> int ipr_log_sense(struct ipr_dev *, u8, void *, u16);
> int ipr_is_log_page_supported(struct ipr_dev *, u8);
> +void *ipr_sas_log_get_param(const struct ipr_sas_log_page *page, uint32_t
> param,
> + int *entries_cnt);
> int ipr_reset_device(struct ipr_dev *);
> int ipr_re_read_partition(struct ipr_dev *);
> int ipr_read_capacity(struct ipr_dev *, void *);
>
--
Brian King
Power Linux I/O
IBM Linux Technology Center
------------------------------------------------------------------------------
_______________________________________________
Iprdd-devel mailing list
Iprdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iprdd-devel
