From 09762dcb623b5cff08c624250130cdcd389d5044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20H=C3=B6ppner?= Date: Mon, 21 Nov 2016 13:21:59 +0100 Subject: s390/dasd: Always store parameter elements in an array MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the DASD driver is built into the kernel, the entire comma separated parameter list is stored as one single element in the dasd[] array, opposed to the module build where each element is stored separately in dasd[]. There is no point in doing so. Therefore, store each part of the list as single elements in dasd[] as well when built into the kernel. Also, create a define for the maximum of 256 parameters. Reviewed-by: Stefan Haberland Reviewed-by: Sebastian Ott Signed-off-by: Jan Höppner Signed-off-by: Martin Schwidefsky --- drivers/s390/block/dasd_devmap.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'drivers/s390/block') diff --git a/drivers/s390/block/dasd_devmap.c b/drivers/s390/block/dasd_devmap.c index dd46e96a3034..4c8eff6b1aab 100644 --- a/drivers/s390/block/dasd_devmap.c +++ b/drivers/s390/block/dasd_devmap.c @@ -26,6 +26,7 @@ /* This is ugly... */ #define PRINTK_HEADER "dasd_devmap:" #define DASD_BUS_ID_SIZE 20 +#define DASD_MAX_PARAMS 256 #include "dasd_int.h" @@ -76,7 +77,7 @@ EXPORT_SYMBOL_GPL(dasd_nofcx); * it is named 'dasd' to directly be filled by insmod with the comma separated * strings when running as a module. */ -static char *dasd[256]; +static char *dasd[DASD_MAX_PARAMS]; module_param_array(dasd, charp, NULL, S_IRUGO); /* @@ -104,18 +105,19 @@ dasd_hash_busid(const char *bus_id) } #ifndef MODULE -/* - * The parameter parsing functions for builtin-drivers are called - * before kmalloc works. Store the pointers to the parameters strings - * into dasd[] for later processing. - */ -static int __init -dasd_call_setup(char *str) +static int __init dasd_call_setup(char *opt) { - static int count = 0; + static int i; + char *tmp; + + while (i < DASD_MAX_PARAMS) { + tmp = strsep(&opt, ","); + if (!tmp) + break; + + dasd[i++] = tmp; + } - if (count < 256) - dasd[count++] = str; return 1; } @@ -364,10 +366,8 @@ dasd_parse_next_element( char *parsestring ) { /* * Parse parameters stored in dasd[] * The 'dasd=...' parameter allows to specify a comma separated list of - * keywords and device ranges. When the dasd driver is build into the kernel, - * the complete list will be stored as one element of the dasd[] array. - * When the dasd driver is build as a module, then the list is broken into - * it's elements and each dasd[] entry contains one element. + * keywords and device ranges. The parameters in that list will be stored as + * separate elementes in dasd[]. */ int dasd_parse(void) @@ -376,7 +376,7 @@ dasd_parse(void) char *parsestring; rc = 0; - for (i = 0; i < 256; i++) { + for (i = 0; i < DASD_MAX_PARAMS; i++) { if (dasd[i] == NULL) break; parsestring = dasd[i]; -- cgit v1.2.3 From 3b1bea012710c1a299573c7a6a0584d623e6cbcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20H=C3=B6ppner?= Date: Tue, 22 Nov 2016 18:11:46 +0100 Subject: s390/dasd: Improve parameter list parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function dasd_busid() still uses simple_strtoul() to convert a string to an integer value. This function is obsolete for quite some time already and should be replaced. The whole parameter parsing semantic still relies somewhat on the fact, that simple_strtoul() parses a string containing literals without complains and just returns the parsed integer value plus the residual string. kstrtoint(), however, would return -EINVAL in such a case. Since we want to get rid of simple_strtoul() and now have a nice dasd[] containing only single elements, we can clean up and simplify a few things. Replace simple_strtoul() with kstrtouint(), improve and simplify the overall parameter parsing by the following: - instead of residual strings return proper error codes - remove dasd_parse_next_element() and decide directly what sort of element is being parsed - if we parse a device or a range of devices, split that element into separate bits with a new function - remove warning about invalid ending as it doesn't apply anymore - annotate all parsing functions and data that can be freed after initialisation with __init and __initdata respectively - clean up bits and pieces while at it Reviewed-by: Stefan Haberland Reviewed-by: Sebastian Ott Signed-off-by: Jan Höppner Signed-off-by: Martin Schwidefsky --- drivers/s390/block/dasd_devmap.c | 258 +++++++++++++++++++-------------------- drivers/s390/block/dasd_int.h | 2 +- 2 files changed, 125 insertions(+), 135 deletions(-) (limited to 'drivers/s390/block') diff --git a/drivers/s390/block/dasd_devmap.c b/drivers/s390/block/dasd_devmap.c index 4c8eff6b1aab..041bdc4429c1 100644 --- a/drivers/s390/block/dasd_devmap.c +++ b/drivers/s390/block/dasd_devmap.c @@ -107,7 +107,7 @@ dasd_hash_busid(const char *bus_id) #ifndef MODULE static int __init dasd_call_setup(char *opt) { - static int i; + static int i __initdata; char *tmp; while (i < DASD_MAX_PARAMS) { @@ -129,14 +129,13 @@ __setup ("dasd=", dasd_call_setup); /* * Read a device busid/devno from a string. */ -static int - -dasd_busid(char **str, int *id0, int *id1, int *devno) +static int __init dasd_busid(char *str, int *id0, int *id1, int *devno) { - int val, old_style; + unsigned int val; + char *tok; /* Interpret ipldev busid */ - if (strncmp(DASD_IPLDEV, *str, strlen(DASD_IPLDEV)) == 0) { + if (strncmp(DASD_IPLDEV, str, strlen(DASD_IPLDEV)) == 0) { if (ipl_info.type != IPL_TYPE_CCW) { pr_err("The IPL device is not a CCW device\n"); return -EINVAL; @@ -144,63 +143,50 @@ dasd_busid(char **str, int *id0, int *id1, int *devno) *id0 = 0; *id1 = ipl_info.data.ccw.dev_id.ssid; *devno = ipl_info.data.ccw.dev_id.devno; - *str += strlen(DASD_IPLDEV); return 0; } - /* check for leading '0x' */ - old_style = 0; - if ((*str)[0] == '0' && (*str)[1] == 'x') { - *str += 2; - old_style = 1; - } - if (!isxdigit((*str)[0])) /* We require at least one hex digit */ - return -EINVAL; - val = simple_strtoul(*str, str, 16); - if (old_style || (*str)[0] != '.') { + + /* Old style 0xXXXX or XXXX */ + if (!kstrtouint(str, 16, &val)) { *id0 = *id1 = 0; if (val < 0 || val > 0xffff) return -EINVAL; *devno = val; return 0; } + /* New style x.y.z busid */ - if (val < 0 || val > 0xff) + tok = strsep(&str, "."); + if (kstrtouint(tok, 16, &val) || val > 0xff) return -EINVAL; *id0 = val; - (*str)++; - if (!isxdigit((*str)[0])) /* We require at least one hex digit */ - return -EINVAL; - val = simple_strtoul(*str, str, 16); - if (val < 0 || val > 0xff || (*str)++[0] != '.') + + tok = strsep(&str, "."); + if (kstrtouint(tok, 16, &val) || val > 0xff) return -EINVAL; *id1 = val; - if (!isxdigit((*str)[0])) /* We require at least one hex digit */ - return -EINVAL; - val = simple_strtoul(*str, str, 16); - if (val < 0 || val > 0xffff) + + tok = strsep(&str, "."); + if (kstrtouint(tok, 16, &val) || val > 0xffff) return -EINVAL; *devno = val; + return 0; } /* - * Read colon separated list of dasd features. Currently there is - * only one: "ro" for read-only devices. The default feature set - * is empty (value 0). + * Read colon separated list of dasd features. */ -static int -dasd_feature_list(char *str, char **endp) +static int __init dasd_feature_list(char *str) { int features, len, rc; + features = 0; rc = 0; - if (*str != '(') { - *endp = str; + + if (!str) return DASD_FEATURE_DEFAULT; - } - str++; - features = 0; while (1) { for (len = 0; @@ -225,15 +211,8 @@ dasd_feature_list(char *str, char **endp) break; str++; } - if (*str != ')') { - pr_warn("A closing parenthesis ')' is missing in the dasd= parameter\n"); - rc = -EINVAL; - } else - str++; - *endp = str; - if (rc != 0) - return rc; - return features; + + return rc ? : features; } /* @@ -242,48 +221,38 @@ dasd_feature_list(char *str, char **endp) * action and return a pointer to the residual string. If the first element * could not be matched to any keyword then return an error code. */ -static char * -dasd_parse_keyword( char *parsestring ) { - - char *nextcomma, *residual_str; - int length; +static int __init dasd_parse_keyword(char *keyword) +{ + int length = strlen(keyword); - nextcomma = strchr(parsestring,','); - if (nextcomma) { - length = nextcomma - parsestring; - residual_str = nextcomma + 1; - } else { - length = strlen(parsestring); - residual_str = parsestring + length; - } - if (strncmp("autodetect", parsestring, length) == 0) { + if (strncmp("autodetect", keyword, length) == 0) { dasd_autodetect = 1; pr_info("The autodetection mode has been activated\n"); - return residual_str; + return 0; } - if (strncmp("probeonly", parsestring, length) == 0) { + if (strncmp("probeonly", keyword, length) == 0) { dasd_probeonly = 1; pr_info("The probeonly mode has been activated\n"); - return residual_str; + return 0; } - if (strncmp("nopav", parsestring, length) == 0) { + if (strncmp("nopav", keyword, length) == 0) { if (MACHINE_IS_VM) pr_info("'nopav' is not supported on z/VM\n"); else { dasd_nopav = 1; pr_info("PAV support has be deactivated\n"); } - return residual_str; + return 0; } - if (strncmp("nofcx", parsestring, length) == 0) { + if (strncmp("nofcx", keyword, length) == 0) { dasd_nofcx = 1; pr_info("High Performance FICON support has been " "deactivated\n"); - return residual_str; + return 0; } - if (strncmp("fixedbuffers", parsestring, length) == 0) { + if (strncmp("fixedbuffers", keyword, length) == 0) { if (dasd_page_cache) - return residual_str; + return 0; dasd_page_cache = kmem_cache_create("dasd_page_cache", PAGE_SIZE, PAGE_SIZE, SLAB_CACHE_DMA, @@ -294,73 +263,97 @@ dasd_parse_keyword( char *parsestring ) { else DBF_EVENT(DBF_INFO, "%s", "turning on fixed buffer mode"); - return residual_str; - } - return ERR_PTR(-EINVAL); + return 0; + } + + return -EINVAL; } /* - * Try to interprete the first element on the comma separated parse string - * as a device number or a range of devices. If the interpretation is - * successful, create the matching dasd_devmap entries and return a pointer - * to the residual string. - * If interpretation fails or in case of an error, return an error code. + * Split a string of a device range into its pieces and return the from, to, and + * feature parts separately. + * e.g.: + * 0.0.1234-0.0.5678(ro:erplog) -> from: 0.0.1234 to: 0.0.5678 features: ro:erplog + * 0.0.8765(raw) -> from: 0.0.8765 to: null features: raw + * 0x4321 -> from: 0x4321 to: null features: null */ -static char * -dasd_parse_range( char *parsestring ) { +static int __init dasd_evaluate_range_param(char *range, char **from_str, + char **to_str, char **features_str) +{ + int rc = 0; + /* Do we have a range or a single device? */ + if (strchr(range, '-')) { + *from_str = strsep(&range, "-"); + *to_str = strsep(&range, "("); + *features_str = strsep(&range, ")"); + } else { + *from_str = strsep(&range, "("); + *features_str = strsep(&range, ")"); + } + + if (*features_str && !range) { + pr_warn("A closing parenthesis ')' is missing in the dasd= parameter\n"); + rc = -EINVAL; + } + + return rc; +} + +/* + * Try to interprete the range string as a device number or a range of devices. + * If the interpretation is successful, create the matching dasd_devmap entries. + * If interpretation fails or in case of an error, return an error code. + */ +static int __init dasd_parse_range(const char *range) +{ struct dasd_devmap *devmap; int from, from_id0, from_id1; int to, to_id0, to_id1; - int features, rc; - char bus_id[DASD_BUS_ID_SIZE+1], *str; - - str = parsestring; - rc = dasd_busid(&str, &from_id0, &from_id1, &from); - if (rc == 0) { - to = from; - to_id0 = from_id0; - to_id1 = from_id1; - if (*str == '-') { - str++; - rc = dasd_busid(&str, &to_id0, &to_id1, &to); + int features; + char bus_id[DASD_BUS_ID_SIZE + 1]; + char *features_str = NULL; + char *from_str = NULL; + char *to_str = NULL; + size_t len = strlen(range) + 1; + char tmp[len]; + + strlcpy(tmp, range, len); + + if (dasd_evaluate_range_param(tmp, &from_str, &to_str, &features_str)) + goto out_err; + + if (dasd_busid(from_str, &from_id0, &from_id1, &from)) + goto out_err; + + to = from; + to_id0 = from_id0; + to_id1 = from_id1; + if (to_str) { + if (dasd_busid(to_str, &to_id0, &to_id1, &to)) + goto out_err; + if (from_id0 != to_id0 || from_id1 != to_id1 || from > to) { + pr_err("%s is not a valid device range\n", range); + goto out_err; } } - if (rc == 0 && - (from_id0 != to_id0 || from_id1 != to_id1 || from > to)) - rc = -EINVAL; - if (rc) { - pr_err("%s is not a valid device range\n", parsestring); - return ERR_PTR(rc); - } - features = dasd_feature_list(str, &str); + + features = dasd_feature_list(features_str); if (features < 0) - return ERR_PTR(-EINVAL); + goto out_err; /* each device in dasd= parameter should be set initially online */ features |= DASD_FEATURE_INITIAL_ONLINE; while (from <= to) { - sprintf(bus_id, "%01x.%01x.%04x", - from_id0, from_id1, from++); + sprintf(bus_id, "%01x.%01x.%04x", from_id0, from_id1, from++); devmap = dasd_add_busid(bus_id, features); if (IS_ERR(devmap)) - return (char *)devmap; + return PTR_ERR(devmap); } - if (*str == ',') - return str + 1; - if (*str == '\0') - return str; - pr_warn("The dasd= parameter value %s has an invalid ending\n", str); - return ERR_PTR(-EINVAL); -} -static char * -dasd_parse_next_element( char *parsestring ) { - char * residual_str; - residual_str = dasd_parse_keyword(parsestring); - if (!IS_ERR(residual_str)) - return residual_str; - residual_str = dasd_parse_range(parsestring); - return residual_str; + return 0; + +out_err: + return -EINVAL; } /* @@ -369,30 +362,27 @@ dasd_parse_next_element( char *parsestring ) { * keywords and device ranges. The parameters in that list will be stored as * separate elementes in dasd[]. */ -int -dasd_parse(void) +int __init dasd_parse(void) { int rc, i; - char *parsestring; + char *cur; rc = 0; for (i = 0; i < DASD_MAX_PARAMS; i++) { - if (dasd[i] == NULL) + cur = dasd[i]; + if (!cur) break; - parsestring = dasd[i]; - /* loop over the comma separated list in the parsestring */ - while (*parsestring) { - parsestring = dasd_parse_next_element(parsestring); - if(IS_ERR(parsestring)) { - rc = PTR_ERR(parsestring); - break; - } - } - if (rc) { - DBF_EVENT(DBF_ALERT, "%s", "invalid range found"); + if (*cur == '\0') + continue; + + rc = dasd_parse_keyword(cur); + if (rc) + rc = dasd_parse_range(cur); + + if (rc) break; - } } + return rc; } diff --git a/drivers/s390/block/dasd_int.h b/drivers/s390/block/dasd_int.h index 24be210c10e5..518dba2732d5 100644 --- a/drivers/s390/block/dasd_int.h +++ b/drivers/s390/block/dasd_int.h @@ -805,7 +805,7 @@ struct dasd_device *dasd_device_from_devindex(int); void dasd_add_link_to_gendisk(struct gendisk *, struct dasd_device *); struct dasd_device *dasd_device_from_gendisk(struct gendisk *); -int dasd_parse(void); +int dasd_parse(void) __init; int dasd_busid_known(const char *); /* externals in dasd_gendisk.c */ -- cgit v1.2.3 From defc2a9b9899511445cfbaa2c3a6509964b71207 Mon Sep 17 00:00:00 2001 From: Stefan Haberland Date: Wed, 25 Jan 2017 14:08:20 +0100 Subject: s390/dasd: allow 0 for path_threshold attribute Allow 0 as valid input for the path_threshold attribute to deactivate the IFCC/CCC error handling. Signed-off-by: Stefan Haberland Signed-off-by: Martin Schwidefsky --- drivers/s390/block/dasd_devmap.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/s390/block') diff --git a/drivers/s390/block/dasd_devmap.c b/drivers/s390/block/dasd_devmap.c index 041bdc4429c1..1164b51d09f3 100644 --- a/drivers/s390/block/dasd_devmap.c +++ b/drivers/s390/block/dasd_devmap.c @@ -1518,14 +1518,12 @@ dasd_path_threshold_store(struct device *dev, struct device_attribute *attr, if (IS_ERR(device)) return -ENODEV; - if ((kstrtoul(buf, 10, &val) != 0) || - (val > DASD_THRHLD_MAX) || val == 0) { + if (kstrtoul(buf, 10, &val) != 0 || val > DASD_THRHLD_MAX) { dasd_put_device(device); return -EINVAL; } spin_lock_irqsave(get_ccwdev_lock(to_ccwdev(dev)), flags); - if (val) - device->path_thrhld = val; + device->path_thrhld = val; spin_unlock_irqrestore(get_ccwdev_lock(to_ccwdev(dev)), flags); dasd_put_device(device); return count; -- cgit v1.2.3 From 2202134e48a3b50320aeb9e3dd1186833e9d7e66 Mon Sep 17 00:00:00 2001 From: Stefan Haberland Date: Wed, 25 Jan 2017 14:47:32 +0100 Subject: s390/dasd: check for device error pointer within state change interrupts Check if the device pointer is valid. Just a sanity check since we already are in the int handler of the device. Signed-off-by: Stefan Haberland Signed-off-by: Martin Schwidefsky --- drivers/s390/block/dasd.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/s390/block') diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c index 0e3fdfdbd098..f3099c8cc143 100644 --- a/drivers/s390/block/dasd.c +++ b/drivers/s390/block/dasd.c @@ -1712,8 +1712,11 @@ void dasd_int_handler(struct ccw_device *cdev, unsigned long intparm, /* check for for attention message */ if (scsw_dstat(&irb->scsw) & DEV_STAT_ATTENTION) { device = dasd_device_from_cdev_locked(cdev); - device->discipline->check_attention(device, irb->esw.esw1.lpum); - dasd_put_device(device); + if (!IS_ERR(device)) { + device->discipline->check_attention(device, + irb->esw.esw1.lpum); + dasd_put_device(device); + } } if (!cqr) -- cgit v1.2.3 From ca732e111ff7017e79a0cbb8aa0636c6ce48eb7d Mon Sep 17 00:00:00 2001 From: Stefan Haberland Date: Wed, 25 Jan 2017 16:56:41 +0100 Subject: s390/dasd: check blockdevice pointer before trying to sync blockdevice If safe offline is called for a DASD alias device a null pointer is passed to fsync_bdev. So check for existence of the blockdevice before calling fsync_bdev. Should not be a real world problem since safe offline for an alias device does not make sense and fsync_bdev can deal with a NULL pointer which it gets after successful NULL pointer dereferencing on s390. Signed-off-by: Stefan Haberland Signed-off-by: Martin Schwidefsky --- drivers/s390/block/dasd.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'drivers/s390/block') diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c index f3099c8cc143..6fb3fd5efc11 100644 --- a/drivers/s390/block/dasd.c +++ b/drivers/s390/block/dasd.c @@ -3601,10 +3601,11 @@ int dasd_generic_set_offline(struct ccw_device *cdev) * empty */ /* sync blockdev and partitions */ - rc = fsync_bdev(device->block->bdev); - if (rc != 0) - goto interrupted; - + if (device->block) { + rc = fsync_bdev(device->block->bdev); + if (rc != 0) + goto interrupted; + } /* schedule device tasklet and wait for completion */ dasd_schedule_device_bh(device); rc = wait_event_interruptible(shutdown_waitq, -- cgit v1.2.3 From ba21d0ea218983f269f957de267ca62fdbf96be3 Mon Sep 17 00:00:00 2001 From: Stefan Haberland Date: Wed, 25 Jan 2017 16:59:46 +0100 Subject: s390/dasd: correct inconsistent indenting Signed-off-by: Stefan Haberland Signed-off-by: Martin Schwidefsky --- drivers/s390/block/dasd_eckd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/s390/block') diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c index ade04216c970..0f1713727d4c 100644 --- a/drivers/s390/block/dasd_eckd.c +++ b/drivers/s390/block/dasd_eckd.c @@ -2543,8 +2543,8 @@ dasd_eckd_build_format(struct dasd_device *base, DASD_ECKD_CCW_WRITE_CKD_MT; ccw->flags = CCW_FLAG_SLI; ccw->count = 8; - ccw->cda = (__u32)(addr_t) ect; - ccw++; + ccw->cda = (__u32)(addr_t) ect; + ccw++; } } } -- cgit v1.2.3 From a63f53e34db8b49675448d03ae324f6c5bc04fe6 Mon Sep 17 00:00:00 2001 From: Gerald Schaefer Date: Mon, 30 Jan 2017 15:52:14 +0100 Subject: s390/dcssblk: fix device size calculation in dcssblk_direct_access() Since commit dd22f551 "block: Change direct_access calling convention", the device size calculation in dcssblk_direct_access() is off-by-one. This results in bdev_direct_access() always returning -ENXIO because the returned value is not page aligned. Fix this by adding 1 to the dev_sz calculation. Fixes: dd22f551 ("block: Change direct_access calling convention") Cc: # 4.0+ Signed-off-by: Gerald Schaefer Signed-off-by: Martin Schwidefsky --- drivers/s390/block/dcssblk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/s390/block') diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c index 9d66b4fb174b..415d10a67b7a 100644 --- a/drivers/s390/block/dcssblk.c +++ b/drivers/s390/block/dcssblk.c @@ -892,7 +892,7 @@ dcssblk_direct_access (struct block_device *bdev, sector_t secnum, dev_info = bdev->bd_disk->private_data; if (!dev_info) return -ENODEV; - dev_sz = dev_info->end - dev_info->start; + dev_sz = dev_info->end - dev_info->start + 1; offset = secnum * 512; *kaddr = (void *) dev_info->start + offset; *pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset), PFN_DEV); -- cgit v1.2.3