Re: [PATCH 3/4] libata-scsi: Update SATL for ZAC drives
On 07/29/2014 07:12 PM, Christoph Hellwig wrote: On Tue, Jul 29, 2014 at 04:45:16PM +0200, Hannes Reinecke wrote: ZAC (zoned-access command) drives translate into ZBC (Zoned block command) device type for SCSI. So implement the correct mappings into libata-scsi and update the SCSI command set versions. This seems to both set the periphal type to ZBC and set the HAW_ZBC bit, which is invalid for the version of ZBC spec I've seen. D'oh. You are correct, one should read the doc properly. Doesn't ZAC also destinguish between host aware and host managed device like ZBC does? Not as such. The spec I have only differentiates between CMR (conventional media recording) and SMR (shingled media recording) zones. And every SMR zone is a sequential write required zone. So the host aware device type with sequential write preferred zones is not supported. I'll be sending an updated patch with the correct bits set. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] scsi: qla4xxx: ql4_os.c: Cleaning up missing null-terminate in conjunction with strncpy
Acked-By: Nilesh Javali nilesh.jav...@qlogic.com -Original Message- From: Rickard Strandqvist [mailto:rickard_strandqv...@spectrumdigital.se] Sent: Saturday, July 26, 2014 8:20 PM To: Vikas Chaudhary; Dept-Eng iSCSI Driver Cc: Rickard Strandqvist; James E.J. Bottomley; linux-scsi; linux-kernel Subject: [PATCH] scsi: qla4xxx: ql4_os.c: Cleaning up missing null-terminate in conjunction with strncpy Replacing strncpy with strlcpy to avoid strings that lacks null terminate. Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se --- drivers/scsi/qla4xxx/ql4_os.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c index 3202063..a07c53e 100644 --- a/drivers/scsi/qla4xxx/ql4_os.c +++ b/drivers/scsi/qla4xxx/ql4_os.c @@ -756,9 +756,9 @@ static int qla4xxx_get_chap_list(struct Scsi_Host *shost, uint16_t chap_tbl_idx, continue; chap_rec-chap_tbl_idx = i; - strncpy(chap_rec-username, chap_table-name, + strlcpy(chap_rec-username, chap_table-name, ISCSI_CHAP_AUTH_NAME_MAX_LEN); - strncpy(chap_rec-password, chap_table-secret, + strlcpy(chap_rec-password, chap_table-secret, QL4_CHAP_MAX_SECRET_LEN); chap_rec-password_length = chap_table-secret_len; @@ -6027,8 +6027,8 @@ static int qla4xxx_get_bidi_chap(struct scsi_qla_host *ha, char *username, if (!(chap_table-flags BIT_6)) /* Not BIDI */ continue; - strncpy(password, chap_table-secret, QL4_CHAP_MAX_SECRET_LEN); - strncpy(username, chap_table-name, QL4_CHAP_MAX_NAME_LEN); + strlcpy(password, chap_table-secret, QL4_CHAP_MAX_SECRET_LEN); + strlcpy(username, chap_table-name, QL4_CHAP_MAX_NAME_LEN); ret = 0; break; } @@ -6258,8 +6258,8 @@ static void qla4xxx_get_param_ddb(struct ddb_entry *ddb_entry, tddb-tpgt = sess-tpgt; tddb-port = conn-persistent_port; - strncpy(tddb-iscsi_name, sess-targetname, ISCSI_NAME_SIZE); - strncpy(tddb-ip_addr, conn-persistent_address, DDB_IPADDR_LEN); + strlcpy(tddb-iscsi_name, sess-targetname, ISCSI_NAME_SIZE); + strlcpy(tddb-ip_addr, conn-persistent_address, DDB_IPADDR_LEN); } static void qla4xxx_convert_param_ddb(struct dev_db_entry *fw_ddb_entry, @@ -7764,7 +7764,7 @@ static int qla4xxx_sysfs_ddb_logout(struct iscsi_bus_flash_session *fnode_sess, goto exit_ddb_logout; } - strncpy(flash_tddb-iscsi_name, fnode_sess-targetname, + strlcpy(flash_tddb-iscsi_name, fnode_sess-targetname, ISCSI_NAME_SIZE); if (!strncmp(fnode_sess-portal_type, PORTAL_TYPE_IPV6, 4)) -- 1.7.10.4 attachment: winmail.dat
RE: [PATCH] scsi: qla4xxx: ql4_mbx.c: Cleaning up missing null-terminate in conjunction with strncpy
Acked-By: Nilesh Javali nilesh.jav...@qlogic.com
-Original Message-
From: Rickard Strandqvist [mailto:rickard_strandqv...@spectrumdigital.se]
Sent: Sunday, July 27, 2014 8:28 PM
To: Vikas Chaudhary; Dept-Eng iSCSI Driver
Cc: Rickard Strandqvist; James E.J. Bottomley; linux-scsi; linux-kernel
Subject: [PATCH] scsi: qla4xxx: ql4_mbx.c: Cleaning up missing null-terminate
in conjunction with strncpy
Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
And in some cases modified to copy one character less than the overall length,
as the entire area is already zeroed.
Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
---
drivers/scsi/qla4xxx/ql4_mbx.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/qla4xxx/ql4_mbx.c b/drivers/scsi/qla4xxx/ql4_mbx.c
index 0a3312c..2b2756e 100644
--- a/drivers/scsi/qla4xxx/ql4_mbx.c
+++ b/drivers/scsi/qla4xxx/ql4_mbx.c
@@ -1620,8 +1620,8 @@ int qla4xxx_get_chap(struct scsi_qla_host *ha, char
*username, char *password,
goto exit_get_chap;
}
- strncpy(password, chap_table-secret, QL4_CHAP_MAX_SECRET_LEN);
- strncpy(username, chap_table-name, QL4_CHAP_MAX_NAME_LEN);
+ strlcpy(password, chap_table-secret, QL4_CHAP_MAX_SECRET_LEN);
+ strlcpy(username, chap_table-name, QL4_CHAP_MAX_NAME_LEN);
chap_table-cookie = __constant_cpu_to_le16(CHAP_VALID_COOKIE);
exit_get_chap:
@@ -1663,8 +1663,8 @@ int qla4xxx_set_chap(struct scsi_qla_host *ha, char
*username, char *password,
else
chap_table-flags |= BIT_7; /* local */
chap_table-secret_len = strlen(password);
- strncpy(chap_table-secret, password, MAX_CHAP_SECRET_LEN);
- strncpy(chap_table-name, username, MAX_CHAP_NAME_LEN);
+ strncpy(chap_table-secret, password, MAX_CHAP_SECRET_LEN - 1);
+ strncpy(chap_table-name, username, MAX_CHAP_NAME_LEN - 1);
chap_table-cookie = __constant_cpu_to_le16(CHAP_VALID_COOKIE);
if (is_qla40XX(ha)) {
@@ -1742,8 +1742,8 @@ int qla4xxx_get_uni_chap_at_index(struct scsi_qla_host
*ha, char *username,
goto exit_unlock_uni_chap;
}
- strncpy(password, chap_table-secret, MAX_CHAP_SECRET_LEN);
- strncpy(username, chap_table-name, MAX_CHAP_NAME_LEN);
+ strlcpy(password, chap_table-secret, MAX_CHAP_SECRET_LEN);
+ strlcpy(username, chap_table-name, MAX_CHAP_NAME_LEN);
rval = QLA_SUCCESS;
@@ -2295,7 +2295,7 @@ int qla4_8xxx_set_param(struct scsi_qla_host *ha, int
param)
if (param == SET_DRVR_VERSION) {
mbox_cmd[1] = SET_DRVR_VERSION;
strncpy((char *)mbox_cmd[2], QLA4XXX_DRIVER_VERSION,
- MAX_DRVR_VER_LEN);
+ MAX_DRVR_VER_LEN - 1);
} else {
ql4_printk(KERN_ERR, ha, %s: invalid parameter 0x%x\n,
__func__, param);
--
1.7.10.4
attachment: winmail.dat
RE: [PATCH] qla4xxx: check the return value of dma_alloc_coherent()
Acked-By: Nilesh Javali nilesh.jav...@qlogic.com
-Original Message-
From: Maurizio Lombardi [mailto:mlomb...@redhat.com]
Sent: Monday, July 28, 2014 6:25 PM
To: Vikas Chaudhary
Cc: Dept-Eng iSCSI Driver; linux-scsi; james.bottom...@hansenpartnership.com;
h...@lst.de
Subject: [PATCH] qla4xxx: check the return value of dma_alloc_coherent()
the qla4xxx_alloc_fw_dump() calls dma_alloc_coherent() but does not
check its return value.
Signed-off-by: Maurizio Lombardi mlomb...@redhat.com
---
drivers/scsi/qla4xxx/ql4_init.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/scsi/qla4xxx/ql4_init.c b/drivers/scsi/qla4xxx/ql4_init.c
index 6f12f85..4180d6d 100644
--- a/drivers/scsi/qla4xxx/ql4_init.c
+++ b/drivers/scsi/qla4xxx/ql4_init.c
@@ -334,6 +334,12 @@ void qla4xxx_alloc_fw_dump(struct scsi_qla_host *ha)
/* Allocate memory for saving the template */
md_tmp = dma_alloc_coherent(ha-pdev-dev, ha-fw_dump_tmplt_size,
md_tmp_dma, GFP_KERNEL);
+ if (!md_tmp) {
+ ql4_printk(KERN_INFO, ha,
+ scsi%ld: Failed to allocate DMA memory\n,
+ ha-host_no);
+ return;
+ }
/* Request template */
status = qla4xxx_get_minidump_template(ha, md_tmp_dma);
--
Maurizio Lombardi
attachment: winmail.dat
Re: [PATCH 4/4] sd: Optionally attach to ZBC devices
On 07/29/2014 07:14 PM, Christoph Hellwig wrote: On Tue, Jul 29, 2014 at 04:45:17PM +0200, Hannes Reinecke wrote: ZBC drives are close to disk devices, so sd.c is well suited as a testbed for ZBC devices. This patch introduces a module option 'attach_zbc' to sd which will make the sd driver accept ZBC devices as normal disk drives. Signed-off-by: Hannes Reinecke h...@suse.de Looks reasonable to me. I'd also really like a sysfs file that tells us if we're dealing with a host aware device, as those report as TYPE_SBC. Yes, eventually. But can't we defer that to a next patchset? I'd really like to have this one in, as it makes dealing with ZAC devices _so much_ easier ... Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv4 0/4] Initial SMR drive support
This is a first stab at implementing SMR support. The powers that be decided to call the ATA implementation 'ZAC' (zoned access commands), and the SCSI implementation 'ZBC' (zoned block commands). This is just basic enablement to get ZAC and ZBC drives handled correctly. The first three patches update the libata SATL to handle ZAC devices correctly, and the last patch updates the 'sd' to work correctly with ZBC devices. The 'sd' driver will not automatically bind to ZBC devices; for testing purposes I have added a module parameter 'attach_zbc' to the 'sd' driver. This allows for easy testing of ZBC devices. None of the specific commands like 'report zones' or 'reset write pointer' have been implemented yet as the actual format is still not finalized. This patch is made against the core-for-3.17 tree from hch. Changes to v3: - Remove setting of HAW_ZBC flag Hannes Reinecke (4): libata: consolidate ata_dev_classify() libata: Implement ATA_DEV_ZAC libata-scsi: Update SATL for ZAC drives sd: Optionally attach to ZBC devices drivers/ata/libahci.c | 11 +++ drivers/ata/libata-core.c | 34 +--- drivers/ata/libata-eh.c | 7 +++-- drivers/ata/libata-scsi.c | 30 -- drivers/ata/libata-sff.c| 2 +- drivers/ata/libata-transport.c | 1 + drivers/ata/sata_fsl.c | 11 +++ drivers/ata/sata_inic162x.c | 2 +- drivers/ata/sata_sil24.c| 2 +- drivers/scsi/aic94xx/aic94xx_task.c | 10 +++--- drivers/scsi/isci/request.c | 4 +-- drivers/scsi/libsas/sas_ata.c | 63 + drivers/scsi/mvsas/mv_sas.c | 4 +-- drivers/scsi/pm8001/pm8001_hwi.c| 2 +- drivers/scsi/pm8001/pm80xx_hwi.c| 2 +- drivers/scsi/sd.c | 25 +++ include/linux/libata.h | 8 +++-- include/scsi/libsas.h | 11 ++- 18 files changed, 114 insertions(+), 115 deletions(-) -- 1.7.12.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] sd: Optionally attach to ZBC devices
ZBC drives are close to disk devices, so sd.c is well suited as a testbed for ZBC devices. This patch introduces a module option 'attach_zbc' to sd which will make the sd driver accept ZBC devices as normal disk drives. Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/sd.c | 25 +++-- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 377a520..f957a85 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -91,6 +91,13 @@ MODULE_ALIAS_BLOCKDEV_MAJOR(SCSI_DISK15_MAJOR); MODULE_ALIAS_SCSI_DEVICE(TYPE_DISK); MODULE_ALIAS_SCSI_DEVICE(TYPE_MOD); MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC); +MODULE_ALIAS_SCSI_DEVICE(TYPE_ZBC); + +static bool sd_attach_zbc; + +module_param_named(attach_zbc, sd_attach_zbc, bool, S_IRUGO|S_IWUSR); +MODULE_PARM_DESC(attach_zbc, + Attach to ZBC devices (default=0)); #if !defined(CONFIG_DEBUG_BLOCK_EXT_DEVT) #define SD_MINORS 16 @@ -161,7 +168,7 @@ cache_type_store(struct device *dev, struct device_attribute *attr, static const char temp[] = temporary ; int len; - if (sdp-type != TYPE_DISK) + if (sdp-type != TYPE_DISK sdp-type != TYPE_ZBC) /* no cache control on RBC devices; theoretically they * can do it, but there's probably so many exceptions * it's not worth the risk */ @@ -259,7 +266,7 @@ allow_restart_store(struct device *dev, struct device_attribute *attr, if (!capable(CAP_SYS_ADMIN)) return -EACCES; - if (sdp-type != TYPE_DISK) + if (sdp-type != TYPE_DISK sdp-type != TYPE_ZBC) return -EINVAL; sdp-allow_restart = simple_strtoul(buf, NULL, 10); @@ -389,7 +396,7 @@ provisioning_mode_store(struct device *dev, struct device_attribute *attr, if (!capable(CAP_SYS_ADMIN)) return -EACCES; - if (sdp-type != TYPE_DISK) + if (sdp-type != TYPE_DISK sdp-type != TYPE_ZBC) return -EINVAL; if (!strncmp(buf, lbp_mode[SD_LBP_UNMAP], 20)) @@ -456,7 +463,7 @@ max_write_same_blocks_store(struct device *dev, struct device_attribute *attr, if (!capable(CAP_SYS_ADMIN)) return -EACCES; - if (sdp-type != TYPE_DISK) + if (sdp-type != TYPE_DISK sdp-type != TYPE_ZBC) return -EINVAL; err = kstrtoul(buf, 10, max); @@ -2537,7 +2544,7 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer) struct scsi_mode_data data; struct scsi_sense_hdr sshdr; - if (sdp-type != TYPE_DISK) + if (sdp-type != TYPE_DISK sdp-type != TYPE_ZBC) return; if (sdkp-protection_type == 0) @@ -2957,7 +2964,13 @@ static int sd_probe(struct device *dev) int error; error = -ENODEV; - if (sdp-type != TYPE_DISK sdp-type != TYPE_MOD sdp-type != TYPE_RBC) + if (sdp-type != TYPE_DISK + sdp-type != TYPE_MOD + sdp-type != TYPE_RBC + sdp-type != TYPE_ZBC) + goto out; + + if (sdp-type == TYPE_ZBC !sd_attach_zbc) goto out; SCSI_LOG_HLQUEUE(3, sdev_printk(KERN_INFO, sdp, -- 1.7.12.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] libata: Implement ATA_DEV_ZAC
Add new ATA device type for ZAC devices.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/ata/libata-core.c | 20 ++--
drivers/ata/libata-eh.c| 7 +--
drivers/ata/libata-scsi.c | 5 +++--
drivers/ata/libata-transport.c | 1 +
include/linux/libata.h | 6 --
5 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 2c4d742..5f23804 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1044,8 +1044,8 @@ const char *sata_spd_string(unsigned int spd)
* None.
*
* RETURNS:
- * Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP or
- * %ATA_DEV_UNKNOWN the event of failure.
+ * Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP,
+ * %ATA_DEV_ZAC, or %ATA_DEV_UNKNOWN the event of failure.
*/
unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal)
{
@@ -1090,6 +1090,11 @@ unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal)
return ATA_DEV_SEMB;
}
+ if ((lbam == 0xcd) (lbah == 0xab)) {
+ DPRINTK(found ZAC device by sig\n);
+ return ATA_DEV_ZAC;
+ }
+
DPRINTK(unknown device\n);
return ATA_DEV_UNKNOWN;
}
@@ -1330,7 +1335,7 @@ static int ata_hpa_resize(struct ata_device *dev)
int rc;
/* do we need to do it? */
- if (dev-class != ATA_DEV_ATA ||
+ if ((dev-class != ATA_DEV_ATA dev-class != ATA_DEV_ZAC) ||
!ata_id_has_lba(dev-id) || !ata_id_hpa_enabled(dev-id) ||
(dev-horkage ATA_HORKAGE_BROKEN_HPA))
return 0;
@@ -1890,6 +1895,7 @@ retry:
case ATA_DEV_SEMB:
class = ATA_DEV_ATA;/* some hard drives report SEMB sig */
case ATA_DEV_ATA:
+ case ATA_DEV_ZAC:
tf.command = ATA_CMD_ID_ATA;
break;
case ATA_DEV_ATAPI:
@@ -1981,7 +1987,7 @@ retry:
rc = -EINVAL;
reason = device reports invalid type;
- if (class == ATA_DEV_ATA) {
+ if (class == ATA_DEV_ATA || class == ATA_DEV_ZAC) {
if (!ata_id_is_ata(id) !ata_id_is_cfa(id))
goto err_out;
if (ap-host-flags ATA_HOST_IGNORE_ATA
@@ -2016,7 +2022,8 @@ retry:
goto retry;
}
- if ((flags ATA_READID_POSTRESET) class == ATA_DEV_ATA) {
+ if ((flags ATA_READID_POSTRESET)
+ (class == ATA_DEV_ATA || class == ATA_DEV_ZAC)) {
/*
* The exact sequence expected by certain pre-ATA4 drives is:
* SRST RESET
@@ -2281,7 +2288,7 @@ int ata_dev_configure(struct ata_device *dev)
sizeof(modelbuf));
/* ATA-specific feature tests */
- if (dev-class == ATA_DEV_ATA) {
+ if (dev-class == ATA_DEV_ATA || dev-class == ATA_DEV_ZAC) {
if (ata_id_is_cfa(id)) {
/* CPRM may make this media unusable */
if (id[ATA_ID_CFA_KEY_MGMT] 1)
@@ -4034,6 +4041,7 @@ int ata_dev_revalidate(struct ata_device *dev, unsigned
int new_class,
if (ata_class_enabled(new_class)
new_class != ATA_DEV_ATA
new_class != ATA_DEV_ATAPI
+ new_class != ATA_DEV_ZAC
new_class != ATA_DEV_SEMB) {
ata_dev_info(dev, class mismatch %u != %u\n,
dev-class, new_class);
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 6760fc4..f620c4a 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1809,6 +1809,7 @@ static unsigned int ata_eh_analyze_tf(struct
ata_queued_cmd *qc,
switch (qc-dev-class) {
case ATA_DEV_ATA:
+ case ATA_DEV_ZAC:
if (err ATA_ICRC)
qc-err_mask |= AC_ERR_ATA_BUS;
if (err ATA_UNC)
@@ -3791,7 +3792,8 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t
prereset,
struct ata_eh_context *ehc = link-eh_context;
unsigned long tmp;
- if (dev-class != ATA_DEV_ATA)
+ if (dev-class != ATA_DEV_ATA
+ dev-class != ATA_DEV_ZAC)
continue;
if (!(ehc-i.dev_action[dev-devno]
ATA_EH_PARK))
@@ -3872,7 +3874,8 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t
prereset,
/* retry flush if necessary */
ata_for_each_dev(dev, link, ALL) {
- if (dev-class != ATA_DEV_ATA)
+ if (dev-class != ATA_DEV_ATA
+ dev-class != ATA_DEV_ZAC)
continue;
rc = ata_eh_maybe_retry_flush(dev);
[PATCH 3/4] libata-scsi: Update SATL for ZAC drives
ZAC (zoned-access command) drives translate into ZBC (Zoned block
command) device type for SCSI. So implement the correct mappings
into libata-scsi and update the SCSI command set versions.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/ata/libata-scsi.c | 25 -
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index bea6e7f..1db6eab 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1969,6 +1969,7 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
{
const u8 versions[] = {
+ 0x00,
0x60, /* SAM-3 (no version claimed) */
0x03,
@@ -1977,6 +1978,20 @@ static unsigned int ata_scsiop_inq_std(struct
ata_scsi_args *args, u8 *rbuf)
0x02,
0x60/* SPC-3 (no version claimed) */
};
+ const u8 versions_zbc[] = {
+ 0x00,
+ 0xA0, /* SAM-5 (no version claimed) */
+
+ 0x04,
+ 0xC0, /* SBC-3 (no version claimed) */
+
+ 0x04,
+ 0x60, /* SPC-4 (no version claimed) */
+
+ 0x60,
+ 0x20, /* ZBC (no version claimed) */
+ };
+
u8 hdr[] = {
TYPE_DISK,
0,
@@ -1991,6 +2006,11 @@ static unsigned int ata_scsiop_inq_std(struct
ata_scsi_args *args, u8 *rbuf)
if (ata_id_removeable(args-id))
hdr[1] |= (1 7);
+ if (args-dev-class == ATA_DEV_ZAC) {
+ hdr[0] = TYPE_ZBC;
+ hdr[2] = 0x6; /* ZBC is defined in SPC-4 */
+ }
+
memcpy(rbuf, hdr, sizeof(hdr));
memcpy(rbuf[8], ATA , 8);
ata_id_string(args-id, rbuf[16], ATA_ID_PROD, 16);
@@ -2003,7 +2023,10 @@ static unsigned int ata_scsiop_inq_std(struct
ata_scsi_args *args, u8 *rbuf)
if (rbuf[32] == 0 || rbuf[32] == ' ')
memcpy(rbuf[32], n/a , 4);
- memcpy(rbuf + 59, versions, sizeof(versions));
+ if (args-dev-class == ATA_DEV_ZAC)
+ memcpy(rbuf + 58, versions_zbc, sizeof(versions_zbc));
+ else
+ memcpy(rbuf + 58, versions, sizeof(versions));
return 0;
}
--
1.7.12.4
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] libata: consolidate ata_dev_classify()
ata_dev_classify() just uses the 'lbah' and 'lbam'
fields from the taskfile, so we can as well use those
as arguments and rip out the custom code from sas_ata.
Cc: Dan Williams dan.j.willi...@intel.com
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/ata/libahci.c | 11 +++
drivers/ata/libata-core.c | 14 +
drivers/ata/libata-sff.c| 2 +-
drivers/ata/sata_fsl.c | 11 +++
drivers/ata/sata_inic162x.c | 2 +-
drivers/ata/sata_sil24.c| 2 +-
drivers/scsi/aic94xx/aic94xx_task.c | 10 +++---
drivers/scsi/isci/request.c | 4 +--
drivers/scsi/libsas/sas_ata.c | 63 +
drivers/scsi/mvsas/mv_sas.c | 4 +--
drivers/scsi/pm8001/pm8001_hwi.c| 2 +-
drivers/scsi/pm8001/pm80xx_hwi.c| 2 +-
include/linux/libata.h | 2 +-
include/scsi/libsas.h | 11 ++-
14 files changed, 44 insertions(+), 96 deletions(-)
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index d72ce04..09f9fcb 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1185,16 +1185,15 @@ static void ahci_dev_config(struct ata_device *dev)
unsigned int ahci_dev_classify(struct ata_port *ap)
{
void __iomem *port_mmio = ahci_port_base(ap);
- struct ata_taskfile tf;
+ u8 lbah, lbam, lbal;
u32 tmp;
tmp = readl(port_mmio + PORT_SIG);
- tf.lbah = (tmp 24)0xff;
- tf.lbam = (tmp 16)0xff;
- tf.lbal = (tmp 8) 0xff;
- tf.nsect= (tmp) 0xff;
+ lbah= (tmp 24)0xff;
+ lbam= (tmp 16)0xff;
+ lbal= (tmp 8) 0xff;
- return ata_dev_classify(tf);
+ return ata_dev_classify(lbah, lbam, lbal);
}
EXPORT_SYMBOL_GPL(ahci_dev_classify);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 18d97d5..2c4d742 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1032,7 +1032,9 @@ const char *sata_spd_string(unsigned int spd)
/**
* ata_dev_classify - determine device type based on ATA-spec signature
- * @tf: ATA taskfile register set for device to be identified
+ * @lbah: LBA high byte (LBA bits 23:16)
+ * @lbam: LBA mid byte (LBA bits 15:8)
+ * @lbal: LBA low byte (LBA bits 7:0)
*
* Determine from taskfile register contents whether a device is
* ATA or ATAPI, as per Signature and persistence section
@@ -1045,7 +1047,7 @@ const char *sata_spd_string(unsigned int spd)
* Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP or
* %ATA_DEV_UNKNOWN the event of failure.
*/
-unsigned int ata_dev_classify(const struct ata_taskfile *tf)
+unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal)
{
/* Apple's open source Darwin code hints that some devices only
* put a proper signature into the LBA mid/high registers,
@@ -1068,22 +1070,22 @@ unsigned int ata_dev_classify(const struct ata_taskfile
*tf)
* SEMB signature. This is worked around in
* ata_dev_read_id().
*/
- if ((tf-lbam == 0) (tf-lbah == 0)) {
+ if ((lbam == 0) (lbah == 0)) {
DPRINTK(found ATA device by sig\n);
return ATA_DEV_ATA;
}
- if ((tf-lbam == 0x14) (tf-lbah == 0xeb)) {
+ if ((lbam == 0x14) (lbah == 0xeb)) {
DPRINTK(found ATAPI device by sig\n);
return ATA_DEV_ATAPI;
}
- if ((tf-lbam == 0x69) (tf-lbah == 0x96)) {
+ if ((lbam == 0x69) (lbah == 0x96)) {
DPRINTK(found PMP device by sig\n);
return ATA_DEV_PMP;
}
- if ((tf-lbam == 0x3c) (tf-lbah == 0xc3)) {
+ if ((lbam == 0x3c) (lbah == 0xc3)) {
DPRINTK(found SEMB device by sig (could be ATA device)\n);
return ATA_DEV_SEMB;
}
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 1121153..ec9b5db 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -1903,7 +1903,7 @@ unsigned int ata_sff_dev_classify(struct ata_device *dev,
int present,
return ATA_DEV_NONE;
/* determine if device is ATA or ATAPI */
- class = ata_dev_classify(tf);
+ class = ata_dev_classify(tf.lbah, tf.lbam, tf.lbal);
if (class == ATA_DEV_UNKNOWN) {
/* If the device failed diagnostic, it's likely to
diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 616a6d2..2194fa0 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -807,8 +807,8 @@ static unsigned int sata_fsl_dev_classify(struct ata_port
*ap)
{
struct sata_fsl_host_priv *host_priv = ap-host-private_data;
void __iomem *hcr_base = host_priv-hcr_base;
- struct ata_taskfile tf;
u32 temp;
+ u8 lbah, lbam, lbal;
temp =
Re: [RFD] brd.ko Never supported partitions should we remove the dead code ?
On 07/29/2014 08:19 PM, Ross Zwisler wrote:
On Tue, 2014-07-29 at 19:37 +0300, Boaz Harrosh wrote:
Hi folks
I've been playing with yet unseen prd block device, and hit an issue with
partitioning
but since prd.c is a copy/paste of brd.c the same exact issue exists with
brd.
It does not support partitions!
An attempt to fdisk say a couple of partitions, then mkfs mount an
individual
partition will result in the primary device being accessed and a corrupted
disk data.
If I enable max_part(=2) properly on modprobe, then fdisk a couple of
partitions all
goes well
But then if I pass the device to Kernel (Say via mount), after a call to
bdev = blkdev_get_by_path(dev_name, mode, fs_type);
I get the following results:
(size is extracted using:i_size_read(bdev-bd_inode);
part_size is extracted using: bdev-bd_part-nr_sects 9;
)
dev_name == /dev/ram0
[Jul29 17:40] foo: [foo_mount:880] size=0x4000 bdev=88003dc24340
bd_inode=88003dc24430 bd_part=88003ca22848 part_size=0x4000
dev_name == /dev/ram0p1
[ +16.816065] foo: [foo_mount:880] size=0x4000 bdev=88003d2f6d80
bd_inode=88003d2f6e70 bd_part=88003ca22848 part_size=0x4000
dev_name == /dev/ram0p2
[Jul29 17:41] foo: [foo_mount:880] size=0x4000 bdev=88003dc24680
bd_inode=88003dc24770 bd_part=88003ca22848 part_size=0x4000
We can see that all 3 different bdev's point to the same bd_part, which is
the BUG. Funny is that bd_inode is different but contains
same wrong size, for exactly the same reason.
Yep, I've seen this as well. It turns out that partitions *do* work in brd if
you don't specify the 'rd_nr' module parameter, but if you do specify the
module parameter you get the partition overlapping behavior that you observed.
AFAICT the difference between these two scenarios is that when you set rd_nr,
the 'range' variable is set so something small, but when you don't set it
'range' is 1,048,576. That's done by this bit of code in brd_init:
if (rd_nr) {
nr = rd_nr;
range = rd_nr part_shift;
} else {
nr = CONFIG_BLK_DEV_RAM_COUNT;
range = 1UL MINORBITS;
}
The difference in behavior happens up the stack somewhere as a result of you
passing 'range' into blk_register_region().
Anyway, the quick and dirty workaround is to always have 'range' be something
large. This is what I've done in the current master branch of the upstream
PRD tree - see commit 0256739ccab03d1662221f595b03797370c2ac12.
For what it's worth, I'm planning on updating prd (and probably brd) so that
it dynamically allocates partition numbers. See commit
469071a37afc8a627b6b2ddf29db0a097d864845 for how this was done in the nvme
driver.
Hi Ross
I've looked at 469071a37afc8a627b6b2ddf29db0a097d864845, but surly as you said
it is not the fix. I have posted the fix, in the code you snipped out.
So I get it that you guys are not convinced that we need to remove support for
Partitions for brd? For me a partition for /dev/ramX is totally bogus. What's
the point at all? Since we have the same effect if we just do rd_nr and
partition memory this way, and save 1M of memory. And it is not as if you can
loose power and want the partitioning persistent for the next boot. But sigh I
will not fight you about it, I made my point of this being pointless and that is
that.
I will post a proper patch for brd on top of Jens's tree unless you want it
rebased over some other tree, I guess it can just be queued for 3.17.
- Ross
Thanks
Boaz
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] sd: Optionally attach to ZBC devices
On Wed, Jul 30, 2014 at 08:41:55AM +0200, Hannes Reinecke wrote: Yes, eventually. But can't we defer that to a next patchset? I'd really like to have this one in, as it makes dealing with ZAC devices _so much_ easier ... I'm happy to put the sd patch in if I can get another review for it. But even if you want me to carry the libata bits I will need a clear ACK from Tejun. And I really think it would belong into the libata tree. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] iscsi: iscsi changes for 3.17
Can I get reviews for the remaining patches, please? On Sat, Jul 12, 2014 at 03:51:47PM -0500, micha...@cs.wisc.edu wrote: The following patches were made over Chrisoph's scsi-queue drivers-for-3.17 branch. They are some fixes to the get_host_stats code and new error code for when a iscsi ping times out. Note for applying/merging: The patches also apply over James's scsi misc branch, but that branch is missing some patches that Chrisoph has picked up. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ---end quoted text--- -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] pm8001: fix pm8001_store_update_fw
From: Suresh Thiagarajan suresh.thiagara...@pmcs.com
The current implementation may mix the negative value returned
from pm8001_set_nvmd with with count. -(-ENOMEM) could be interpreted
as bytes programmed, this patch fixes it.
Signed-off-by: Tomas Henzl the...@redhat.com
Signed-off-by: Suresh Thiagarajan suresh.thiagara...@pmcs.com
---
drivers/scsi/pm8001/pm8001_ctl.c | 137 +++---
drivers/scsi/pm8001/pm8001_hwi.c |4 +-
2 files changed, 70 insertions(+), 71 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
index d3a08ae..69aedea 100644
--- a/drivers/scsi/pm8001/pm8001_ctl.c
+++ b/drivers/scsi/pm8001/pm8001_ctl.c
@@ -526,18 +526,19 @@ static int pm8001_set_nvmd(struct pm8001_hba_info
*pm8001_ha)
{
struct pm8001_ioctl_payload *payload;
DECLARE_COMPLETION_ONSTACK(completion);
- u8 *ioctlbuffer = NULL;
- u32 length = 0;
- u32 ret = 0;
+ u8 *ioctlbuffer;
+ u32 ret;
+ u32 length = 1024 * 5 + sizeof(*payload) - 1;
+
+ if (pm8001_ha-fw_image-size 4096) {
+ pm8001_ha-fw_status = FAIL_FILE_SIZE;
+ return -EFAULT;
+ }
- length = 1024 * 5 + sizeof(*payload) - 1;
ioctlbuffer = kzalloc(length, GFP_KERNEL);
- if (!ioctlbuffer)
+ if (!ioctlbuffer) {
+ pm8001_ha-fw_status = FAIL_OUT_MEMORY;
return -ENOMEM;
- if ((pm8001_ha-fw_image-size = 0) ||
- (pm8001_ha-fw_image-size 4096)) {
- ret = FAIL_FILE_SIZE;
- goto out;
}
payload = (struct pm8001_ioctl_payload *)ioctlbuffer;
memcpy((u8 *)payload-func_specific, (u8 *)pm8001_ha-fw_image-data,
@@ -547,6 +548,10 @@ static int pm8001_set_nvmd(struct pm8001_hba_info
*pm8001_ha)
payload-minor_function = 0x1;
pm8001_ha-nvmd_completion = completion;
ret = PM8001_CHIP_DISP-set_nvmd_req(pm8001_ha, payload);
+ if (ret) {
+ pm8001_ha-fw_status = FAIL_OUT_MEMORY;
+ goto out;
+ }
wait_for_completion(completion);
out:
kfree(ioctlbuffer);
@@ -557,26 +562,25 @@ static int pm8001_update_flash(struct pm8001_hba_info
*pm8001_ha)
{
struct pm8001_ioctl_payload *payload;
DECLARE_COMPLETION_ONSTACK(completion);
- u8 *ioctlbuffer = NULL;
- u32 length = 0;
+ u8 *ioctlbuffer;
struct fw_control_info *fwControl;
- u32 loopNumber, loopcount = 0;
- u32 sizeRead = 0;
u32 partitionSize, partitionSizeTmp;
- u32 ret = 0;
- u32 partitionNumber = 0;
+ u32 loopNumber, loopcount;
struct pm8001_fw_image_header *image_hdr;
+ u32 sizeRead = 0;
+ u32 ret = 0;
+ u32 length = 1024 * 16 + sizeof(*payload) - 1;
- length = 1024 * 16 + sizeof(*payload) - 1;
+ if (pm8001_ha-fw_image-size 28) {
+ pm8001_ha-fw_status = FAIL_FILE_SIZE;
+ return -EFAULT;
+ }
ioctlbuffer = kzalloc(length, GFP_KERNEL);
- image_hdr = (struct pm8001_fw_image_header *)pm8001_ha-fw_image-data;
- if (!ioctlbuffer)
+ if (!ioctlbuffer) {
+ pm8001_ha-fw_status = FAIL_OUT_MEMORY;
return -ENOMEM;
- if (pm8001_ha-fw_image-size 28) {
- ret = FAIL_FILE_SIZE;
- goto out;
}
-
+ image_hdr = (struct pm8001_fw_image_header *)pm8001_ha-fw_image-data;
while (sizeRead pm8001_ha-fw_image-size) {
partitionSizeTmp =
*(u32 *)((u8 *)image_hdr-image_length + sizeRead);
@@ -617,18 +621,18 @@ static int pm8001_update_flash(struct pm8001_hba_info
*pm8001_ha)
pm8001_ha-nvmd_completion = completion;
ret = PM8001_CHIP_DISP-fw_flash_update_req(pm8001_ha, payload);
- if (ret)
- break;
+ if (ret) {
+ pm8001_ha-fw_status = FAIL_OUT_MEMORY;
+ goto out;
+ }
wait_for_completion(completion);
if (fwControl-retcode FLASH_UPDATE_IN_PROGRESS) {
- ret = fwControl-retcode;
- break;
+ pm8001_ha-fw_status = fwControl-retcode;
+ ret = -EFAULT;
+ goto out;
+ }
}
}
- if (ret)
- break;
- partitionNumber++;
-}
out:
kfree(ioctlbuffer);
return ret;
@@ -643,22 +647,29 @@ static ssize_t pm8001_store_update_fw(struct device *cdev,
char *cmd_ptr, *filename_ptr;
int res, i;
int flash_command = FLASH_CMD_NONE;
-
[PATCH 2/3] pm8001: Fix erratic calculation in update_flash
From: Suresh Thiagarajan suresh.thiagara...@pmcs.com
The loopcount is calculated by using some weird magic.
Use instead a boring macro
Signed-off-by: Tomas Henzl the...@redhat.com
Signed-off-by: Suresh Thiagarajan suresh.thiagara...@pmcs.com
---
drivers/scsi/pm8001/pm8001_ctl.c |7 ++-
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
index 69aedea..7abbf28 100644
--- a/drivers/scsi/pm8001/pm8001_ctl.c
+++ b/drivers/scsi/pm8001/pm8001_ctl.c
@@ -585,11 +585,8 @@ static int pm8001_update_flash(struct pm8001_hba_info
*pm8001_ha)
partitionSizeTmp =
*(u32 *)((u8 *)image_hdr-image_length + sizeRead);
partitionSize = be32_to_cpu(partitionSizeTmp);
- loopcount = (partitionSize + HEADER_LEN)/IOCTL_BUF_SIZE;
- if (loopcount % IOCTL_BUF_SIZE)
- loopcount++;
- if (loopcount == 0)
- loopcount++;
+ loopcount = DIV_ROUND_UP(partitionSize + HEADER_LEN,
+ IOCTL_BUF_SIZE);
for (loopNumber = 0; loopNumber loopcount; loopNumber++) {
payload = (struct pm8001_ioctl_payload *)ioctlbuffer;
payload-length = 1024*16;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] pm8001: Update MAINTAINERS list
From: Suresh Thiagarajan suresh.thiagara...@pmcs.com Update pmcs mail list for pm8001 driver support Signed-off-by: Suresh Thiagarajan suresh.thiagara...@pmcs.com --- MAINTAINERS |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 3f2e171..a63259c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6987,6 +6987,7 @@ F:drivers/scsi/pmcraid.* PMC SIERRA PM8001 DRIVER M: xjtu...@gmail.com M: lindar_...@usish.com +L: pmc...@pmcs.com L: linux-scsi@vger.kernel.org S: Supported F: drivers/scsi/pm8001/ -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] pm8001: fix pm8001_store_update_fw
On Wed, 2014-07-30 at 17:33 +0530, Suresh Thiagarajan wrote: From: Suresh Thiagarajan suresh.thiagara...@pmcs.com The current implementation may mix the negative value returned from pm8001_set_nvmd with with count. -(-ENOMEM) could be interpreted as bytes programmed, this patch fixes it. Signed-off-by: Tomas Henzl the...@redhat.com Signed-off-by: Suresh Thiagarajan suresh.thiagara...@pmcs.com This author and signoff chain doesn't make sense. Is the patch from Tomas Henzl? In which case the From: field is wrong; it should read From: Tomas Henzl the...@redhat.com James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] pm8001: Update MAINTAINERS list
On Wed, 2014-07-30 at 17:37 +0530, Suresh Thiagarajan wrote: From: Suresh Thiagarajan suresh.thiagara...@pmcs.com Update pmcs mail list for pm8001 driver support Signed-off-by: Suresh Thiagarajan suresh.thiagara...@pmcs.com --- MAINTAINERS |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 3f2e171..a63259c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6987,6 +6987,7 @@ F: drivers/scsi/pmcraid.* PMC SIERRA PM8001 DRIVER M: xjtu...@gmail.com M: lindar_...@usish.com +L: pmc...@pmcs.com This list doesn't appear to work: pmc...@pmcs.com: host smtp.pmc-sierra.com[216.241.235.148] said: 550 5.1.1 pmc...@pmcs.com: Recipient address rejected: User unknown in local recipient table (in reply to RCPT TO command) James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WARNING: CPU: 1 PID: 495 at mm/slab_common.c:69 kmem_cache_create+0x1a9/0x330()
On Tue, Jul 29, 2014 at 02:26:11PM +0200, Christoph Hellwig wrote: On Mon, Jul 28, 2014 at 11:49:22AM +0400, James Bottomley wrote: That needs to be From: James Bottomley jbottom...@parallels.com As well. I do list handling on hansenpartnership.com to minimise exchange wreckage on mailinglists, but I should acknowledge Parallels as supporting the work I do. Thanks, I've update the author, added a Cc to ћtable and pushed it out to the core-for-3.17 branch. This fixes a bug in the 3.16 kernel. Why wouldn't it be sent to Linus for inclusion in the final release there? josh -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3] pm8001: fix pm8001_store_update_fw
On Wed, Jul 30, 2014 at 5:40 PM, James Bottomley james.bottom...@hansenpartnership.com wrote: On Wed, 2014-07-30 at 17:33 +0530, Suresh Thiagarajan wrote: From: Suresh Thiagarajan suresh.thiagara...@pmcs.com The current implementation may mix the negative value returned from pm8001_set_nvmd with with count. -(-ENOMEM) could be interpreted as bytes programmed, this patch fixes it. Signed-off-by: Tomas Henzl the...@redhat.com Signed-off-by: Suresh Thiagarajan suresh.thiagara...@pmcs.com This author and signoff chain doesn't make sense. Is the patch from Tomas Henzl? In which case the From: field is wrong; it should read From: Tomas Henzl the...@redhat.com Yes. This patch is from Tomas. I will correct the From and send again -Suresh James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] iscsi class: fix get_host_stats error handling
On 07/12/2014 10:51 PM, micha...@cs.wisc.edu wrote:
From: Mike Christie micha...@cs.wisc.edu
iscsi_get_host_stats was dropping the error code returned
by drivers like qla4xxx.
Signed-off-by: Mike Christie micha...@cs.wisc.edu
---
drivers/scsi/scsi_transport_iscsi.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index b481e62..14bfa53 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -3467,6 +3467,10 @@ iscsi_get_host_stats(struct iscsi_transport *transport,
struct nlmsghdr *nlh)
memset(buf, 0, host_stats_size);
err = transport-get_host_stats(shost, buf, host_stats_size);
+ if (err) {
+ kfree(skbhost_stats);
+ goto exit_host_stats;
+ }
actual_size = nlmsg_total_size(sizeof(*ev) + host_stats_size);
skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size));
What happens with the skbhost_stats allocated earlier? Shouldn't it
be freed here, too?
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] qla4xxx: fix get_host_stats error propagation
On 07/12/2014 10:51 PM, micha...@cs.wisc.edu wrote:
From: Mike Christie micha...@cs.wisc.edu
qla4xxx was not always returning -EXYZ error codes when
qla4xxx_get_host_stats failed.
Signed-off-by: Mike Christie micha...@cs.wisc.edu
Acked-by: Vikas Chaudhary vikas.chaudh...@qlogic.com
---
drivers/scsi/qla4xxx/ql4_os.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index c5d9564..b79b48c 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -1050,6 +1050,7 @@ static int qla4xxx_get_host_stats(struct Scsi_Host
*shost, char *buf, int len)
if (!ql_iscsi_stats) {
ql4_printk(KERN_ERR, ha,
Unable to allocate memory for iscsi stats\n);
+ ret = -ENOMEM;
goto exit_host_stats;
}
@@ -1058,6 +1059,7 @@ static int qla4xxx_get_host_stats(struct Scsi_Host
*shost, char *buf, int len)
if (ret != QLA_SUCCESS) {
ql4_printk(KERN_ERR, ha,
Unable to retrieve iscsi stats\n);
+ ret = -EIO;
goto exit_host_stats;
}
host_stats-mactx_frames = le64_to_cpu(ql_iscsi_stats-mac_tx_frames);
Reviewed-by: Hannes Reinecke h...@suse.de
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] iscsi: iscsi changes for 3.17
On 13/07/14 3:20 pm, Christoph Hellwig h...@infradead.org wrote: On Sat, Jul 12, 2014 at 03:51:47PM -0500, micha...@cs.wisc.edu wrote: The following patches were made over Chrisoph's scsi-queue drivers-for-3.17 branch. They are some fixes to the get_host_stats code and new error code for when a iscsi ping times out. The series looks fine to me, I'll merge it as soon as I get the remaining second reviews. Ack for series Acked-by: Vikas Chaudhary vikas.chaudh...@qlogic.com attachment: winmail.dat
Re: [PATCH 3/4] iscsi class: fix get_host_stats return code when not supported
On 07/12/2014 10:51 PM, micha...@cs.wisc.edu wrote: From: Mike Christie micha...@cs.wisc.edu When the get_host_stats call was not supported we were returing EINVAL. This has us return ENOSYS, because for software iscsi drivers where there is no host it is ok to not have this callout. Signed-off-by: Mike Christie micha...@cs.wisc.edu --- drivers/scsi/scsi_transport_iscsi.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c index 14bfa53..534d3fb 100644 --- a/drivers/scsi/scsi_transport_iscsi.c +++ b/drivers/scsi/scsi_transport_iscsi.c @@ -3429,7 +3429,7 @@ iscsi_get_host_stats(struct iscsi_transport *transport, struct nlmsghdr *nlh) char *buf; if (!transport-get_host_stats) - return -EINVAL; + return -ENOSYS; priv = iscsi_if_transport_lookup(transport); if (!priv) Reviewed-by: Hannes Reinecke h...@suse.de Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] libiscsi: return new error code when nop times out
On 07/12/2014 10:51 PM, micha...@cs.wisc.edu wrote:
From: Mike Christie micha...@cs.wisc.edu
When a iscsi nop as ping timedout we were failing with the
common connection error code, ISCSI_ERR_CONN_FAILED. This
patch adds a new error code for this problem so can properly
track/distinguish in userspace.
Signed-off-by: Mike Christie micha...@cs.wisc.edu
---
drivers/scsi/libiscsi.c |2 +-
include/scsi/iscsi_if.h |1 +
2 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index f2db82b..b0813fd 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -2097,7 +2097,7 @@ static void iscsi_check_transport_timeouts(unsigned long
data)
conn-ping_timeout, conn-recv_timeout,
last_recv, conn-last_ping, jiffies);
spin_unlock(session-frwd_lock);
- iscsi_conn_failure(conn, ISCSI_ERR_CONN_FAILED);
+ iscsi_conn_failure(conn, ISCSI_ERR_NOP_TIMEDOUT);
return;
}
diff --git a/include/scsi/iscsi_if.h b/include/scsi/iscsi_if.h
index fd0421c..95ed942 100644
--- a/include/scsi/iscsi_if.h
+++ b/include/scsi/iscsi_if.h
@@ -527,6 +527,7 @@ enum iscsi_err {
ISCSI_ERR_XMIT_FAILED = ISCSI_ERR_BASE + 19,
ISCSI_ERR_TCP_CONN_CLOSE= ISCSI_ERR_BASE + 20,
ISCSI_ERR_SCSI_EH_SESSION_RST = ISCSI_ERR_BASE + 21,
+ ISCSI_ERR_NOP_TIMEDOUT = ISCSI_ERR_BASE + 22,
};
/*
Reviewed-by: Hannes Reinecke h...@suse.de
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] pm8001: fix pm8001_store_update_fw
From: Tomas Henzl the...@redhat.com
The current implementation may mix the negative value returned
from pm8001_set_nvmd with with count. -(-ENOMEM) could be interpreted
as bytes programmed, this patch fixes it.
Signed-off-by: Tomas Henzl the...@redhat.com
Signed-off-by: Suresh Thiagarajan suresh.thiagara...@pmcs.com
---
drivers/scsi/pm8001/pm8001_ctl.c | 137 +++---
drivers/scsi/pm8001/pm8001_hwi.c |4 +-
2 files changed, 70 insertions(+), 71 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
index d3a08ae..69aedea 100644
--- a/drivers/scsi/pm8001/pm8001_ctl.c
+++ b/drivers/scsi/pm8001/pm8001_ctl.c
@@ -526,18 +526,19 @@ static int pm8001_set_nvmd(struct pm8001_hba_info
*pm8001_ha)
{
struct pm8001_ioctl_payload *payload;
DECLARE_COMPLETION_ONSTACK(completion);
- u8 *ioctlbuffer = NULL;
- u32 length = 0;
- u32 ret = 0;
+ u8 *ioctlbuffer;
+ u32 ret;
+ u32 length = 1024 * 5 + sizeof(*payload) - 1;
+
+ if (pm8001_ha-fw_image-size 4096) {
+ pm8001_ha-fw_status = FAIL_FILE_SIZE;
+ return -EFAULT;
+ }
- length = 1024 * 5 + sizeof(*payload) - 1;
ioctlbuffer = kzalloc(length, GFP_KERNEL);
- if (!ioctlbuffer)
+ if (!ioctlbuffer) {
+ pm8001_ha-fw_status = FAIL_OUT_MEMORY;
return -ENOMEM;
- if ((pm8001_ha-fw_image-size = 0) ||
- (pm8001_ha-fw_image-size 4096)) {
- ret = FAIL_FILE_SIZE;
- goto out;
}
payload = (struct pm8001_ioctl_payload *)ioctlbuffer;
memcpy((u8 *)payload-func_specific, (u8 *)pm8001_ha-fw_image-data,
@@ -547,6 +548,10 @@ static int pm8001_set_nvmd(struct pm8001_hba_info
*pm8001_ha)
payload-minor_function = 0x1;
pm8001_ha-nvmd_completion = completion;
ret = PM8001_CHIP_DISP-set_nvmd_req(pm8001_ha, payload);
+ if (ret) {
+ pm8001_ha-fw_status = FAIL_OUT_MEMORY;
+ goto out;
+ }
wait_for_completion(completion);
out:
kfree(ioctlbuffer);
@@ -557,26 +562,25 @@ static int pm8001_update_flash(struct pm8001_hba_info
*pm8001_ha)
{
struct pm8001_ioctl_payload *payload;
DECLARE_COMPLETION_ONSTACK(completion);
- u8 *ioctlbuffer = NULL;
- u32 length = 0;
+ u8 *ioctlbuffer;
struct fw_control_info *fwControl;
- u32 loopNumber, loopcount = 0;
- u32 sizeRead = 0;
u32 partitionSize, partitionSizeTmp;
- u32 ret = 0;
- u32 partitionNumber = 0;
+ u32 loopNumber, loopcount;
struct pm8001_fw_image_header *image_hdr;
+ u32 sizeRead = 0;
+ u32 ret = 0;
+ u32 length = 1024 * 16 + sizeof(*payload) - 1;
- length = 1024 * 16 + sizeof(*payload) - 1;
+ if (pm8001_ha-fw_image-size 28) {
+ pm8001_ha-fw_status = FAIL_FILE_SIZE;
+ return -EFAULT;
+ }
ioctlbuffer = kzalloc(length, GFP_KERNEL);
- image_hdr = (struct pm8001_fw_image_header *)pm8001_ha-fw_image-data;
- if (!ioctlbuffer)
+ if (!ioctlbuffer) {
+ pm8001_ha-fw_status = FAIL_OUT_MEMORY;
return -ENOMEM;
- if (pm8001_ha-fw_image-size 28) {
- ret = FAIL_FILE_SIZE;
- goto out;
}
-
+ image_hdr = (struct pm8001_fw_image_header *)pm8001_ha-fw_image-data;
while (sizeRead pm8001_ha-fw_image-size) {
partitionSizeTmp =
*(u32 *)((u8 *)image_hdr-image_length + sizeRead);
@@ -617,18 +621,18 @@ static int pm8001_update_flash(struct pm8001_hba_info
*pm8001_ha)
pm8001_ha-nvmd_completion = completion;
ret = PM8001_CHIP_DISP-fw_flash_update_req(pm8001_ha, payload);
- if (ret)
- break;
+ if (ret) {
+ pm8001_ha-fw_status = FAIL_OUT_MEMORY;
+ goto out;
+ }
wait_for_completion(completion);
if (fwControl-retcode FLASH_UPDATE_IN_PROGRESS) {
- ret = fwControl-retcode;
- break;
+ pm8001_ha-fw_status = fwControl-retcode;
+ ret = -EFAULT;
+ goto out;
+ }
}
}
- if (ret)
- break;
- partitionNumber++;
-}
out:
kfree(ioctlbuffer);
return ret;
@@ -643,22 +647,29 @@ static ssize_t pm8001_store_update_fw(struct device *cdev,
char *cmd_ptr, *filename_ptr;
int res, i;
int flash_command = FLASH_CMD_NONE;
- int err = 0;
Re: [PATCH 4/4] scsi: pm8001: fix pm8001_store_update_fw
On 07/10/2014 03:30 PM, Tomas Henzl wrote: On 07/10/2014 08:43 AM, Christoph Hellwig wrote: On Mon, Jul 07, 2014 at 05:20:01PM +0200, Tomas Henzl wrote: The current implementation may mix the negative value returned from pm8001_set_nvmd with with count. -(-ENOMEM) could be interpreted as bytes programmed, this patch fixes it. This still doesn;t look correct to me as err mixes up the driver internal FAIL_* codes with Linux error codes. It seems like for the FAIL_* codes should only go into -fw_status and the return value should be a proper Linux error code. And the fw_status might be later used to show error strings in pm8001_show_update_fw, if it is so it depends on the flash utility but it seems likely. Funny fact: the FAIL_* / FLASH_IN_PROGRESS codes seems to be the same between aic94xx and pm8001. And similar story there too - asd_store_update_bios -...- asd_poll_flash(might return -ENOENT) Maybe the flash utility ignores the return value or it has never happened. - I'll try to find what seems to be the most probable way and post it in few days. Christoph, from few days it is three weeks and the patch has been just replaced by [PATCH 1/3] pm8001: fix pm8001_store_update_fw sent by Suresh, so you can drop this patch. (I've asked Suresh off-list to test it, I didn't want to brick my card by testing fw flashes.) Thanks, Tomas This patch is not related to the patches 1-3/4, so just wait with this one. Thanks, Tomas -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] pm8001: Fix erratic calculation in update_flash
From: Tomas Henzl the...@redhat.com
The loopcount is calculated by using some weird magic.
Use instead a boring macro
Signed-off-by: Tomas Henzl the...@redhat.com
Signed-off-by: Suresh Thiagarajan suresh.thiagara...@pmcs.com
---
drivers/scsi/pm8001/pm8001_ctl.c |7 ++-
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
index 69aedea..7abbf28 100644
--- a/drivers/scsi/pm8001/pm8001_ctl.c
+++ b/drivers/scsi/pm8001/pm8001_ctl.c
@@ -585,11 +585,8 @@ static int pm8001_update_flash(struct pm8001_hba_info
*pm8001_ha)
partitionSizeTmp =
*(u32 *)((u8 *)image_hdr-image_length + sizeRead);
partitionSize = be32_to_cpu(partitionSizeTmp);
- loopcount = (partitionSize + HEADER_LEN)/IOCTL_BUF_SIZE;
- if (loopcount % IOCTL_BUF_SIZE)
- loopcount++;
- if (loopcount == 0)
- loopcount++;
+ loopcount = DIV_ROUND_UP(partitionSize + HEADER_LEN,
+ IOCTL_BUF_SIZE);
for (loopNumber = 0; loopNumber loopcount; loopNumber++) {
payload = (struct pm8001_ioctl_payload *)ioctlbuffer;
payload-length = 1024*16;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] brd: Fix the partitions BUG
With current code after a call to:
bdev = blkdev_get_by_path(dev_name, mode, fs_type);
size = i_size_read(bdev-bd_inode);
part_size = bdev-bd_part-nr_sects 9;
I get the following bad results:
dev_name == /dev/ram0
foo: [foo_mount:880] size=0x4000 bdev=88003dc24340 \
bd_inode=88003dc24430 bd_part=88003ca22848 part_size=0x4000
dev_name == /dev/ram0p1
foo: [foo_mount:880] size=0x4000 bdev=88003d2f6d80 \
bd_inode=88003d2f6e70 bd_part=88003ca22848 part_size=0x4000
dev_name == /dev/ram0p2
foo: [foo_mount:880] size=0x4000 bdev=88003dc24680 \
bd_inode=88003dc24770 bd_part=88003ca22848 part_size=0x4000
Note how all three bdev(s) point to the same bd_part.
This is do to a single bad clubber in brd_probe() which is
removed in this patch:
- *part = 0;
because of this all 3 bdev(s) above get to point to the same bd_part[0]
While at it fix/rename brd_init_one() since all devices are created on
load of driver, brd_probe() will never be called with a new un-created
device.
brd_init_one() is now renamed to brd_find() which is what it does.
TODO: There is one more partitions BUG regarding
brd_direct_access() which is fixed on the next patch.
Signed-off-by: Boaz Harrosh b...@plexistor.com
---
drivers/block/brd.c | 19 ---
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index c7d138e..92334f6 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -523,22 +523,20 @@ static void brd_free(struct brd_device *brd)
kfree(brd);
}
-static struct brd_device *brd_init_one(int i)
+static struct brd_device *brd_find(int i)
{
struct brd_device *brd;
list_for_each_entry(brd, brd_devices, brd_list) {
if (brd-brd_number == i)
- goto out;
+ return brd;
}
- brd = brd_alloc(i);
- if (brd) {
- add_disk(brd-brd_disk);
- list_add_tail(brd-brd_list, brd_devices);
- }
-out:
- return brd;
+ /* brd always allocates all its devices at load time, therefor
+* brd_probe will never be called with a new brd_number
+*/
+ printk(KERN_EROR brd: brd_find unexpected device %d\n, i);
+ return NULL;
}
static void brd_del_one(struct brd_device *brd)
@@ -554,11 +552,10 @@ static struct kobject *brd_probe(dev_t dev, int *part,
void *data)
struct kobject *kobj;
mutex_lock(brd_devices_mutex);
- brd = brd_init_one(MINOR(dev) part_shift);
+ brd = brd_find(MINOR(dev) part_shift);
kobj = brd ? get_disk(brd-brd_disk) : NULL;
mutex_unlock(brd_devices_mutex);
- *part = 0;
return kobj;
}
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] brd: Fix brd_direct_access with partitions
When brd_direct_access() is called on a partition-bdev it would access the wrong sector. And caller would then corrupt the device's data. This is a preliminary fix, Matthew Wilcox has a patch in his DAX patchset which will define a global wrapper to bdev-bd_disk-fops-direct_access that will do the proper checks and translations before calling a driver global member. (The way it is done at the rest of the block stack) CC: Matthew Wilcox matthew.r.wil...@intel.com Signed-off-by: Boaz Harrosh b...@plexistor.com --- drivers/block/brd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/block/brd.c b/drivers/block/brd.c index 92334f6..7506864 100644 --- a/drivers/block/brd.c +++ b/drivers/block/brd.c @@ -378,9 +378,10 @@ static int brd_direct_access(struct block_device *bdev, sector_t sector, if (!brd) return -ENODEV; + sector += get_start_sect(bdev); if (sector (PAGE_SECTORS-1)) return -EINVAL; - if (sector + PAGE_SECTORS get_capacity(bdev-bd_disk)) + if (unlikely(sector + PAGE_SECTORS part_nr_sects_read(bdev-bd_part))) return -ERANGE; page = brd_insert_page(brd, sector); if (!page) -- 1.9.3 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] driver core: enable drivers to use deferred probefrom init
On Wed, Jul 30, 2014 at 11:22:14AM +0900, Tetsuo Handa wrote: Luis R. Rodriguez wrote: Tetsuo is it possible / desirable to allow tasks to not kill unless the reason is OOM ? Its unclear if this was discussed before, sorry if it was, have just been a bit busy today to review the archive / discussions on this. Are we aware that the 10 seconds timeout after SIGKILL is not the duration between the beginning of module loading and the end of kthread_create() but the duration to wait for kthreadd to create a new kernel thread? If the kthreadd is unable to create a new kernel thread within 10 seconds, something very bad is happening. For example, memory allocation deadlock sequence shown below might be happening. (1) process1 holds a mutex using mutex_lock(). (2) process1 calls kthread_create() and enters into killable wait state at wait_for_completion_killable(). (3) kthreadd calls kernel_thread() and enters into oom-killable busy loop due to out of memory at alloc_pages_nodemask(). (4) process2 is chosen by the OOM killer, but process2 is unable to terminate because process2 is waiting in unkillable state at mutex_lock() which was held by process1 at (1). (5) kthreadd continues busy loop because process2 does not release memory and the OOM killer does not kill more processes. (6) process1 continues waiting in oom-killable state because process1 is not chosen by the OOM killer. See? The system will remain unresponding unless somebody releases memory that is enough for kthreadd to complete. I see but we're talking about large systems with gobs of memory so I'm pretty sure memory should not be the issue here. We cannot teach process1 that process1 needs to give up waiting for kthreadd and call mutex_unlock() in order to allow process2 to terminate. Also, we cannot teach the OOM killer that process1 needs to be oom-killed after process2 is oom-killed. Making the 10 seconds timeout after SIGKILL longer is safe. Changing it to no-timeout-unless-oom-killed is unsafe. To be clear we have *not* merged the 10 second workaround: https://launchpadlibrarian.net/169714201/kthread-Do-not-leave-kthread_create-immediately.patch and come to think of it the work aroaund is aligned with what I was thinking *without *waiting for 10 seconds, but my question was whether or not it was reasonable to have the process request to go through this excemption. So we would not do this all the time, but only for processes that would request this, in this case modprobe. Luis -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3 V2] pm8001: Fix erratic calculation in update_flash
V2 - 'from' string removed (as it comes from me now) and changed a signed-off
to acked-by (Suresh is the maintainer)
Tomas
The loopcount is calculated by using some weird magic.
Use instead a boring macro
Signed-off-by: Tomas Henzl the...@redhat.com
Acked-by: Suresh Thiagarajan suresh.thiagara...@pmcs.com
---
drivers/scsi/pm8001/pm8001_ctl.c |7 ++-
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
index 69aedea..7abbf28 100644
--- a/drivers/scsi/pm8001/pm8001_ctl.c
+++ b/drivers/scsi/pm8001/pm8001_ctl.c
@@ -585,11 +585,8 @@ static int pm8001_update_flash(struct pm8001_hba_info
*pm8001_ha)
partitionSizeTmp =
*(u32 *)((u8 *)image_hdr-image_length + sizeRead);
partitionSize = be32_to_cpu(partitionSizeTmp);
- loopcount = (partitionSize + HEADER_LEN)/IOCTL_BUF_SIZE;
- if (loopcount % IOCTL_BUF_SIZE)
- loopcount++;
- if (loopcount == 0)
- loopcount++;
+ loopcount = DIV_ROUND_UP(partitionSize + HEADER_LEN,
+ IOCTL_BUF_SIZE);
for (loopNumber = 0; loopNumber loopcount; loopNumber++) {
payload = (struct pm8001_ioctl_payload *)ioctlbuffer;
payload-length = 1024*16;
-- 1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WARNING: CPU: 1 PID: 495 at mm/slab_common.c:69 kmem_cache_create+0x1a9/0x330()
On Wed, Jul 30, 2014 at 08:21:35AM -0400, Josh Boyer wrote: Thanks, I've update the author, added a Cc to ћtable and pushed it out to the core-for-3.17 branch. This fixes a bug in the 3.16 kernel. Why wouldn't it be sent to Linus for inclusion in the final release there? I'm only collecting patches for scsi, but James remains formal maintainer, so I don't send anything to Linus. Given that delays between me committing things, them getting picked up by James and finally making it to Linux-next I don't feel like another for-3.16 branch at this point is easily workable. If James wants to cherry pick it and send it on at this time that might still work fine, but he seems fairly busy. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 07/10][SCSI]mpt2sas: Added Reply Descriptor Post Queue (RDPQ) Array support
Hi Martin,
Sorry for the delay as I was on vacation.
Here are the change set when compared to the initial patch
1. Declared the following functions at the beginning of the
mpt2sas_base.c file instead of moving all these functions before
mpt2sas_base_map_resources() function
a. _base_wait_for_doorbell_int()
b. _base_wait_for_doorbell_ack()
c. _base_wait_for_doorbell_not_used()
d. _base_handshake_req_reply_wait()
e. _base_get_ioc_facts()
2. Initially set the consistent DMA mask to 32 bit and then change it
to 64 bit mask after allocating RDPQ pools by calling the function
_base_change_consistent_dma_mask. This is to ensure that all the upper
32 bits of RDPQ entries's base address to be same. Also allocated the
RDPQ pools first and then allocated remaining pools such as request
pool, sense buffer pools etc. Also this patch takes care to set the
consistent DMA mask to 64 bit (if it supports) even after resume
operation from hibernation.
3. Reduced the redundancy between the RDPQ and non-RDPQ support in
these following functions
a. _base_release_memory_pools()
b. _base_allocate_memory_pools()
c. _base_send_ioc_init()
d. _base_make_ioc_operational()
Signed-off-by: Sreekanth Reddy sreekanth.re...@avagotech.com
---
diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 6b2a79e..d81230a 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -92,6 +92,22 @@ static int disable_discovery = -1;
module_param(disable_discovery, int, 0);
MODULE_PARM_DESC(disable_discovery, disable discovery );
+static int dma_mask;
+
+static int
+_base_wait_for_doorbell_int(struct MPT2SAS_ADAPTER *ioc, int timeout,
+int sleep_flag);
+static int
+_base_wait_for_doorbell_ack(struct MPT2SAS_ADAPTER *ioc, int timeout,
+int sleep_flag);
+static int
+_base_wait_for_doorbell_not_used(struct MPT2SAS_ADAPTER *ioc, int timeout,
+ int sleep_flag);
+static int
+_base_handshake_req_reply_wait(struct MPT2SAS_ADAPTER *ioc, int request_bytes,
+u32 *request, int reply_bytes, u16 *reply, int timeout, int sleep_flag);
+static int
+_base_get_ioc_facts(struct MPT2SAS_ADAPTER *ioc, int sleep_flag);
/**
* _scsih_set_fwfault_debug - global setting of ioc-fwfault_debug.
*
@@ -1179,17 +1195,22 @@ static int
_base_config_dma_addressing(struct MPT2SAS_ADAPTER *ioc, struct pci_dev *pdev)
{
struct sysinfo s;
-char *desc = NULL;
+u64 consistent_dma_mask;
+
+if (dma_mask)
+consistent_dma_mask = DMA_BIT_MASK(64);
+else
+consistent_dma_mask = DMA_BIT_MASK(32);
if (sizeof(dma_addr_t) 4) {
const uint64_t required_mask =
dma_get_required_mask(pdev-dev);
if ((required_mask DMA_BIT_MASK(32)) !pci_set_dma_mask(pdev,
DMA_BIT_MASK(64)) !pci_set_consistent_dma_mask(pdev,
-DMA_BIT_MASK(64))) {
+consistent_dma_mask)) {
ioc-base_add_sg_single = _base_add_sg_single_64;
ioc-sge_size = sizeof(Mpi2SGESimple64_t);
-desc = 64;
+dma_mask = 64;
goto out;
}
}
@@ -1198,18 +1219,29 @@ _base_config_dma_addressing(struct
MPT2SAS_ADAPTER *ioc, struct pci_dev *pdev)
!pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32))) {
ioc-base_add_sg_single = _base_add_sg_single_32;
ioc-sge_size = sizeof(Mpi2SGESimple32_t);
-desc = 32;
+dma_mask = 32;
} else
return -ENODEV;
out:
si_meminfo(s);
-printk(MPT2SAS_INFO_FMT %s BIT PCI BUS DMA ADDRESSING SUPPORTED,
-total mem (%ld kB)\n, ioc-name, desc, convert_to_kb(s.totalram));
+printk(MPT2SAS_INFO_FMT
+%d BIT PCI BUS DMA ADDRESSING SUPPORTED, total mem (%ld kB)\n,
+ioc-name, dma_mask, convert_to_kb(s.totalram));
return 0;
}
+static int
+_base_change_consistent_dma_mask(struct MPT2SAS_ADAPTER *ioc,
+ struct pci_dev *pdev)
+{
+if (pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64))) {
+if (pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32)))
+return -ENODEV;
+}
+return 0;
+}
/**
* _base_check_enable_msix - checks MSIX capabable.
* @ioc: per adapter object
@@ -1424,6 +1456,9 @@ _base_enable_msix(struct MPT2SAS_ADAPTER *ioc)
ioc-reply_queue_count = min_t(int, ioc-cpu_count,
ioc-msix_vector_count);
+if (!ioc-rdpq_array_enable max_msix_vectors == -1)
+max_msix_vectors = 8;
+
if (max_msix_vectors 0) {
ioc-reply_queue_count = min_t(int, max_msix_vectors,
ioc-reply_queue_count);
@@ -1552,6 +1587,16 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER *ioc)
}
_base_mask_interrupts(ioc);
+
+r = _base_get_ioc_facts(ioc, CAN_SLEEP);
+if (r)
+goto out_fail;
+
+if (!ioc-rdpq_array_enable_assigned) {
+ioc-rdpq_array_enable =
Re: [RFC] hpsa: work in progress lockless monster patches
On 07/25/2014 09:28 PM, scame...@beardog.cce.hp.com wrote:
hpsa: Work In Progress: lockless monster patches
To be clear, I am not suggesting that these patches be merged at this time.
This patchset is vs. Christoph Hellwig's scsi-mq.4 branch which
may be found here: git://git.infradead.org/users/hch/scsi.git
We've been working for a long time on a patchset for hpsa to remove
all the locks from the main i/o path in pursuit of high IOPS. Some
of that work is already upstream, but a lot more of it is not quite
yet ready to be merged. However, I think we've gone dark for a bit
too long on this, and even though the patches aren't really ready to
be merged just yet, I thought I should let other people who might be
interested have a look anyway, as things are starting to be at least
more stable than unstable. Be warned though, there are still some
problems, esp. around error recovery.
That being said, with the right hardware (fast SAS SSDs, recent Smart
Arrays e.g. P430, with up-to-date firmware, attached to recent disk
enclosures)
with these patches and the scsi-mq patches, it is possible to get around
~970k IOPS from a single logical drive on a single controller.
There are about 150 patches in this set. Rather than bomb the list
with that, here is a link to a tarball of the patches in the form of
an stgit patch series:
Hi Stephen,
I've looked only at the alloc functions - I'm not sure if I got it right,
it seems it uses the same cmd_pool for both alloc function,
from (0 - reserved_cmds) it's cmd_alloc and above that it's handled
by cmd_tagged_alloc.
The logic in both functions seems to be valid for me.
In cmd_tagged_alloc Thus, there should never be a collision here between
two requests if this is true you don't need the refcount and just a simple
flag were sufficient for your other needs. (And maybe you get to ~971k IOPS..)
cmd_alloc some minor changes are possible
- try to find free entries only to reserved_cmds
(the bitmap might be shorter too)
- next thread should start + 1 from the last result
- when nr_cmds is read from hw it should be tested (nr_cmds reserved_cmds)
- what is the correct pronunciation of benignhly?
@@ -5634,7 +5634,8 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h)
offset = h-last_allocation; /* benighly racy */
for (;;) {
- i = find_next_zero_bit(h-cmd_pool_bits, h-nr_cmds, offset);
+ offset %= h-reserved_cmds;
+ i = find_next_zero_bit(h-cmd_pool_bits, h-reserved_cmds,
offset);
if (unlikely(i = h-reserved_cmds)) {
offset = 0;
continue;
@@ -5642,15 +5643,16 @@ static struct CommandList *cmd_alloc(struct ctlr_info
*h)
c = h-cmd_pool + i;
refcount = atomic_inc_return(c-refcount);
if (unlikely(refcount 1)) {
- cmd_free(h, c); /* already in use */
- offset = (i + 1) % h-nr_cmds;
+ atomic_dec(c-refcount); /* already in use, since we
+don't own the command just decrease the refcount */
+ offset = i + 1;
continue;
}
set_bit(i (BITS_PER_LONG - 1),
h-cmd_pool_bits + (i / BITS_PER_LONG));
break; /* it's ours now. */
}
- h-last_allocation = i; /* benignly racy */
+ h-last_allocation = i + 1; /* benignly racy */
hpsa_cmd_partial_init(h, i, c);
return c;
}
@@ -5670,6 +5672,7 @@ static void cmd_free(struct ctlr_info *h, struct
CommandList *c)
clear_bit(i (BITS_PER_LONG - 1),
h-cmd_pool_bits + (i / BITS_PER_LONG));
}
+ else ; /*BUG ? or just atomic_dec and no if */
}
And please have a look at [PATCH] hpsa: refine the pci enble/disable handling
I posted in June and add it to your series or review in the list.
Thanks,
Tomas
https://github.com/smcameron/hpsa-lockless-patches-work-in-progress/blob/master/hpsa-lockless-vs-hch-scsi-mq.4-2014-07-25-1415CDT.tar.bz2?raw=true
In some cases, I have probably erred on the side of having too many too
small patches, on the theory that it is easier to bake a cake than to
unbake a cake. Before these are submitted for reals, there will be
some squashing of patches, along with other cleaning up.
There are some patches in this set which are already upstream in
James's tree which do not happen to be in Christoph's tree
(most of which are named *_sent_to_james). There are also
quite a few patches which are strictly for debugging and are not
ever intended to be merged.
Here is a list of all the patches sorted by author:
Arnd Bergmann (1):
[SCSI] hpsa: fix non-x86 builds
Christoph Hellwig (1):
reserve block tags in scsi host
Joe Handzik (9):
Re: WARNING: CPU: 1 PID: 495 at mm/slab_common.c:69 kmem_cache_create+0x1a9/0x330()
On Wed, 2014-07-30 at 16:52 +0200, Christoph Hellwig wrote: On Wed, Jul 30, 2014 at 08:21:35AM -0400, Josh Boyer wrote: Thanks, I've update the author, added a Cc to ћtable and pushed it out to the core-for-3.17 branch. This fixes a bug in the 3.16 kernel. Why wouldn't it be sent to Linus for inclusion in the final release there? I'm only collecting patches for scsi, but James remains formal maintainer, so I don't send anything to Linus. Given that delays between me committing things, them getting picked up by James and finally making it to Linux-next I don't feel like another for-3.16 branch at this point is easily workable. It's been remarkably current, I believe ... it's already up to date. However, we only have 2 -next builds between now and the anticipated 3.16 release (unless Linus does another -rc) so there's not enough time for a patch that isn't already in (although this one is). If James wants to cherry pick it and send it on at this time that might still work fine, but he seems fairly busy. I'll pull it out and refactor the tree. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] brd: Fix brd_direct_access with partitions
On Wed, Jul 30, 2014 at 05:18:47PM +0300, Boaz Harrosh wrote: When brd_direct_access() is called on a partition-bdev it would access the wrong sector. And caller would then corrupt the device's data. This is a preliminary fix, Matthew Wilcox has a patch in his DAX patchset which will define a global wrapper to bdev-bd_disk-fops-direct_access that will do the proper checks and translations before calling a driver global member. (The way it is done at the rest of the block stack) Uh, no, let's just focus on getting the DAX code in instead of putting in interim patches that will conflict. Patch 4/22 is uncontroversial, fixes this problem, has no dependencies, and is key to the rest of the DAX patchset. If this problem wants to be fixed, then put 4/22 in instead. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] brd: Fix brd_direct_access with partitions
On 07/30/2014 06:34 PM, Matthew Wilcox wrote: On Wed, Jul 30, 2014 at 05:18:47PM +0300, Boaz Harrosh wrote: When brd_direct_access() is called on a partition-bdev it would access the wrong sector. And caller would then corrupt the device's data. This is a preliminary fix, Matthew Wilcox has a patch in his DAX patchset which will define a global wrapper to bdev-bd_disk-fops-direct_access that will do the proper checks and translations before calling a driver global member. (The way it is done at the rest of the block stack) Uh, no, let's just focus on getting the DAX code in instead of putting in interim patches that will conflict. Patch 4/22 is uncontroversial, fixes this problem, has no dependencies, and is key to the rest of the DAX patchset. If this problem wants to be fixed, then put 4/22 in instead. OK, I agree, could you please push 4/22 as reply to here for Jens to take for 3.17 ? (together with my 1/2) Thanks Boaz -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] brd: Fix the partitions BUG
On Wed, 2014-07-30 at 17:15 +0300, Boaz Harrosh wrote:
With current code after a call to:
bdev = blkdev_get_by_path(dev_name, mode, fs_type);
size = i_size_read(bdev-bd_inode);
part_size = bdev-bd_part-nr_sects 9;
I get the following bad results:
dev_name == /dev/ram0
foo: [foo_mount:880] size=0x4000 bdev=88003dc24340 \
bd_inode=88003dc24430 bd_part=88003ca22848 part_size=0x4000
dev_name == /dev/ram0p1
foo: [foo_mount:880] size=0x4000 bdev=88003d2f6d80 \
bd_inode=88003d2f6e70 bd_part=88003ca22848 part_size=0x4000
dev_name == /dev/ram0p2
foo: [foo_mount:880] size=0x4000 bdev=88003dc24680 \
bd_inode=88003dc24770 bd_part=88003ca22848 part_size=0x4000
Note how all three bdev(s) point to the same bd_part.
This is do to a single bad clubber in brd_probe() which is
removed in this patch:
- *part = 0;
because of this all 3 bdev(s) above get to point to the same bd_part[0]
While at it fix/rename brd_init_one() since all devices are created on
load of driver, brd_probe() will never be called with a new un-created
device.
brd_init_one() is now renamed to brd_find() which is what it does.
TODO: There is one more partitions BUG regarding
brd_direct_access() which is fixed on the next patch.
Signed-off-by: Boaz Harrosh b...@plexistor.com
---
drivers/block/brd.c | 19 ---
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index c7d138e..92334f6 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -523,22 +523,20 @@ static void brd_free(struct brd_device *brd)
kfree(brd);
}
-static struct brd_device *brd_init_one(int i)
+static struct brd_device *brd_find(int i)
{
struct brd_device *brd;
list_for_each_entry(brd, brd_devices, brd_list) {
if (brd-brd_number == i)
- goto out;
+ return brd;
}
- brd = brd_alloc(i);
- if (brd) {
- add_disk(brd-brd_disk);
- list_add_tail(brd-brd_list, brd_devices);
- }
-out:
- return brd;
+ /* brd always allocates all its devices at load time, therefor
+ * brd_probe will never be called with a new brd_number
+ */
+ printk(KERN_EROR brd: brd_find unexpected device %d\n, i);
s/KERN_EROR/KERN_ERR/
+ return NULL;
}
static void brd_del_one(struct brd_device *brd)
@@ -554,11 +552,10 @@ static struct kobject *brd_probe(dev_t dev, int *part,
void *data)
struct kobject *kobj;
mutex_lock(brd_devices_mutex);
- brd = brd_init_one(MINOR(dev) part_shift);
+ brd = brd_find(MINOR(dev) part_shift);
kobj = brd ? get_disk(brd-brd_disk) : NULL;
mutex_unlock(brd_devices_mutex);
- *part = 0;
return kobj;
}
It is possible to create new block devices with BRD at runtime:
# mknod /dev/new_brd b 1 4
# fdisk -l /dev/new_brd
This causes a new BRD disk to be created, and hits your error case:
Jul 30 10:40:57 alara kernel: brd: brd_find unexpected device 4
I guess in general I'm not saying that BRD needs to have partitions - indeed
it may not give you much in the way of added functionality. As the code
currently stands partitions aren't surfaced anyway
(GENHD_FL_SUPPRESS_PARTITION_INFO is set). For PRD, however, I *do* want to
enable partitions correctly because eventually I'd like to enhance PRD so that
it *does* actually handle NVDIMMs correctly, and for that partitions do make
sense. And if I have to implement and debug partitions for PRD, it's easy to
stick them in BRD in case anyone wants to use them.
- Ross
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WARNING: CPU: 1 PID: 495 at mm/slab_common.c:69 kmem_cache_create+0x1a9/0x330()
On 07/26/2014 11:21 AM, Christoph Hellwig wrote:
Here's a formal one.
James, can I get your signoff for it?
Vladimir, can I get a reviewed-by from you (or anyone else)?
---
From 73b1034ab1418e2dea75ccf642bc85c728b57313 Mon Sep 17 00:00:00 2001
From: James Bottomley james.bottom...@hansenpartnership.com
Date: Sat, 26 Jul 2014 12:21:26 -0400
Subject: scsi: use short driver name for per-driver cmd slab caches
hostt-name might contain space, so use the -proc_name short name instead
when creating per-driver command slabs.
Reported-by: Vladimir Davydov vdavy...@parallels.com
Tested-by: Vladimir Davydov vdavy...@parallels.com
---
drivers/scsi/scsi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 33318f5..df33060 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -365,8 +365,8 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
if (!pool)
return NULL;
- pool-cmd_name = kasprintf(GFP_KERNEL, %s_cmd, hostt-name);
- pool-sense_name = kasprintf(GFP_KERNEL, %s_sense, hostt-name);
+ pool-cmd_name = kasprintf(GFP_KERNEL, %s_cmd, hostt-proc_name);
+ pool-sense_name = kasprintf(GFP_KERNEL, %s_sense, hostt-proc_name);
if (!pool-cmd_name || !pool-sense_name) {
scsi_free_host_cmd_pool(pool);
return NULL;
Some drivers like qla2xxx do not set proc_name. I think if 2 drivers
like that are loaded then you will hit some other warns/bugs in the kmem
cache setup code right?
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WARNING: CPU: 1 PID: 495 at mm/slab_common.c:69 kmem_cache_create+0x1a9/0x330()
On Wed, Jul 30, 2014 at 12:22:11PM -0500, Mike Christie wrote: Some drivers like qla2xxx do not set proc_name. I think if 2 drivers like that are loaded then you will hit some other warns/bugs in the kmem cache setup code right? Drivers have to opt into using their own caches by setting .cmd_size in the host template. We could enforce they set -proc_name for them, but I'd rather keep the amount of sanity checks low unless there's a real need. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] brd: Fix the partitions BUG
On 07/30/2014 07:50 PM, Ross Zwisler wrote: + */ +printk(KERN_EROR brd: brd_find unexpected device %d\n, i); s/KERN_EROR/KERN_ERR/ Yes thanks, sigh, code should compile driver error. I used pr_err but last inspection I saw that printk is used everywhere and, crapped ... +return NULL; } static void brd_del_one(struct brd_device *brd) @@ -554,11 +552,10 @@ static struct kobject *brd_probe(dev_t dev, int *part, void *data) struct kobject *kobj; mutex_lock(brd_devices_mutex); -brd = brd_init_one(MINOR(dev) part_shift); +brd = brd_find(MINOR(dev) part_shift); kobj = brd ? get_disk(brd-brd_disk) : NULL; mutex_unlock(brd_devices_mutex); -*part = 0; return kobj; } It is possible to create new block devices with BRD at runtime: # mknod /dev/new_brd b 1 4 # fdisk -l /dev/new_brd This causes a new BRD disk to be created, and hits your error case: Jul 30 10:40:57 alara kernel: brd: brd_find unexpected device 4 Ha OK, So this is the mystery key. I never knew that trick, OK I guess I need to leave this in I guess in general I'm not saying that BRD needs to have partitions - indeed it may not give you much in the way of added functionality. As the code currently stands partitions aren't surfaced anyway (GENHD_FL_SUPPRESS_PARTITION_INFO is set). For PRD, however, I *do* want to enable partitions correctly because eventually I'd like to enhance PRD so that it *does* actually handle NVDIMMs correctly, and for that partitions do make sense. So lets talk about that for a bit. Why would you want legacy partitions for NVDIMMs. For one fdisk will waste 1M of memory on this. And with NVDIMMs you actually need a different manager all together, more like lvm stuff. I have patches here that changes prd a lot. - Global memory parameters are gone each device remaps and operates on it's own memory range. - the prd_params are gone instead you have one memmap= parameter of the form memmap=nnn$ooo,[nnn$ooo]... where: nnn - is size in bytes of the form number[K/M/G] ooo - is offset in bytes of the form number[K/M/G] Very much the same as the memmap= Kernel parameters. So just copy/paste what you did there and it will work. Now each memory range has one prd_device created for it. Note that if specified ranges are just contiguous then it is like your prd_count thing where you took a contiguous range and sliced it up, only here they can be of different size. Off course it has an added fixture of dis-contiguous ranges like we have in our multy-nodes NUMA system in the lab that host DDR3 NvDIMMs in two banks. - A sysfs interface is added to add/remove memory range on the fly like osdblk - If no parameters are specified at all, the Kernel command-line is parsed and all memmap= sections are attempted and used if are not already claimed. An interface like mknod above is not supported since it is actually pointless. My current code still supports partitions but I still think it is silly. [ This is all speculative for DDR3-NVDIMMs, we have seen that the memory controller actually tags these DIMMs with type 12 but it looks like this is all vendor specific right now and I understand that DDR4 standardize all that. So I was hoping you guys are working on all that with the NvDIMM stuff. Now lets say that I have established auto-detection for each DIMM and have extracted its SN (Our DDR3 DIMMs each have an SN that can be displayed by the vendor supplied tool) An NvDIMM manager will need to establish an NvDIMM table and set order. The motivation of a partition table is that a disk moved to another machine and/or booted into another OS will have persistent and STD way to not clobber over established DATA. But here we have like a disk cluster/raid, an ordered set of drives. Unique to NvDIMMs is the interleaving. If pairs are then inserted in wrong order or are re-mixed. This should be detected and refused to be mounted (unless a re-initialize is forced) So data is not silently lost on operator errors. All this is then persisted on some DIMM, first or last. If code is able to re-arrange order, like when physical address have changed it will, but in the wrong interleaving case it will refuse to mount. ] And if I have to implement and debug partitions for PRD, it's easy to stick them in BRD in case anyone wants to use them. I was finally playing with BRD, and it is missing your getgeo patch, so it is currently completely alien to fdisk and partitions. So why, why keep a fixture that never existed, I still hope to convince you that this is crap. Specially with brd that can have devices added on the fly like you showed above. Who needs them at all? Why waist 1M of memory on each device for no extra gain? Specially in light of my new prd that does away of any needs of a partitioning since it supports arbitrary slicing in another way. -
Re: [RFC] hpsa: work in progress lockless monster patches
Hi Tomas,
Thanks for taking a look and for the feedback. I'm actually the one
responsible for the changes you refer to, so I'll try to address your
comments.
On 7/30/14 10:55 AM, Tomas Henzl wrote:
I'm not sure if I got it right,
it seems it uses the same cmd_pool for both alloc function,
from (0 - reserved_cmds) it's cmd_alloc and above that it's handled
by cmd_tagged_alloc.
The logic in both functions seems to be valid for me.
Good. With Christoph's proposed changes, the block layer will select a
tag for I/O requests, and it provides for the driver to reserve part of
the tag space for its own use. Thus, HPSA uses a common pool for both
I/O requests and internally-driven requests (IOCTLs, aborts, etc.), and
we index the pool using the tag. The code is this way mostly to
minimize change from the previous version, but it's also handy in terms
of managing the pool (i.e., there is a single pool to administer,
instead of two, and all the items are initialized the same way, etc.).
The portion of the pool which is reserved to the driver (the
low-numbered tags) continues to be managed through a bitmap, just as it
was prior to this change; the bitmap does cover the whole pool (which is
a historical artifact), but the higher bits are never set, since
allocation of those entries is determined by the tag from the block
layer -- nevertheless, it is convenient to have the map extend over the
whole pool in certain corner cases (such as resets).
In cmd_tagged_alloc Thus, there should never be a collision here between
two requests if this is true you don't need the refcount and just a simple
flag were sufficient for your other needs. (And maybe you get to ~971k IOPS..)
:-) The code previously had the reference count in there to close
certain race conditions' asynchronous accesses to the command block
(e.g., between an abort and a completing command). We hope that, using
the block layer tags, those races no longer occur, but there didn't seem
to be any point in removing the reference count until we'd had more
experience with the code...and, in a fit of healthy paranoia I added the
check to which you refer. Unfortunately, in some of Rob Elliott's
recent tests, he managed to drive a case where the check triggers. So,
until we reconcile that, I'm inclined to leave the check in place
(hopefully it's not costing a full 1k IOPS :-) ).
cmd_alloc some minor changes are possible
- try to find free entries only to reserved_cmds
(the bitmap might be shorter too)
Excellent catch! I'll make that change. (As I said, we currently rely
on the over-sized bit map, but there is no reason for this code to
search the whole thing...although, with the block layer doing the heavy
lifting, this allocation path is lightly used, and it will be rare that
it runs off the end of the reserved range.)
- next thread should start + 1 from the last result
That's a fair point, but the suggestion is somewhat more complicated
than it seems: if start + 1 is greater than reserved_cmds, then we
want to start at zero, instead. I think it ends up being more code than
it's worth.
In fact, with the partitioned pool, the reserved-to-driver section is
now so small that it's probably not worth the effort to try to resume
the search where the last search left off -- we might as well start at
zero each time. (Thanks for bringing that up! ;-) )
- when nr_cmds is read from hw it should be tested (nr_cmds reserved_cmds)
That's a fair point.
- what is the correct pronunciation of benignhly?
You would have to ask in certain parts of Boston or perhaps out on the
South Fork of Long Island. :-D (I thought I fixed those typos -- I'm
_sure_ I did! -- and yet)
@@ -5634,7 +5634,8 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h)
offset = h-last_allocation; /* benighly racy */
for (;;) {
- i = find_next_zero_bit(h-cmd_pool_bits, h-nr_cmds, offset);
+ offset %= h-reserved_cmds;
+ i = find_next_zero_bit(h-cmd_pool_bits, h-reserved_cmds,
offset);
Aside from changing the limit from nr_cmds to reserved_cmds, I don't
think this change is helpful, because it inserts a modulo operation on
every iteration, which is probably unnecessary. (It would be better to
ensure that offset is a reasonable value to begin with, and, even it is
not, the call to find_next_zero_bit() will handle it properly and then
the code will retry.)
@@ -5642,15 +5643,16 @@ static struct CommandList *cmd_alloc(struct ctlr_info
*h)
c = h-cmd_pool + i;
refcount = atomic_inc_return(c-refcount);
if (unlikely(refcount 1)) {
- cmd_free(h, c); /* already in use */
- offset = (i + 1) % h-nr_cmds;
+ atomic_dec(c-refcount); /* already in use, since we
+don't own the command just decrease the refcount */
+
[PATCH] scsi: ch.c: Cleaning up missing null-terminate in conjunction with strncpy
If you are going to use memset before strncpy you must copy sizeof -1.
And removed unnecessary magic numbers.
Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
---
drivers/scsi/ch.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index 2a32374..1fecf60 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -634,19 +634,23 @@ static long ch_ioctl(struct file *file,
memset(vparams,0,sizeof(vparams));
if (ch-counts[CHET_V1]) {
vparams.cvp_n1 = ch-counts[CHET_V1];
- strncpy(vparams.cvp_label1,vendor_labels[0],16);
+ strncpy(vparams.cvp_label1, vendor_labels[0],
+ sizeof(vparams.cvp_label1) - 1);
}
if (ch-counts[CHET_V2]) {
vparams.cvp_n2 = ch-counts[CHET_V2];
- strncpy(vparams.cvp_label2,vendor_labels[1],16);
+ strncpy(vparams.cvp_label2, vendor_labels[1],
+ sizeof(vparams.cvp_label2) - 1);
}
if (ch-counts[CHET_V3]) {
vparams.cvp_n3 = ch-counts[CHET_V3];
- strncpy(vparams.cvp_label3,vendor_labels[2],16);
+ strncpy(vparams.cvp_label3, vendor_labels[2],
+ sizeof(vparams.cvp_label3) - 1);
}
if (ch-counts[CHET_V4]) {
vparams.cvp_n4 = ch-counts[CHET_V4];
- strncpy(vparams.cvp_label4,vendor_labels[3],16);
+ strncpy(vparams.cvp_label4, vendor_labels[3],
+ sizeof(vparams.cvp_label4) - 1);
}
if (copy_to_user(argp, vparams, sizeof(vparams)))
return -EFAULT;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi: csiostor: csio_init.c: Cleaning up missing null-terminate in conjunction with strncpy
If you use kzalloc before strncpy you must copy sizeof -1. And removed unnecessary magic numbers. Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se --- drivers/scsi/csiostor/csio_init.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/csiostor/csio_init.c b/drivers/scsi/csiostor/csio_init.c index 1aafc33..d5ea81c 100644 --- a/drivers/scsi/csiostor/csio_init.c +++ b/drivers/scsi/csiostor/csio_init.c @@ -532,7 +532,8 @@ static struct csio_hw *csio_hw_alloc(struct pci_dev *pdev) goto err; hw-pdev = pdev; - strncpy(hw-drv_version, CSIO_DRV_VERSION, 32); + strncpy(hw-drv_version, CSIO_DRV_VERSION, + sizeof(hw-drv_version) - 1); /* memory pool/DMA pool allocation */ if (csio_resource_alloc(hw)) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi: hpsa.c: Cleaning up code clarification using strlcpy
Code clarification using strlcpy instead of strncpy.
And removed unnecessary memset
Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
---
drivers/scsi/hpsa.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 31184b3..814d64d 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -315,16 +315,15 @@ static ssize_t host_store_hp_ssd_smart_path_status(struct
device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
- int status, len;
+ int status;
struct ctlr_info *h;
struct Scsi_Host *shost = class_to_shost(dev);
char tmpbuf[10];
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
return -EACCES;
- len = count sizeof(tmpbuf) - 1 ? sizeof(tmpbuf) - 1 : count;
- strncpy(tmpbuf, buf, len);
- tmpbuf[len] = '\0';
+ strlcpy(tmpbuf, buf,
+ count sizeof(tmpbuf) ? sizeof(tmpbuf) : count);
if (sscanf(tmpbuf, %d, status) != 1)
return -EINVAL;
h = shost_to_hba(shost);
@@ -339,16 +338,15 @@ static ssize_t host_store_raid_offload_debug(struct
device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
- int debug_level, len;
+ int debug_level;
struct ctlr_info *h;
struct Scsi_Host *shost = class_to_shost(dev);
char tmpbuf[10];
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
return -EACCES;
- len = count sizeof(tmpbuf) - 1 ? sizeof(tmpbuf) - 1 : count;
- strncpy(tmpbuf, buf, len);
- tmpbuf[len] = '\0';
+ strlcpy(tmpbuf, buf,
+ count sizeof(tmpbuf) ? sizeof(tmpbuf) : count);
if (sscanf(tmpbuf, %d, debug_level) != 1)
return -EINVAL;
if (debug_level 0)
@@ -5881,8 +5879,8 @@ static int hpsa_controller_hard_reset(struct pci_dev
*pdev,
static void init_driver_version(char *driver_version, int len)
{
- memset(driver_version, 0, len);
strncpy(driver_version, HPSA HPSA_DRIVER_VERSION, len - 1);
+ driver_version[len - 1] = '\0';
}
static int write_driver_ver_to_cfgtable(struct CfgTable __iomem *cfgtable)
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] driver core: enable drivers to use deferred probe from init
From: Luis R. Rodriguez mcg...@do-not-panic.com Date: Mon, 28 Jul 2014 11:28:28 -0700 Tetsuo bisected and found that commit 786235ee kthread: make kthread_create() killable modified kthread_create() to bail as soon as SIGKILL is received. This is causing some issues with some drivers and at times boot. Joseph then found that failures occur as the systemd-udevd process sends SIGKILL to modprobe if probe on a driver takes over 30 seconds. When this happens probe will fail on any driver, its why booting on some system will fail if the driver happens to be a storage related driver. Some folks have suggested fixing this by modifying kthread_create() to not leave upon SIGKILL [3], upon review Oleg rejected this change and the discussion was punted out to systemd to see if the default timeout could be increased from 30 seconds to 120. The opinion of the systemd maintainers is that the driver's behavior should be fixed [4]. Linus seems to agree [5], however more recently even networking drivers have been reported to fail on probe since just writing the firmware to a device and kicking it can take easy over 60 seconds [6]. Benjamim was able to trace the issues recently reported on cxgb4 down to the same systemd-udevd 30 second timeout [6]. This is an alternative solution which enables drivers that are known to take long to use deferred probe workqueue. This avoids the 30 second timeout and lets us annotate drivers with long init sequences. As drivers determine a component is not yet available and needs to defer probe you'll be notified this happen upon init for each device but now with a message such as: pci :03:00.0: Driver cxgb4 requests probe deferral on init You should see one of these per struct device probed. It seems we're still discussing this. I think I understand all of the underlying issues, and what I'll say is that perhaps we should use what Greg KH requested but via a helper that is easy to grep for. I don't care if it's something like module_long_probe_init() and module_long_probe_exit(), but it just needs to be some properly named interface which does the whole kthread or whatever bit. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi: ibmvscsi: ibmvfc.c: Cleaning up missing null-terminate in conjunction with strncpy
And as I understand it, it is fine to replace strncpy to strlcpy here. But if any of this code do not want a null terminated, or want to have the whole rest of size filled with null characters. If that is the case this is not a good patch. Rickard Strandqvist (1): scsi: ibmvscsi: ibmvfc.c: Cleaning up missing null-terminate in conjunction with strncpy drivers/scsi/ibmvscsi/ibmvfc.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi: ibmvscsi: ibmvfc.c: Cleaning up missing null-terminate in conjunction with strncpy
Replacing strncpy with strlcpy to avoid strings that lacks null terminate. Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se --- drivers/scsi/ibmvscsi/ibmvfc.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c index 8dd4768..56c26ef 100644 --- a/drivers/scsi/ibmvscsi/ibmvfc.c +++ b/drivers/scsi/ibmvscsi/ibmvfc.c @@ -1149,7 +1149,8 @@ static void ibmvfc_gather_partition_info(struct ibmvfc_host *vhost) name = of_get_property(rootdn, ibm,partition-name, NULL); if (name) - strncpy(vhost-partition_name, name, sizeof(vhost-partition_name)); + strlcpy(vhost-partition_name, name, + sizeof(vhost-partition_name)); num = of_get_property(rootdn, ibm,partition-no, NULL); if (num) vhost-partition_number = *num; @@ -1186,13 +1187,14 @@ static void ibmvfc_set_login_info(struct ibmvfc_host *vhost) login_info-capabilities = IBMVFC_CAN_MIGRATE; login_info-async.va = vhost-async_crq.msg_token; login_info-async.len = vhost-async_crq.size * sizeof(*vhost-async_crq.msgs); - strncpy(login_info-partition_name, vhost-partition_name, IBMVFC_MAX_NAME); - strncpy(login_info-device_name, + strlcpy(login_info-partition_name, vhost-partition_name, + IBMVFC_MAX_NAME); + strlcpy(login_info-device_name, dev_name(vhost-host-shost_gendev), IBMVFC_MAX_NAME); location = of_get_property(of_node, ibm,loc-code, NULL); location = location ? location : dev_name(vhost-dev); - strncpy(login_info-drc_name, location, IBMVFC_MAX_NAME); + strlcpy(login_info-drc_name, location, IBMVFC_MAX_NAME); } /** -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Very Urgent............
Greetings from gulf region Thanks for the e-mail. I am very interested on funding lucrative business partnership with you acting as the manager and sole controller of the investment while i remain a silent investor for a period of ten yrs , though I am only looking at investment opportunities within the range you specified for a start. You can reply me here (fmoris...@yahoo.com) Let me know your thought asap Regards Financial Consultant Mr.Fabian Morision -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
