Re: [PATCH v2 78/71] ncr5380: Add support for HP 53C400A-based cards (C2502)
On Tue, 8 Dec 2015, Ondrej Zary wrote:
> HP C2502 cards (based on 53C400A chips) use different magic numbers for
> software-based I/O address configuration than other cards. The
> configuration is also extended to allow setting the IRQ.
>
> Move the configuration to a new function magic_configure() and move
> magic the magic numbers into an array. Add new magic numbers for these
> HP cards and hp_53c400a module parameter to use them.
>
> Tested with HP C2502 and DTCT-436P.
>
> Signed-off-by: Ondrej Zary
> ---
> drivers/scsi/g_NCR5380.c | 76
> --
> drivers/scsi/g_NCR5380.h |1 +
> 2 files changed, 61 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
> index 42fdeaf..121d143 100644
> --- a/drivers/scsi/g_NCR5380.c
> +++ b/drivers/scsi/g_NCR5380.c
> @@ -80,6 +80,7 @@ static int ncr_5380;
> static int ncr_53c400;
> static int ncr_53c400a;
> static int dtc_3181e;
> +static int hp_53c400a;
>From the photos that I've seen, the HP cards have either NCR or SYMBIOS
53C400A devices.
You've added a new setup option and a new board type, but the name you've
chosen is neither the board type nor the device type. It is a combination.
>
> static struct override {
> NCR5380_map_type NCR5380_map_name;
> @@ -225,6 +226,32 @@ static int __init do_DTC3181E_setup(char *str)
>
> #endif
>
> +#ifndef SCSI_G_NCR5380_MEM
> +/*
> + * Configure I/O address of 53C400A or DTC 3181 by writing magic numbers
> + * to ports 0x779 and 0x379. Two magic number sequences are known:
> + * 1. for generic NCR 53C400A-based cards and DTC436 chips
> + * 2. for HP C2502 card (also based on 53C400A but different decode logic)
Forgive my ignorance of ISA card design, but that suggests that these
magic numbers are only relevant to HP C2502 boards and not SYM 53C400A
devices in general (?) ...
> + */
> +static void magic_configure(int idx, u8 irq, u8 magic[])
> +{
> + u8 cfg = 0;
> +
> + outb(magic[0], 0x779);
> + outb(magic[1], 0x379);
> + outb(magic[2], 0x379);
> + outb(magic[3], 0x379);
> + outb(magic[4], 0x379);
> +
> + /* allowed IRQs for HP 53C400A */
> + if (irq != 2 && irq != 3 && irq != 4 && irq != 5 && irq != 7)
> + irq = 0;
> + if (idx >= 0 && idx <= 7)
> + cfg = 0x80 | idx | (irq << 4);
> + outb(cfg, 0x379);
> +}
> +#endif
> +
> /**
> * generic_NCR5380_detect - look for NCR5380 controllers
> * @tpnt: the scsi template
> @@ -241,8 +268,9 @@ static int __init generic_NCR5380_detect(struct
> scsi_host_template *tpnt)
> static int current_override;
> int count;
> unsigned int *ports;
> + u8 *magic;
> #ifndef SCSI_G_NCR5380_MEM
> - int i;
> + int i, port_idx;
> unsigned long region_size = 16;
> #endif
> static unsigned int __initdata ncr_53c400a_ports[] = {
> @@ -251,6 +279,12 @@ static int __init generic_NCR5380_detect(struct
> scsi_host_template *tpnt)
> static unsigned int __initdata dtc_3181e_ports[] = {
> 0x220, 0x240, 0x280, 0x2a0, 0x2c0, 0x300, 0x320, 0x340, 0
> };
> + static u8 ncr_53c400a_magic[] __initdata = {/* 53C400A & DTC 3181 */
> + 0x59, 0xb9, 0xc5, 0xae, 0xa6
> + };
> + static u8 hp_53c400a_magic[] __initdata = { /* HP C2502 */
> + 0x0f, 0x22, 0xf0, 0x20, 0x80
> + };
... so maybe that should be called 'hp_c2502_magic'?
> int flags;
> struct Scsi_Host *instance;
> struct NCR5380_hostdata *hostdata;
> @@ -273,6 +307,8 @@ static int __init generic_NCR5380_detect(struct
> scsi_host_template *tpnt)
> overrides[0].board = BOARD_NCR53C400A;
> else if (dtc_3181e)
> overrides[0].board = BOARD_DTC3181E;
> + else if (hp_53c400a)
> + overrides[0].board = BOARD_HP53C400A;
> #ifndef SCSI_G_NCR5380_MEM
> if (!current_override && isapnp_present()) {
> struct pnp_dev *dev = NULL;
> @@ -326,10 +362,17 @@ static int __init generic_NCR5380_detect(struct
> scsi_host_template *tpnt)
> case BOARD_NCR53C400A:
> flags = FLAG_NO_DMA_FIXUP;
> ports = ncr_53c400a_ports;
> + magic = ncr_53c400a_magic;
> + break;
> + case BOARD_HP53C400A:
> + flags = FLAG_NO_DMA_FIXUP;
> + ports = ncr_53c400a_ports;
> + magic = hp_53c400a_magic;
> break;
> case BOARD_DTC3181E:
> flags = FLAG_NO_DMA_FIXUP;
> ports = dtc_3181e_ports;
> + magic = ncr_53c400a_magic;
> break;
> }
>
> @@ -338,12 +381,7 @@ static int __init generic_NCR5380_detect(struct
> scsi_host_template *tpnt)
> /* wakeup sequence for the NCR53C400A and DTC3181E
[PATCH RFC 2/2] scsi_proc: Change /proc/scsi/scsi to use bus device iterator
From: "Ewan D. Milne"
This prevents crashing due to accessing a removed element on the list,
the iterator will now hold the correct reference. It was not sufficient
to rely on the klist's reference on the containing device object.
>From a patch originally developed by David Jeffery
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_proc.c | 49
1 file changed, 29 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c
index 251598e..6c1b79d 100644
--- a/drivers/scsi/scsi_proc.c
+++ b/drivers/scsi/scsi_proc.c
@@ -40,6 +40,11 @@
/* 4K page size, but our output routines, use some slack for overruns */
#define PROC_BLOCK_SIZE (3*1024)
+struct scsi_proc_state {
+ struct klist_iter iter;
+ int pos;
+};
+
static struct proc_dir_entry *proc_scsi;
/* Protect sht->present and sht->proc_dir */
@@ -370,47 +375,50 @@ static ssize_t proc_scsi_write(struct file *file, const
char __user *buf,
return err;
}
-static int always_match(struct device *dev, void *data)
-{
- return 1;
-}
-
-static inline struct device *next_scsi_device(struct device *start)
-{
- struct device *next = bus_find_device(&scsi_bus_type, start, NULL,
- always_match);
- put_device(start);
- return next;
-}
-
static void *scsi_seq_start(struct seq_file *sfile, loff_t *pos)
{
+ struct scsi_proc_state *state = sfile->private;
struct device *dev = NULL;
loff_t n = *pos;
+ int err;
+
+ err = bus_device_iter_init(&state->iter, &scsi_bus_type);
+ if (err < 0)
+ return ERR_PTR(err);
- while ((dev = next_scsi_device(dev))) {
+ while ((dev = bus_device_iter_next(&state->iter))) {
if (!n--)
break;
- sfile->private++;
+ put_device(dev);
+ state->pos++;
}
return dev;
}
static void *scsi_seq_next(struct seq_file *sfile, void *v, loff_t *pos)
{
+ struct scsi_proc_state *state = sfile->private;
+
(*pos)++;
- sfile->private++;
- return next_scsi_device(v);
+ put_device(v);
+ state->pos++;
+
+ return bus_device_iter_next(&state->iter);
}
static void scsi_seq_stop(struct seq_file *sfile, void *v)
{
+ struct scsi_proc_state *state = sfile->private;
+
put_device(v);
+ bus_device_iter_exit(&state->iter);
}
static int scsi_seq_show(struct seq_file *sfile, void *dev)
{
- if (!sfile->private)
+ struct scsi_proc_state *state = sfile->private;
+
+ if (!state->pos)
seq_puts(sfile, "Attached devices:\n");
return proc_print_scsidevice(dev, sfile);
@@ -436,7 +444,8 @@ static int proc_scsi_open(struct inode *inode, struct file
*file)
* We don't really need this for the write case but it doesn't
* harm either.
*/
- return seq_open(file, &scsi_seq_ops);
+ return seq_open_private(file, &scsi_seq_ops,
+ sizeof(struct scsi_proc_state));
}
static const struct file_operations proc_scsi_operations = {
@@ -445,7 +454,7 @@ static const struct file_operations proc_scsi_operations = {
.read = seq_read,
.write = proc_scsi_write,
.llseek = seq_lseek,
- .release= seq_release,
+ .release= seq_release_private,
};
/**
--
2.1.0
--
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 RFC 1/2] drivers/base: add bus_device_iter_init, bus_device_iter_next, bus_device_iter_exit
From: "Ewan D. Milne"
These functions are needed to expose an iterator for SCSI usage.
>From a patch originally developed by David Jeffery
Signed-off-by: Ewan D. Milne
---
drivers/base/bus.c | 59 ++
include/linux/device.h | 5 +
2 files changed, 64 insertions(+)
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 5005924..a472e46 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -318,6 +318,65 @@ int bus_for_each_dev(struct bus_type *bus, struct device
*start,
EXPORT_SYMBOL_GPL(bus_for_each_dev);
/**
+ * bus_device_iter_init - Initialize an iterator for walking a bus's devices.
+ * @iter: iterator structure to initialize
+ * @bus: bus type
+ *
+ * Initializes an iterator for safely walking a bus's list of devices. The
+ * iterator can be used by bus_device_iter_next() to safely walk the list, even
+ * if a device is removed from the list while being examined. Needs to be
+ * matched with a call to bus_device_iter_exit() to clean up the iterator when
+ * finished.
+ *
+ * Return 0 on success, non-zero on failure. Iterator cannot be used
+ * for a non-zero result
+ */
+int bus_device_iter_init(struct klist_iter *iter,
+struct bus_type *bus)
+{
+ if (!bus || !bus->p)
+ return -EINVAL;
+
+ klist_iter_init_node(&bus->p->klist_devices, iter, NULL);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(bus_device_iter_init);
+
+/**
+ * bus_device_iter_next - Get a bus's next device from the iterator.
+ * @iter: iterator structure from bus_device_iter_init()
+ *
+ * Finds the next valid device entry on a bus's device list. Allows the list
+ * to be safely traversed by the caller even when other tasks remove devices
+ * from the list.
+ *
+ * Returns a reference to the next device, or NULL if no more devices.
+ */
+struct device *bus_device_iter_next(struct klist_iter *iter)
+{
+ struct device *dev;
+
+ while ((dev = next_device(iter)))
+ if (get_device(dev))
+ break;
+
+ return dev;
+}
+EXPORT_SYMBOL_GPL(bus_device_iter_next);
+
+/**
+ * bus_device_iter_exit - Clean up an iterator from walking a bus's device
list.
+ * @iter: iterator structure from bus_device_iter_init() to clean up
+ *
+ * Clean up any remaining state after finishing walking a bus's device list.
+ */
+void bus_device_iter_exit(struct klist_iter *iter)
+{
+ klist_iter_exit(iter);
+}
+EXPORT_SYMBOL_GPL(bus_device_iter_exit);
+
+/**
* bus_find_device - device iterator for locating a particular device.
* @bus: bus type
* @start: Device to begin with
diff --git a/include/linux/device.h b/include/linux/device.h
index b8f411b..a44d912 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -151,6 +151,11 @@ void subsys_dev_iter_exit(struct subsys_dev_iter *iter);
int bus_for_each_dev(struct bus_type *bus, struct device *start, void *data,
int (*fn)(struct device *dev, void *data));
+
+int bus_device_iter_init(struct klist_iter *iter, struct bus_type *bus);
+struct device *bus_device_iter_next(struct klist_iter *iter);
+void bus_device_iter_exit(struct klist_iter *iter);
+
struct device *bus_find_device(struct bus_type *bus, struct device *start,
void *data,
int (*match)(struct device *dev, void *data));
--
2.1.0
--
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 RFC 0/2] avoid crashing when reading /proc/scsi/scsi and simultaneously removing devices
From: "Ewan D. Milne" The klist traversal used by the reading of /proc/scsi/scsi is not interlocked against device removal. It takes a reference on the containing object, but this does not prevent the device from being removed from the list. Thus, we get errors and eventually panic, as shown in the traces below. Fix this by keeping a klist iterator in the seq_file private data. The problem can be easily reproduced by repeatedly increasing scsi_debug's max_luns to 30 and then deleting the devices via sysfs, while simulatenously accessing /proc/scsi/scsi. >From a patch originally developed by David Jeffery Dec 3 13:22:02 localhost kernel: WARNING: CPU: 2 PID: 28073 at include/linux/kref.h:47 klist_iter_init_node+0x3d/0x50() Dec 3 13:22:02 localhost kernel: Modules linked in: scsi_debug x86_pkg_temp_thermal kvm_intel kvm irqbypass crc32c_intel joydev iTCO_wdt dcdbas ipmi_devintf acpi_power_meter iTCO_vendor_support ipmi_si imsghandler pcspkr wmi acpi_cpufreq tpm_tis tpm shpchp lpc_ich mfd_core nfsd nfs_acl lockd grace sunrpc tg3 ptp pps_core Dec 3 13:22:02 localhost kernel: CPU: 2 PID: 28073 Comm: cat Not tainted 4.4.0-rc1+ #2 Dec 3 13:22:02 localhost kernel: Hardware name: Dell Inc. PowerEdge R320/08VT7V, BIOS 2.0.22 11/19/2013 Dec 3 13:22:02 localhost kernel: 81a20e77 880613acfd18 81321eef Dec 3 13:22:02 localhost kernel: 880613acfd50 8107ca52 88061176b198 Dec 3 13:22:02 localhost kernel: 814542b0 880610cfb100 88061176b198 880613acfd60 Dec 3 13:22:02 localhost kernel: Call Trace: Dec 3 13:22:02 localhost kernel: [] dump_stack+0x44/0x55 Dec 3 13:22:02 localhost kernel: [] warn_slowpath_common+0x82/0xc0 Dec 3 13:22:02 localhost kernel: [] ? proc_scsi_show+0x20/0x20 Dec 3 13:22:02 localhost kernel: [] warn_slowpath_null+0x1a/0x20 Dec 3 13:22:02 localhost kernel: [] klist_iter_init_node+0x3d/0x50 Dec 3 13:22:02 localhost kernel: [] bus_find_device+0x51/0xb0 Dec 3 13:22:02 localhost kernel: [] scsi_seq_next+0x2d/0x40 Dec 3 13:22:02 localhost kernel: [] seq_read+0x290/0x370 Dec 3 13:22:02 localhost kernel: [] proc_reg_read+0x48/0x70 Dec 3 13:22:02 localhost kernel: [] __vfs_read+0x28/0xd0 Dec 3 13:22:02 localhost kernel: [] ? security_file_permission+0xa3/0xc0 Dec 3 13:22:02 localhost kernel: [] ? rw_verify_area+0x53/0xf0 Dec 3 13:22:02 localhost kernel: [] vfs_read+0x86/0x130 Dec 3 13:22:02 localhost kernel: [] SyS_read+0x46/0xa0 Dec 3 13:22:02 localhost kernel: [] entry_SYSCALL_64_fastpath+0x12/0x6a Dec 3 13:22:02 localhost kernel: ---[ end trace 99a60fb1c41fc8c9 ]--- Dec 3 13:22:02 localhost kernel: [ cut here ] Dec 3 13:22:02 localhost kernel: WARNING: CPU: 2 PID: 28073 at lib/klist.c:189 klist_release+0xa8/0xb0() Dec 3 13:22:02 localhost kernel: Modules linked in: scsi_debug x86_pkg_temp_thermal kvm_intel kvm irqbypass crc32c_intel joydev iTCO_wdt dcdbas ipmi_devintf acpi_power_meter iTCO_vendor_support ipmi_si imsghandler pcspkr wmi acpi_cpufreq tpm_tis tpm shpchp lpc_ich mfd_core nfsd nfs_acl lockd grace sunrpc tg3 ptp pps_core Dec 3 13:22:02 localhost kernel: CPU: 2 PID: 28073 Comm: cat Tainted: G W 4.4.0-rc1+ #2 Dec 3 13:22:02 localhost kernel: Hardware name: Dell Inc. PowerEdge R320/08VT7V, BIOS 2.0.22 11/19/2013 Dec 3 13:22:02 localhost kernel: 81aaa040 880613acfcc0 81321eef Dec 3 13:22:02 localhost kernel: 880613acfcf8 8107ca52 dead00f8 880613acfd80 Dec 3 13:22:02 localhost kernel: 88060f7aa368 88060f7aa380 88061176b198 880613acfd08 Dec 3 13:22:02 localhost kernel: Call Trace: Dec 3 13:22:02 localhost kernel: [] dump_stack+0x44/0x55 Dec 3 13:22:02 localhost kernel: [] warn_slowpath_common+0x82/0xc0 Dec 3 13:22:02 localhost kernel: [] warn_slowpath_null+0x1a/0x20 Dec 3 13:22:02 localhost kernel: [] klist_release+0xa8/0xb0 Dec 3 13:22:02 localhost kernel: [] ? bus_uevent_store+0x50/0x50 Dec 3 13:22:02 localhost kernel: [] klist_next+0x95/0xf0 Dec 3 13:22:02 localhost kernel: [] ? proc_scsi_show+0x20/0x20 Dec 3 13:22:02 localhost kernel: [] bus_find_device+0x72/0xb0 Dec 3 13:22:02 localhost kernel: [] scsi_seq_next+0x2d/0x40 Dec 3 13:22:02 localhost kernel: [] seq_read+0x290/0x370 Dec 3 13:22:02 localhost kernel: [] proc_reg_read+0x48/0x70 Dec 3 13:22:02 localhost kernel: [] __vfs_read+0x28/0xd0 Dec 3 13:22:02 localhost kernel: [] ? security_file_permission+0xa3/0xc0 Dec 3 13:22:02 localhost kernel: [] ? rw_verify_area+0x53/0xf0 Dec 3 13:22:02 localhost kernel: [] vfs_read+0x86/0x130 Dec 3 13:22:02 localhost kernel: [] SyS_read+0x46/0xa0 Dec 3 13:22:02 localhost kernel: [] entry_SYSCALL_64_fastpath+0x12/0x6a Dec 3 13:22:02 localhost kernel: ---[ end trace 99a60fb1c41fc8ca ]--- Dec 3 13:22:02 localhost kernel: [ cut here ] Dec 3 13:22:02 localhost kernel: WARNING: CPU: 2 PID: 28073 a
[PATCH] [SCSI] osd: fix signed char versus %02x issue
If char is signed and one of these bytes happen to have a value
outside the ascii range, the corresponding output will consist of
"ff" followed by the two hex chars that were actually
intended. One way to fix it would be to change the casts to (u8*) aka
(unsigned char*), but it is much simpler (and generates smaller code)
to use the %ph extension which was created for such short hexdumps.
Signed-off-by: Rasmus Villemoes
---
drivers/scsi/osd/osd_initiator.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 0cccd6033feb..d8a2b5185f56 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -170,10 +170,7 @@ static int _osd_get_print_system_info(struct osd_dev *od,
/* FIXME: Where are the time utilities */
pFirst = get_attrs[a++].val_ptr;
- OSD_INFO("CLOCK [0x%02x%02x%02x%02x%02x%02x]\n",
- ((char *)pFirst)[0], ((char *)pFirst)[1],
- ((char *)pFirst)[2], ((char *)pFirst)[3],
- ((char *)pFirst)[4], ((char *)pFirst)[5]);
+ OSD_INFO("CLOCK [0x%6phN]\n", pFirst);
if (a < nelem) { /* IBM-OSD-SIM bug, Might not have it */
unsigned len = get_attrs[a].len;
--
2.6.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 00/20] ALUA device handler update, part II
On 12/08/2015 08:37 AM, Hannes Reinecke wrote: Hi all, as promised here is now the second part of my ALUA device handler update. This contains a major rework of the ALUA device handler as execution is moved onto a workqueue. This has the advantage that we avoid having to do multiple calls to the same LUN (as happens frequently when failing over a LUN with several paths) and finally retries are handled correctly. As some arrays are only capable of handling one STPG at a time I've added a module parameter 'sync_stpg' which switches to a singlethreaded workqueue, thereby effectively synchronize STPG handling. Thanks to Bart for this suggestion. As usual, comments and reviews are welcome. As I've been reminded a git reference would be welcome for review. So you can find the entire patchset at git://git.kernel.org/pub/scm/linux/kernel/git/hare/scsi-devel.git branch alua.v8 Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (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 02/20] qla2xxx: Enable Exchange offload support.
On 12/08/2015 01:48 AM, Himanshu Madhani wrote: > Signed-off-by: Himanshu Madhani > Signed-off-by: Giridhar Malavali > --- Same here. Please add a patch description. 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 01/20] qla2xxx: Enable Extended Login support
On 12/08/2015 01:48 AM, Himanshu Madhani wrote: > Signed-off-by: Himanshu Madhani > Signed-off-by: Giridhar Malavali > --- A patch description would be nice; ATM it doesn't look as if this interface is used anywhere... 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 03/20] qla2xxx: Enable Target counters in DebugFS.
On 12/08/2015 01:48 AM, Himanshu Madhani wrote: > Following counters are added in target mode to help debugging efforts. > > Target Counters > > qla_core_sbt_cmd = 0 > qla_core_ret_sta_ctio = 0 > qla_core_ret_ctio = 0 > core_qla_que_buf = 0 > core_qla_snd_status = 0 > core_qla_free_cmd = 0 > num alloc iocb failed = 0 > num term exchange sent = 0 > num Q full sent = 0 > > Signed-off-by: Himanshu Madhani > Signed-off-by: Giridhar Malavali > --- Reviewed-by: Hannes Reinecke 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 05/20] qla2xxx: Added interface to send ELS commands from driver.
On 12/08/2015 01:48 AM, Himanshu Madhani wrote: > Signed-off-by: Himanshu Madhani > Signed-off-by: Giridhar Malavali > --- > drivers/scsi/qla2xxx/qla_attr.c | 36 +++ > drivers/scsi/qla2xxx/qla_dbg.c|5 +- > drivers/scsi/qla2xxx/qla_def.h| 19 - > drivers/scsi/qla2xxx/qla_gbl.h|2 + > drivers/scsi/qla2xxx/qla_inline.h |2 + > drivers/scsi/qla2xxx/qla_iocb.c | 189 > + > drivers/scsi/qla2xxx/qla_isr.c|6 + > 7 files changed, 255 insertions(+), 4 deletions(-) > Well, this isn't actually an interface to send generic ELS commands, it's an interface to send an explicit LOGO using ELS. Personally, I'd prefer to have a 'real' ELS interface using bsg, and map the explicit LOGO based on that. Alternatively please fixup the description. 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 04/20] qla2xxx: Add FW resource count in DebugFS.
On 12/08/2015 01:48 AM, Himanshu Madhani wrote: > From: Quinn Tran > > DebugFS now will show fw_resource_count node. > > FW Resource count > > Original TGT exchg count[0] > current TGT exchg count[0] > original Initiator Exchange count[2048] > Current Initiator Exchange count[2048] > Original IOCB count[2078] > Current IOCB count[2067] > MAX VP count[254] > MAX FCF count[0] > > Signed-off-by: Quinn Tran > Signed-off-by: Himanshu Madhani > --- Reviewed-by: Hannes Reinecke 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 06/20] qla2xxx: Delete session if initiator is gone from FW
On 12/08/2015 01:48 AM, Himanshu Madhani wrote: > From: Alexei Potashnik > > 1. Initiator A is logged in with fc_id(1)/loop_id(1) > 2. Initiator A re-logs in with fc_id(2)/loop_id(2) > 3. Part of old session deletion async logoout for 1/1 is queued > 4. Initiator B logs in with fc_id(1)/loop_id(1), starts >passing data and creates session. > 5. Async logo from 3 is processed by DPC and sent to FW > > Now initiator B has the session but is logged out from FW. > > This condition is detected first with CTIO error 29 at which > point we should delete current session. During session > deletion we will send LOGO to initiator to force re-login. > > Under rare circumstances initiator might be logged out of FW, > not have driver session, but still think it's logged in. > E.g. the above sequence plus session deletion due to re-config. > Incoming commands will fail to create local session because > initiator is not found in FW. In this case we also issue LOGO > to initiator to force him re-login. > > Finally this patch fixes exchange leak when commands where > received in logged out state. In this case loop_id must be > set to when corresponding exchange is terminated. The > patch modifies exchange termination to always use , > since in certain scenarios it's impossible to tell whether > command was received in logged in or logged out state. > > Signed-off-by: Alexei Potashnik > Acked-by: Quinn Tran > Signed-off-by: Himanshu Madhani > --- Reviewed-by: Hannes Reinecke 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 07/20] qla2xxx: Wait for all conflicts before ack'ing PLOGI
On 12/08/2015 01:48 AM, Himanshu Madhani wrote: > From: Alexei Potashnik > > Until now ack'ing of a new PLOGI has only been delayed if there > was an existing session for the same WWN. Ack was released when > the session deletion completed. > > If there was another WWN session with the same fc_id/loop_id pair > (aka "conflicting session"), PLOGI was still ack'ed immediately. > This potentially caused a problem when old session deletion logged > fc_id/loop_id out of FW after new session has been established. > > Two work-arounds were attempted before: > 1. Dropping PLOGIs until conflicting session goes away. > 2. Detecting initiator being logged out of FW and issuing LOGO > to force re-login. > > This patch introduces proper solution to the problem where PLOGI > is held until either existing session with same WWN or any > conflicting session goes away. Mechanism supports one session holding > two PLOGI acks as well as one PLOGI ack being held by many sessions. > > Signed-off-by: Alexei Potashnik > Acked-by: Quinn Tran > Signed-off-by: Himanshu Madhani > --- Reviewed-by: Hannes Reinecke 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 08/20] qla2xxx: Replace QLA_TGT_STATE_ABORTED with a bit.
On 12/08/2015 01:48 AM, Himanshu Madhani wrote: > From: Quinn Tran > > Replace QLA_TGT_STATE_ABORTED state with a bit because > the current state of the command is lost when an abort > is requested by upper layer. > > Signed-off-by: Quinn Tran > Signed-off-by: Himanshu Madhani > --- Reviewed-by: Hannes Reinecke 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: [Bug 108771] scsi: ses: kasan: ses_enclosure_data_process use after free on boot SAS2X28
On Mon, 2015-12-07 at 14:01 +, bugzilla-dae...@bugzilla.kernel.org wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=108771 > > --- Comment #1 from Pavel Tikhomirov --- > Aditional info about enclosue(from that node, but older 3.10 based kernel): > > [root@p9 crash]# modprobe sg > [root@p9 crash]# sg_map -i > /dev/sg0 LSI SAS2X28 0e12 > /dev/sg1 /dev/sda LSI MR9260-4i 2.13 > [root@p9 crash]# lsscsi -gs > [1:0:16:0] enclosu LSI SAS2X28 0e12 - /dev/sg0 > > - > [1:2:0:0]diskLSI MR9260-4i2.13 /dev/sda /dev/sg1 > 3.99TB > [root@p9 crash]# sg_ses /dev/sg0 > LSI SAS2X28 0e12 > Supported diagnostic pages: > Supported Diagnostic Pages [sdp] [0x0] > Configuration (SES) [cf] [0x1] > Enclosure Status/Control (SES) [ec,es] [0x2] > Element Descriptor (SES) [ed] [0x7] > Additional Element Status (SES-2) [aes] [0xa] > Download Microcode (SES-2) [dm] [0xe] > [root@p9 crash]# sg_ses /dev/sg1 > LSI MR9260-4i 2.13 > disk device (not an enclosure) > Supported diagnostic pages: OK, can you give us the contents of pages 1, 2 and 10 with sg_ses --page=1 --hex /dev/sg0 sg_ses --page=2 --hex /dev/sg0 sg_ses --page=10 --hex /dev/sg0 The version of the kernel you do this on doesn't really matter. Thanks, 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
[PATCH] ses: Fix problems with simple enclosures
Simple enclosure implementations (mostly USB) are allowed to return only
page 8 to every diagnostic query. That really confuses our
implementation because we assume the return is the page we asked for and
end up doing incorrect offsets based on bogus information leading to
accesses outside of allocated ranges. Fix that by checking the page
code of the return and giving an error if it isn't the one we asked for.
This should fix reported bugs with USB storage by simply refusing to
attach to enclosures that behave like this. It's also good defensive
practise now that we're starting to see more USB enclosures.
Reported-by: Andrea Gelmini
Cc: sta...@vger.kernel.org
Signed-off-by: James Bottomley
---
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index dcb0d76..7d9cec5 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -84,6 +84,7 @@ static void init_device_slot_control(unsigned char *dest_desc,
static int ses_recv_diag(struct scsi_device *sdev, int page_code,
void *buf, int bufflen)
{
+ int ret;
unsigned char cmd[] = {
RECEIVE_DIAGNOSTIC,
1, /* Set PCV bit */
@@ -92,9 +93,26 @@ static int ses_recv_diag(struct scsi_device *sdev, int
page_code,
bufflen & 0xff,
0
};
+ unsigned char recv_page_code;
- return scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen,
+ ret = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen,
NULL, SES_TIMEOUT, SES_RETRIES, NULL);
+ if (unlikely(!ret))
+ return ret;
+
+ recv_page_code = ((unsigned char *)buf)[0];
+
+ if (likely(recv_page_code == page_code))
+ return ret;
+
+ /* successful diagnostic but wrong page code. This happens to some
+* USB devices, just print a message and pretend there was an error */
+
+ sdev_printk(KERN_ERR, sdev,
+ "Wrong diagnostic page; asked for %d got %u\n",
+ page_code, recv_page_code);
+
+ return -EINVAL;
}
static int ses_send_diag(struct scsi_device *sdev, int page_code,
--
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] ses: Fix problems with simple enclosures
On Tue, 2015-12-08 at 09:00 -0800, James Bottomley wrote:
> Simple enclosure implementations (mostly USB) are allowed to return only
> page 8 to every diagnostic query. That really confuses our
> implementation because we assume the return is the page we asked for and
> end up doing incorrect offsets based on bogus information leading to
> accesses outside of allocated ranges. Fix that by checking the page
> code of the return and giving an error if it isn't the one we asked for.
> This should fix reported bugs with USB storage by simply refusing to
> attach to enclosures that behave like this. It's also good defensive
> practise now that we're starting to see more USB enclosures.
>
> Reported-by: Andrea Gelmini
> Cc: sta...@vger.kernel.org
> Signed-off-by: James Bottomley
>
> ---
>
> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index dcb0d76..7d9cec5 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -84,6 +84,7 @@ static void init_device_slot_control(unsigned char
> *dest_desc,
> static int ses_recv_diag(struct scsi_device *sdev, int page_code,
>void *buf, int bufflen)
> {
> + int ret;
> unsigned char cmd[] = {
> RECEIVE_DIAGNOSTIC,
> 1, /* Set PCV bit */
> @@ -92,9 +93,26 @@ static int ses_recv_diag(struct scsi_device *sdev, int
> page_code,
> bufflen & 0xff,
> 0
> };
> + unsigned char recv_page_code;
>
> - return scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen,
> + ret = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen,
> NULL, SES_TIMEOUT, SES_RETRIES, NULL);
> + if (unlikely(!ret))
> + return ret;
> +
> + recv_page_code = ((unsigned char *)buf)[0];
> +
> + if (likely(recv_page_code == page_code))
> + return ret;
> +
> + /* successful diagnostic but wrong page code. This happens to some
> + * USB devices, just print a message and pretend there was an error */
> +
> + sdev_printk(KERN_ERR, sdev,
> + "Wrong diagnostic page; asked for %d got %u\n",
> + page_code, recv_page_code);
> +
> + return -EINVAL;
> }
>
> static int ses_send_diag(struct scsi_device *sdev, int page_code,
>
>
Reviewed-by: Ewan D. Milne
--
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 v4 0/3] Badblock tracking for gendisks
v4: - Rebase to v4.4-rc4 v3: - Add kernel-doc style comments to all exported functions in badblocks.c (James) - Make return values from badblocks functions consistent with themselves and the kernel style. Change the polarity of badblocks_set, and update all callers accordingly (James) - In gendisk, don't unconditionally allocate badblocks, export the initializer. This also allows the initializer to be a non-void return type, so that the badblocks user can act upon failures better (James) v2: - In badblocks_free, make 'page' NULL (patch 1) - Move the core badblocks code to a new .c file (patch 1) (Jens) - Fix a sizeof usage in disk_alloc_badblocks (patch 2) (Dan) - Since disk_alloc_badblocks can fail, check disk->bb for NULL in the genhd wrappers (patch 2) (Jeff) - Update the md conversion to also ise the badblocks init and free functions (patch 3) - Remove the BB_* macros from md.h as they are now in badblocks.h (patch 3) Patch 1 copies badblock management code into a header of its own, making it generally available. It follows common libraries of code such as linked lists, where anyone may embed a core data structure in another place, and use the provided accessor functions to manipulate the data. Patch 2 adds badblock tracking to gendisks (in preparation for use by NVDIMM devices). Patch 3 converts md over to use the new badblocks 'library'. I have done some pretty simple testing on this - created a raid 1 device, made sure the sysfs entries show up, and can be used to add and view badblocks. A closer look by the md folks would be nice here. Vishal Verma (3): badblocks: Add core badblock management code block: Add badblock management for gendisks md: convert to use the generic badblocks code block/Makefile| 2 +- block/badblocks.c | 576 ++ block/genhd.c | 76 ++ drivers/md/md.c | 516 ++--- drivers/md/md.h | 40 +--- include/linux/badblocks.h | 53 + include/linux/genhd.h | 7 + 7 files changed, 741 insertions(+), 529 deletions(-) create mode 100644 block/badblocks.c create mode 100644 include/linux/badblocks.h -- 2.5.0 -- 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 v4 3/3] md: convert to use the generic badblocks code
Retain badblocks as part of rdev, but use the accessor functions from
include/linux/badblocks for all manipulation.
Signed-off-by: Vishal Verma
---
drivers/md/md.c | 516 +++-
drivers/md/md.h | 40 +
2 files changed, 28 insertions(+), 528 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 807095f..1e48aa9 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -34,6 +34,7 @@
#include
#include
+#include
#include
#include
#include
@@ -709,8 +710,7 @@ void md_rdev_clear(struct md_rdev *rdev)
put_page(rdev->bb_page);
rdev->bb_page = NULL;
}
- kfree(rdev->badblocks.page);
- rdev->badblocks.page = NULL;
+ badblocks_free(&rdev->badblocks);
}
EXPORT_SYMBOL_GPL(md_rdev_clear);
@@ -1360,8 +1360,6 @@ static __le32 calc_sb_1_csum(struct mdp_superblock_1 *sb)
return cpu_to_le32(csum);
}
-static int md_set_badblocks(struct badblocks *bb, sector_t s, int sectors,
- int acknowledged);
static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int
minor_version)
{
struct mdp_superblock_1 *sb;
@@ -1486,8 +1484,7 @@ static int super_1_load(struct md_rdev *rdev, struct
md_rdev *refdev, int minor_
count <<= sb->bblog_shift;
if (bb + 1 == 0)
break;
- if (md_set_badblocks(&rdev->badblocks,
-sector, count, 1) == 0)
+ if (badblocks_set(&rdev->badblocks, sector, count, 1))
return -EINVAL;
}
} else if (sb->bblog_offset != 0)
@@ -2319,7 +2316,7 @@ repeat:
rdev_for_each(rdev, mddev) {
if (rdev->badblocks.changed) {
rdev->badblocks.changed = 0;
- md_ack_all_badblocks(&rdev->badblocks);
+ ack_all_badblocks(&rdev->badblocks);
md_error(mddev, rdev);
}
clear_bit(Blocked, &rdev->flags);
@@ -2445,7 +2442,7 @@ repeat:
clear_bit(Blocked, &rdev->flags);
if (any_badblocks_changed)
- md_ack_all_badblocks(&rdev->badblocks);
+ ack_all_badblocks(&rdev->badblocks);
clear_bit(BlockedBadBlocks, &rdev->flags);
wake_up(&rdev->blocked_wait);
}
@@ -3046,11 +3043,17 @@ static ssize_t recovery_start_store(struct md_rdev
*rdev, const char *buf, size_
static struct rdev_sysfs_entry rdev_recovery_start =
__ATTR(recovery_start, S_IRUGO|S_IWUSR, recovery_start_show,
recovery_start_store);
-static ssize_t
-badblocks_show(struct badblocks *bb, char *page, int unack);
-static ssize_t
-badblocks_store(struct badblocks *bb, const char *page, size_t len, int unack);
-
+/* sysfs access to bad-blocks list.
+ * We present two files.
+ * 'bad-blocks' lists sector numbers and lengths of ranges that
+ *are recorded as bad. The list is truncated to fit within
+ *the one-page limit of sysfs.
+ *Writing "sector length" to this file adds an acknowledged
+ *bad block list.
+ * 'unacknowledged-bad-blocks' lists bad blocks that have not yet
+ *been acknowledged. Writing to this file adds bad blocks
+ *without acknowledging them. This is largely for testing.
+ */
static ssize_t bb_show(struct md_rdev *rdev, char *page)
{
return badblocks_show(&rdev->badblocks, page, 0);
@@ -3165,14 +3168,7 @@ int md_rdev_init(struct md_rdev *rdev)
* This reserves the space even on arrays where it cannot
* be used - I wonder if that matters
*/
- rdev->badblocks.count = 0;
- rdev->badblocks.shift = -1; /* disabled until explicitly enabled */
- rdev->badblocks.page = kmalloc(PAGE_SIZE, GFP_KERNEL);
- seqlock_init(&rdev->badblocks.lock);
- if (rdev->badblocks.page == NULL)
- return -ENOMEM;
-
- return 0;
+ return badblocks_init(&rdev->badblocks, 0);
}
EXPORT_SYMBOL_GPL(md_rdev_init);
/*
@@ -8478,254 +8474,9 @@ void md_finish_reshape(struct mddev *mddev)
}
EXPORT_SYMBOL(md_finish_reshape);
-/* Bad block management.
- * We can record which blocks on each device are 'bad' and so just
- * fail those blocks, or that stripe, rather than the whole device.
- * Entries in the bad-block table are 64bits wide. This comprises:
- * Length of bad-range, in sectors: 0-511 for lengths 1-512
- * Start of bad-range, sector offset, 54 bits (allows 8 exbibytes)
- * A 'shift' can be set so that larger blocks are tracked and
- * consequently larger devices can be covered.
- * 'Acknowledged' flag - 1 bit. - the most significant bit.
- *
- * Locking of the bad-block table us
[PATCH v4 2/3] block: Add badblock management for gendisks
NVDIMM devices, which can behave more like DRAM rather than block
devices, may develop bad cache lines, or 'poison'. A block device
exposed by the pmem driver can then consume poison via a read (or
write), and cause a machine check. On platforms without machine
check recovery features, this would mean a crash.
The block device maintaining a runtime list of all known sectors that
have poison can directly avoid this, and also provide a path forward
to enable proper handling/recovery for DAX faults on such a device.
Use the new badblock management interfaces to add a badblocks list to
gendisks.
Signed-off-by: Vishal Verma
---
block/genhd.c | 76 +++
include/linux/genhd.h | 7 +
2 files changed, 83 insertions(+)
diff --git a/block/genhd.c b/block/genhd.c
index e5cafa5..81dcf32 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include "blk.h"
@@ -505,6 +506,16 @@ static int exact_lock(dev_t devt, void *data)
return 0;
}
+int disk_alloc_badblocks(struct gendisk *disk)
+{
+ disk->bb = kzalloc(sizeof(*(disk->bb)), GFP_KERNEL);
+ if (!disk->bb)
+ return -ENOMEM;
+
+ return badblocks_init(disk->bb, 1);
+}
+EXPORT_SYMBOL(disk_alloc_badblocks);
+
static void register_disk(struct gendisk *disk)
{
struct device *ddev = disk_to_dev(disk);
@@ -659,6 +670,11 @@ void del_gendisk(struct gendisk *disk)
blk_unregister_queue(disk);
blk_unregister_region(disk_devt(disk), disk->minors);
+ if (disk->bb) {
+ badblocks_free(disk->bb);
+ kfree(disk->bb);
+ }
+
part_stat_set_all(&disk->part0, 0);
disk->part0.stamp = 0;
@@ -672,6 +688,63 @@ void del_gendisk(struct gendisk *disk)
}
EXPORT_SYMBOL(del_gendisk);
+/*
+ * The gendisk usage of badblocks does not track acknowledgements for
+ * badblocks. We always assume they are acknowledged.
+ */
+int disk_check_badblocks(struct gendisk *disk, sector_t s, int sectors,
+ sector_t *first_bad, int *bad_sectors)
+{
+ if (!disk->bb)
+ return 0;
+
+ return badblocks_check(disk->bb, s, sectors, first_bad, bad_sectors);
+}
+EXPORT_SYMBOL(disk_check_badblocks);
+
+int disk_set_badblocks(struct gendisk *disk, sector_t s, int sectors)
+{
+ if (!disk->bb)
+ return 0;
+
+ return badblocks_set(disk->bb, s, sectors, 1);
+}
+EXPORT_SYMBOL(disk_set_badblocks);
+
+int disk_clear_badblocks(struct gendisk *disk, sector_t s, int sectors)
+{
+ if (!disk->bb)
+ return 0;
+
+ return badblocks_clear(disk->bb, s, sectors);
+}
+EXPORT_SYMBOL(disk_clear_badblocks);
+
+/* sysfs access to bad-blocks list. */
+static ssize_t disk_badblocks_show(struct device *dev,
+ struct device_attribute *attr,
+ char *page)
+{
+ struct gendisk *disk = dev_to_disk(dev);
+
+ if (!disk->bb)
+ return 0;
+
+ return badblocks_show(disk->bb, page, 0);
+}
+
+static ssize_t disk_badblocks_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *page, size_t len)
+{
+ struct gendisk *disk = dev_to_disk(dev);
+
+ if (!disk->bb)
+ return 0;
+
+ return badblocks_store(disk->bb, page, len, 0);
+}
+
/**
* get_gendisk - get partitioning information for a given device
* @devt: device to get partitioning information for
@@ -990,6 +1063,8 @@ static DEVICE_ATTR(discard_alignment, S_IRUGO,
disk_discard_alignment_show,
static DEVICE_ATTR(capability, S_IRUGO, disk_capability_show, NULL);
static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL);
static DEVICE_ATTR(inflight, S_IRUGO, part_inflight_show, NULL);
+static DEVICE_ATTR(badblocks, S_IRUGO | S_IWUSR, disk_badblocks_show,
+ disk_badblocks_store);
#ifdef CONFIG_FAIL_MAKE_REQUEST
static struct device_attribute dev_attr_fail =
__ATTR(make-it-fail, S_IRUGO|S_IWUSR, part_fail_show, part_fail_store);
@@ -1011,6 +1086,7 @@ static struct attribute *disk_attrs[] = {
&dev_attr_capability.attr,
&dev_attr_stat.attr,
&dev_attr_inflight.attr,
+ &dev_attr_badblocks.attr,
#ifdef CONFIG_FAIL_MAKE_REQUEST
&dev_attr_fail.attr,
#endif
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 847cc1d..0bbec68 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -162,6 +162,7 @@ struct disk_part_tbl {
};
struct disk_events;
+struct badblocks;
#if defined(CONFIG_BLK_DEV_INTEGRITY)
@@ -213,6 +214,7 @@ struct gendisk {
struct kobject integrity_kobj;
#endif /* CONFIG_BLK_DEV_INTEGRITY */
int node_id;
+ struct badblocks *bb;
};
static inline struct gendisk *part_to_disk(struct hd_struct *part)
@@ -433,6 +435,11 @@ extern v
[PATCH v4 1/3] badblocks: Add core badblock management code
Take the core badblocks implementation from md, and make it generally
available. This follows the same style as kernel implementations of
linked lists, rb-trees etc, where you can have a structure that can be
embedded anywhere, and accessor functions to manipulate the data.
The only changes in this copy of the code are ones to generalize
function/variable names from md-specific ones. Also add init and free
functions.
Signed-off-by: Vishal Verma
---
block/Makefile| 2 +-
block/badblocks.c | 576 ++
include/linux/badblocks.h | 53 +
3 files changed, 630 insertions(+), 1 deletion(-)
create mode 100644 block/badblocks.c
create mode 100644 include/linux/badblocks.h
diff --git a/block/Makefile b/block/Makefile
index 00ecc97..db5f622 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_BLOCK) := bio.o elevator.o blk-core.o blk-tag.o
blk-sysfs.o \
blk-iopoll.o blk-lib.o blk-mq.o blk-mq-tag.o \
blk-mq-sysfs.o blk-mq-cpu.o blk-mq-cpumap.o ioctl.o \
genhd.o scsi_ioctl.o partition-generic.o ioprio.o \
- partitions/
+ badblocks.o partitions/
obj-$(CONFIG_BOUNCE) += bounce.o
obj-$(CONFIG_BLK_DEV_BSG) += bsg.o
diff --git a/block/badblocks.c b/block/badblocks.c
new file mode 100644
index 000..f0ac279
--- /dev/null
+++ b/block/badblocks.c
@@ -0,0 +1,576 @@
+/*
+ * Bad block management
+ *
+ * - Heavily based on MD badblocks code from Neil Brown
+ *
+ * Copyright (c) 2015, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * badblocks_check() - check a given range for bad sectors
+ * @bb:the badblocks structure that holds all badblock
information
+ * @s: sector (start) at which to check for badblocks
+ * @sectors: number of sectors to check for badblocks
+ * @first_bad: pointer to store location of the first badblock
+ * @bad_sectors: pointer to store number of badblocks after @first_bad
+ *
+ * We can record which blocks on each device are 'bad' and so just
+ * fail those blocks, or that stripe, rather than the whole device.
+ * Entries in the bad-block table are 64bits wide. This comprises:
+ * Length of bad-range, in sectors: 0-511 for lengths 1-512
+ * Start of bad-range, sector offset, 54 bits (allows 8 exbibytes)
+ * A 'shift' can be set so that larger blocks are tracked and
+ * consequently larger devices can be covered.
+ * 'Acknowledged' flag - 1 bit. - the most significant bit.
+ *
+ * Locking of the bad-block table uses a seqlock so badblocks_check
+ * might need to retry if it is very unlucky.
+ * We will sometimes want to check for bad blocks in a bi_end_io function,
+ * so we use the write_seqlock_irq variant.
+ *
+ * When looking for a bad block we specify a range and want to
+ * know if any block in the range is bad. So we binary-search
+ * to the last range that starts at-or-before the given endpoint,
+ * (or "before the sector after the target range")
+ * then see if it ends after the given start.
+ *
+ * Return:
+ * 0: there are no known bad blocks in the range
+ * 1: there are known bad block which are all acknowledged
+ * -1: there are bad blocks which have not yet been acknowledged in metadata.
+ * plus the start/length of the first bad section we overlap.
+ */
+int badblocks_check(struct badblocks *bb, sector_t s, int sectors,
+ sector_t *first_bad, int *bad_sectors)
+{
+ int hi;
+ int lo;
+ u64 *p = bb->page;
+ int rv;
+ sector_t target = s + sectors;
+ unsigned seq;
+
+ if (bb->shift > 0) {
+ /* round the start down, and the end up */
+ s >>= bb->shift;
+ target += (1>= bb->shift;
+ sectors = target - s;
+ }
+ /* 'target' is now the first block after the bad range */
+
+retry:
+ seq = read_seqbegin(&bb->lock);
+ lo = 0;
+ rv = 0;
+ hi = bb->count;
+
+ /* Binary search between lo and hi for 'target'
+* i.e. for the last range that starts before 'target'
+*/
+ /* INVARIANT: ranges before 'lo' and at-or-after 'hi'
+* are known not to be the last range before target.
+* VARIANT: hi-lo is the number of possible
+* ranges, and decreases until it reaches 1
+*/
+ while (hi - lo >
Re: [PATCH 01/20] qla2xxx: Enable Extended Login support
On 12/8/15, 7:51 AM, "Hannes Reinecke" wrote: >On 12/08/2015 01:48 AM, Himanshu Madhani wrote: >> Signed-off-by: Himanshu Madhani >> Signed-off-by: Giridhar Malavali >> --- >A patch description would be nice; ATM it doesn't look >as if this interface is used anywhere... I¹ll update patch with description of the feature. > >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) Thanks, Himanshu <>
Re: [PATCH v4 0/3] Badblock tracking for gendisks
On Wed, Dec 09 2015, Vishal Verma wrote: > > Patch 3 converts md over to use the new badblocks 'library'. I have > done some pretty simple testing on this - created a raid 1 device, > made sure the sysfs entries show up, and can be used to add and view > badblocks. A closer look by the md folks would be nice here. Hi, this all looks reasonably sensible. I haven't gone over it with a fine toothed comb (I wonder where that analogy comes from) but nothing is obviously wrong. Acked-by: NeilBrown Thanks, NeilBrown signature.asc Description: PGP signature
Re: [PATCH v2 1/3] badblocks: Add core badblock management code
On Sat, Dec 05 2015, Verma, Vishal L wrote:
>>
>> > +int badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
>> > +{
>> [...]
>> > +#define DO_DEBUG 1
>>
>> Why have this at all if it's unconditionally defined and always set.
>
> Neil - any reason or anything you had in mind for this? Or is it just an
> artifact and can be removed.
Like the comment says:
/* Allow clearing via sysfs *only* for testing/debugging.
* Normally only a successful write may clear a badblock
*/
The DO_DEBUG define and ifdefs are documentation identifying bits of
code that should be removed when it all seems to be working.
Maybe now is a good time to remove that code.
NeilBrown
signature.asc
Description: PGP signature
Re: [PATCH v2 1/3] badblocks: Add core badblock management code
On Wed, 2015-12-09 at 08:03 +1100, NeilBrown wrote:
> On Sat, Dec 05 2015, Verma, Vishal L wrote:
> > >
> > > > +int badblocks_clear(struct badblocks *bb, sector_t s, int
> > > > sectors)
> > > > +{
> > > [...]
> > > > +#define DO_DEBUG 1
> > >
> > > Why have this at all if it's unconditionally defined and always
> > > set.
> >
> > Neil - any reason or anything you had in mind for this? Or is it
> > just an
> > artifact and can be removed.
>
> Like the comment says:
>
> /* Allow clearing via sysfs *only* for testing/debugging.
>* Normally only a successful write may clear a badblock
>*/
>
> The DO_DEBUG define and ifdefs are documentation identifying bits of
> code that should be removed when it all seems to be working.
> Maybe now is a good time to remove that code.
>
Hm, I think it would be nice to continue to have the ability to clear
badblocks using sysfs at least for a while more, as we test the various
error handling paths for NVDIMMS (Dan, thoughts?).
We could either remove it later or (I'm leaning towards) make it a
config option similar to FAIL_MAKE_REQUEST and friends..
-Vishal
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2 1/3] badblocks: Add core badblock management code
On Tue, Dec 8, 2015 at 1:08 PM, Verma, Vishal L
wrote:
> On Wed, 2015-12-09 at 08:03 +1100, NeilBrown wrote:
>> On Sat, Dec 05 2015, Verma, Vishal L wrote:
>> > >
>> > > > +int badblocks_clear(struct badblocks *bb, sector_t s, int
>> > > > sectors)
>> > > > +{
>> > > [...]
>> > > > +#define DO_DEBUG 1
>> > >
>> > > Why have this at all if it's unconditionally defined and always
>> > > set.
>> >
>> > Neil - any reason or anything you had in mind for this? Or is it
>> > just an
>> > artifact and can be removed.
>>
>> Like the comment says:
>>
>> /* Allow clearing via sysfs *only* for testing/debugging.
>>* Normally only a successful write may clear a badblock
>>*/
>>
>> The DO_DEBUG define and ifdefs are documentation identifying bits of
>> code that should be removed when it all seems to be working.
>> Maybe now is a good time to remove that code.
>>
> Hm, I think it would be nice to continue to have the ability to clear
> badblocks using sysfs at least for a while more, as we test the various
> error handling paths for NVDIMMS (Dan, thoughts?).
>
> We could either remove it later or (I'm leaning towards) make it a
> config option similar to FAIL_MAKE_REQUEST and friends..
"later" as in before v4.5-rc1? We can always carry this debug feature
locally for testing. We don't want userspace growing ABI attachments
to this capability now that it's more than just md tooling that will
see this.
--
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 v4 0/3] Badblock tracking for gendisks
On Wed, 9 Dec 2015, NeilBrown wrote: > On Wed, Dec 09 2015, Vishal Verma wrote: > > > > > Patch 3 converts md over to use the new badblocks 'library'. I have > > done some pretty simple testing on this - created a raid 1 device, > > made sure the sysfs entries show up, and can be used to add and view > > badblocks. A closer look by the md folks would be nice here. > > Hi, > this all looks reasonably sensible. I haven't gone over it with a > fine toothed comb (I wonder where that analogy comes from) Delousing. (Not debugging.) > but > nothing is obviously wrong. > > Acked-by: NeilBrown > > Thanks, > NeilBrown > -- -- 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] osd: fix signed char versus %02x issue
On 12/08/2015 04:25 PM, Rasmus Villemoes wrote:
> If char is signed and one of these bytes happen to have a value
> outside the ascii range, the corresponding output will consist of
> "ff" followed by the two hex chars that were actually
> intended. One way to fix it would be to change the casts to (u8*) aka
> (unsigned char*), but it is much simpler (and generates smaller code)
> to use the %ph extension which was created for such short hexdumps.
>
Ha real cool, thanks I hated that crap
ACK-by: Boaz Harrosh
> Signed-off-by: Rasmus Villemoes
> ---
> drivers/scsi/osd/osd_initiator.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/osd/osd_initiator.c
> b/drivers/scsi/osd/osd_initiator.c
> index 0cccd6033feb..d8a2b5185f56 100644
> --- a/drivers/scsi/osd/osd_initiator.c
> +++ b/drivers/scsi/osd/osd_initiator.c
> @@ -170,10 +170,7 @@ static int _osd_get_print_system_info(struct osd_dev *od,
>
> /* FIXME: Where are the time utilities */
> pFirst = get_attrs[a++].val_ptr;
> - OSD_INFO("CLOCK [0x%02x%02x%02x%02x%02x%02x]\n",
> - ((char *)pFirst)[0], ((char *)pFirst)[1],
> - ((char *)pFirst)[2], ((char *)pFirst)[3],
> - ((char *)pFirst)[4], ((char *)pFirst)[5]);
> + OSD_INFO("CLOCK [0x%6phN]\n", pFirst);
>
> if (a < nelem) { /* IBM-OSD-SIM bug, Might not have it */
> unsigned len = get_attrs[a].len;
>
--
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 1/3] badblocks: Add core badblock management code
On Tue, 2015-12-08 at 13:18 -0800, Dan Williams wrote:
> On Tue, Dec 8, 2015 at 1:08 PM, Verma, Vishal L
> wrote:
> > On Wed, 2015-12-09 at 08:03 +1100, NeilBrown wrote:
> > > On Sat, Dec 05 2015, Verma, Vishal L wrote:
> > > > >
> > > > > > +int badblocks_clear(struct badblocks *bb, sector_t s, int
> > > > > > sectors)
> > > > > > +{
> > > > > [...]
> > > > > > +#define DO_DEBUG 1
> > > > >
> > > > > Why have this at all if it's unconditionally defined and
> > > > > always
> > > > > set.
> > > >
> > > > Neil - any reason or anything you had in mind for this? Or is it
> > > > just an
> > > > artifact and can be removed.
> > >
> > > Like the comment says:
> > >
> > > /* Allow clearing via sysfs *only* for testing/debugging.
> > > * Normally only a successful write may clear a badblock
> > > */
> > >
> > > The DO_DEBUG define and ifdefs are documentation identifying bits
> > > of
> > > code that should be removed when it all seems to be working.
> > > Maybe now is a good time to remove that code.
> > >
> > Hm, I think it would be nice to continue to have the ability to
> > clear
> > badblocks using sysfs at least for a while more, as we test the
> > various
> > error handling paths for NVDIMMS (Dan, thoughts?).
> >
> > We could either remove it later or (I'm leaning towards) make it a
> > config option similar to FAIL_MAKE_REQUEST and friends..
>
> "later" as in before v4.5-rc1? We can always carry this debug feature
> locally for testing. We don't want userspace growing ABI attachments
> to this capability now that it's more than just md tooling that will
> see this.
Agreed. The following incremental patch removes sysfs support.
All the latest badblocks patches can also be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/vishal/nvdimm.git
gendisk-badblocks
8<-
From 5f0e7ac31d27a132f314106f1db33af22fde03ed Mon Sep 17 00:00:00 2001
From: Vishal Verma
Date: Tue, 8 Dec 2015 16:28:31 -0700
Subject: [PATCH v4 4/3] badblocks: remove support for clearing via sysfs
sysfs support for clearing badblocks was originally meant for testing
only. With the move to generalize the interface, remove this support so
that userspace doesn't start treating this as an ABI.
Signed-off-by: Vishal Verma
---
block/badblocks.c | 15 ---
1 file changed, 15 deletions(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index f0ac279..e5d2a91 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -503,16 +503,6 @@ ssize_t badblocks_store(struct badblocks *bb, const
char *page, size_t len,
int length;
char newline;
- /* Allow clearing via sysfs *only* for testing/debugging.
- * Normally only a successful write may clear a badblock
- */
- int clear = 0;
-
- if (page[0] == '-') {
- clear = 1;
- page++;
- }
-
switch (sscanf(page, "%llu %d%c", §or, &length, &newline))
{
case 3:
if (newline != '\n')
@@ -525,11 +515,6 @@ ssize_t badblocks_store(struct badblocks *bb, const
char *page, size_t len,
return -EINVAL;
}
- if (clear) {
- badblocks_clear(bb, sector, length);
- return len;
- }
-
if (badblocks_set(bb, sector, length, !unack))
return -ENOSPC;
else
--
2.5.0
RE: [PATCH v2] storvsc: add logging for error/warning messages
> -Original Message-
> From: Long Li [mailto:lon...@microsoft.com]
> Sent: Friday, December 4, 2015 12:07 AM
> To: KY Srinivasan ; Haiyang Zhang
> ; James E.J. Bottomley
> Cc: de...@linuxdriverproject.org; linux-scsi@vger.kernel.org; linux-
> ker...@vger.kernel.org; Long Li
> Subject: [PATCH v2] storvsc: add logging for error/warning messages
>
> Introduce a logging level for storvsc to log certain error/warning messages.
> Those messages are helpful in some environments, e.g. Microsoft Azure, for
> customer support and troubleshooting purposes.
>
> Signed-off-by: Long Li
Reviewed-by : K. Y. Srinivasan
Signed-off-by: K. Y. Srinivasan
> ---
> drivers/scsi/storvsc_drv.c | 34 +-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 40c43ae..f46ed2c 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -164,6 +164,26 @@ static int sense_buffer_size =
> PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
> */
> static int vmstor_proto_version;
>
> +#define STORVSC_LOGGING_NONE 0
> +#define STORVSC_LOGGING_ERROR1
> +#define STORVSC_LOGGING_WARN 2
> +
> +static int logging_level = STORVSC_LOGGING_ERROR;
> +module_param(logging_level, int, S_IRUGO|S_IWUSR);
> +MODULE_PARM_DESC(logging_level,
> + "Logging level, 0 - None, 1 - Error (default), 2 - Warning.");
> +
> +static inline bool do_logging(int level)
> +{
> + return logging_level >= level;
> +}
> +
> +#define storvsc_log(dev, level, fmt, ...)\
> +do { \
> + if (do_logging(level)) \
> + dev_warn(&(dev)->device, fmt, ##__VA_ARGS__); \
> +} while (0)
> +
> struct vmscsi_win8_extension {
> /*
>* The following were added in Windows 8
> @@ -1185,7 +1205,8 @@ static void storvsc_command_completion(struct
> storvsc_cmd_request *cmd_request)
>
> if (scmnd->result) {
> if (scsi_normalize_sense(scmnd->sense_buffer,
> - SCSI_SENSE_BUFFERSIZE, &sense_hdr))
> + SCSI_SENSE_BUFFERSIZE, &sense_hdr) &&
> + do_logging(STORVSC_LOGGING_ERROR))
> scsi_print_sense_hdr(scmnd->device, "storvsc",
>&sense_hdr);
> }
> @@ -1239,6 +1260,13 @@ static void storvsc_on_io_completion(struct
> hv_device *device,
> stor_pkt->vm_srb.sense_info_length =
> vstor_packet->vm_srb.sense_info_length;
>
> + if (vstor_packet->vm_srb.scsi_status != 0 ||
> + vstor_packet->vm_srb.srb_status != SRB_STATUS_SUCCESS)
> + storvsc_log(device, STORVSC_LOGGING_WARN,
> + "cmd 0x%x scsi status 0x%x srb status 0x%x\n",
> + stor_pkt->vm_srb.cdb[0],
> + vstor_packet->vm_srb.scsi_status,
> + vstor_packet->vm_srb.srb_status);
>
> if ((vstor_packet->vm_srb.scsi_status & 0xFF) == 0x02) {
> /* CHECK_CONDITION */
> @@ -1246,6 +1274,10 @@ static void storvsc_on_io_completion(struct
> hv_device *device,
> SRB_STATUS_AUTOSENSE_VALID) {
> /* autosense data available */
>
> + storvsc_log(device, STORVSC_LOGGING_WARN,
> + "stor pkt %p autosense data valid - len
> %d\n",
> + request, vstor_packet-
> >vm_srb.sense_info_length);
> +
> memcpy(request->cmd->sense_buffer,
> vstor_packet->vm_srb.sense_data,
> vstor_packet->vm_srb.sense_info_length);
> --
> 1.8.5.6
--
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
Update
Good day, hoping you read this email and respond to me in good time.I do not intend to solicit for funds but your time and energy in using my own resources to assist the less privileged becauseI am medically ill and confined at the moment hence I request your indulgence.I will give you a comprehensive brief once I hear from you. Thanks and reply. Robert Grondahl -- 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 09/20] qla2xxx: Change check_stop_free to always return 1
On 12/08/2015 01:48 AM, Himanshu Madhani wrote: > From: Quinn Tran > > change tcm_qla2xxx_check_stop_free to always return 1 > to prevent transport_cmd_finish_abort from accidently > taking extra kref_put. > > Signed-off-by: Quinn Tran > Signed-off-by: Himanshu Madhani > --- > drivers/scsi/qla2xxx/tcm_qla2xxx.c |3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c > b/drivers/scsi/qla2xxx/tcm_qla2xxx.c > index 74c6e9b..366142a 100644 > --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c > +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c > @@ -314,7 +314,8 @@ static int tcm_qla2xxx_check_stop_free(struct se_cmd > *se_cmd) > cmd->cmd_flags |= BIT_14; > } > > - return target_put_sess_cmd(se_cmd); > + target_put_sess_cmd(se_cmd); > + return 1; > } > > /* tcm_qla2xxx_release_cmd - Callback from TCM Core to release underlying > Odd. I would have expected target_put_sess_cmd() to never fail. But if it does, doesn't it mean that the command is hosed, and we should have accessed it in the first place? Very suspicious. 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 10/20] qla2xxx: Fix interaction issue between qla2xxx and Target Core Module
On 12/08/2015 01:48 AM, Himanshu Madhani wrote:
> From: Quinn Tran
>
> During lun reset, TMR thread from TCM would issue abort
> to qla driver. At abort time, each command is in different
> state. Depending on the state, qla will use the TMR thread
> to trigger a command free(cmd_kref--) if command is not
> down at firmware.
>
> Signed-off-by: Quinn Tran
> Signed-off-by: Himanshu Madhani
> ---
> drivers/scsi/qla2xxx/qla_target.c | 60 +
> drivers/scsi/qla2xxx/qla_target.h | 59 +
> drivers/scsi/qla2xxx/tcm_qla2xxx.c | 73 ++-
> 3 files changed, 147 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_target.c
> b/drivers/scsi/qla2xxx/qla_target.c
> index 638940f..4d42b79 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -105,7 +105,7 @@ static void qlt_response_pkt(struct scsi_qla_host *ha,
> response_t *pkt);
> static int qlt_issue_task_mgmt(struct qla_tgt_sess *sess, uint32_t lun,
> int fn, void *iocb, int flags);
> static void qlt_send_term_exchange(struct scsi_qla_host *ha, struct
> qla_tgt_cmd
> - *cmd, struct atio_from_isp *atio, int ha_locked);
> + *cmd, struct atio_from_isp *atio, int ha_locked, int ul_abort);
> static void qlt_reject_free_srr_imm(struct scsi_qla_host *ha,
> struct qla_tgt_srr_imm *imm, int ha_lock);
> static void qlt_abort_cmd_on_host_reset(struct scsi_qla_host *vha,
> @@ -2646,7 +2646,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int
> xmit_type,
> /* no need to terminate. FW already freed exchange. */
> qlt_abort_cmd_on_host_reset(cmd->vha, cmd);
> else
> - qlt_send_term_exchange(vha, cmd, &cmd->atio, 1);
> + qlt_send_term_exchange(vha, cmd, &cmd->atio, 1, 0);
> spin_unlock_irqrestore(&ha->hardware_lock, flags);
> return 0;
> }
> @@ -3154,7 +3154,8 @@ static int __qlt_send_term_exchange(struct
> scsi_qla_host *vha,
> }
>
> static void qlt_send_term_exchange(struct scsi_qla_host *vha,
> - struct qla_tgt_cmd *cmd, struct atio_from_isp *atio, int ha_locked)
> + struct qla_tgt_cmd *cmd, struct atio_from_isp *atio, int ha_locked,
> + int ul_abort)
> {
> unsigned long flags = 0;
> int rc;
> @@ -3174,8 +3175,7 @@ static void qlt_send_term_exchange(struct scsi_qla_host
> *vha,
> qlt_alloc_qfull_cmd(vha, atio, 0, 0);
>
> done:
> - if (cmd && (!cmd->aborted ||
> - !cmd->cmd_sent_to_fw)) {
> + if (cmd && !ul_abort && !cmd->aborted) {
> if (cmd->sg_mapped)
> qlt_unmap_sg(vha, cmd);
> vha->hw->tgt.tgt_ops->free_cmd(cmd);
> @@ -3234,21 +3234,43 @@ static void qlt_chk_exch_leak_thresh_hold(struct
> scsi_qla_host *vha)
>
> }
>
> -void qlt_abort_cmd(struct qla_tgt_cmd *cmd)
> +int qlt_abort_cmd(struct qla_tgt_cmd *cmd)
> {
> struct qla_tgt *tgt = cmd->tgt;
> struct scsi_qla_host *vha = tgt->vha;
> struct se_cmd *se_cmd = &cmd->se_cmd;
> + unsigned long flags,refcount;
>
> ql_dbg(ql_dbg_tgt_mgt, vha, 0xf014,
> "qla_target(%d): terminating exchange for aborted cmd=%p "
> "(se_cmd=%p, tag=%llu)", vha->vp_idx, cmd, &cmd->se_cmd,
> se_cmd->tag);
>
> +spin_lock_irqsave(&cmd->cmd_lock, flags);
> +if (cmd->aborted) {
> +spin_unlock_irqrestore(&cmd->cmd_lock, flags);
> +
> +/* It's normal to see 2 calls in this path:
> + * 1) XFER Rdy completion + CMD_T_ABORT
> + * 2) TCM TMR - drain_state_list
> + */
> +refcount = atomic_read(&cmd->se_cmd.cmd_kref.refcount);
> +ql_dbg(ql_dbg_tgt_mgt, vha, 0x,
> + "multiple abort. %p refcount %lx"
> + "transport_state %x, t_state %x, se_cmd_flags %x \n",
> + cmd, refcount,cmd->se_cmd.transport_state,
> + cmd->se_cmd.t_state,cmd->se_cmd.se_cmd_flags);
> +
> +return EIO;
> +}
> +
> cmd->aborted = 1;
> cmd->cmd_flags |= BIT_6;
> +spin_unlock_irqrestore(&cmd->cmd_lock, flags);
> +
> + qlt_send_term_exchange(vha, cmd, &cmd->atio, 0, 1);
>
> - qlt_send_term_exchange(vha, cmd, &cmd->atio, 0);
> + return 0;
> }
> EXPORT_SYMBOL(qlt_abort_cmd);
>
> @@ -3263,6 +3285,9 @@ void qlt_free_cmd(struct qla_tgt_cmd *cmd)
>
> BUG_ON(cmd->cmd_in_wq);
>
> + if (cmd->sg_mapped)
> + qlt_unmap_sg(cmd->vha, cmd);
> +
> if (!cmd->q_full)
> qlt_decr_num_pend_cmds(cmd->vha);
>
> @@ -3380,7 +3405,7 @@ static int qlt_term_ctio_exchange(struct scsi_qla_host
> *vha, void *ctio,
> term = 1;
>
> if (term)
> - qlt_send_term_exchange(vha, cmd, &cmd->atio, 1);
> + qlt_send_term_exchange(vha, cmd, &cmd->atio, 1, 0);
>
> return te
Re: [PATCH 11/20] qla2xxx: Add TAS detection for kernel 3.15 n newer
On 12/08/2015 01:48 AM, Himanshu Madhani wrote:
> From: Quinn Tran
>
> For kernel 3.15 and newer with TCM API change, add detection
> for TCM support of TAS. Instead of default command terminate
> for LUN/TARGET reset error handling, allow SCSI status to go
> out if we know sequece Initiative is own by FW (cmd_sent_to_fw=0)
>
> Signed-off-by: Quinn Tran
> Signed-off-by: Himanshu Madhani
> ---
> drivers/scsi/qla2xxx/qla_target.c | 36
> +---
> drivers/scsi/qla2xxx/tcm_qla2xxx.c | 20
> 2 files changed, 37 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_target.c
> b/drivers/scsi/qla2xxx/qla_target.c
> index 4d42b79..5fca846 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -3239,36 +3239,34 @@ int qlt_abort_cmd(struct qla_tgt_cmd *cmd)
> struct qla_tgt *tgt = cmd->tgt;
> struct scsi_qla_host *vha = tgt->vha;
> struct se_cmd *se_cmd = &cmd->se_cmd;
> - unsigned long flags,refcount;
> + unsigned long flags, refcount;
>
> ql_dbg(ql_dbg_tgt_mgt, vha, 0xf014,
> "qla_target(%d): terminating exchange for aborted cmd=%p "
> "(se_cmd=%p, tag=%llu)", vha->vp_idx, cmd, &cmd->se_cmd,
> se_cmd->tag);
>
> -spin_lock_irqsave(&cmd->cmd_lock, flags);
> -if (cmd->aborted) {
> -spin_unlock_irqrestore(&cmd->cmd_lock, flags);
> + spin_lock_irqsave(&cmd->cmd_lock, flags);
>
> -/* It's normal to see 2 calls in this path:
> - * 1) XFER Rdy completion + CMD_T_ABORT
> - * 2) TCM TMR - drain_state_list
> - */
> -refcount = atomic_read(&cmd->se_cmd.cmd_kref.refcount);
> -ql_dbg(ql_dbg_tgt_mgt, vha, 0x,
> - "multiple abort. %p refcount %lx"
> - "transport_state %x, t_state %x, se_cmd_flags %x \n",
> - cmd, refcount,cmd->se_cmd.transport_state,
> - cmd->se_cmd.t_state,cmd->se_cmd.se_cmd_flags);
> + if (cmd->aborted) {
> + spin_unlock_irqrestore(&cmd->cmd_lock, flags);
> + /* It's normal to see 2 calls in this path:
> + * 1) XFER Rdy completion + CMD_T_ABORT
> + * 2) TCM TMR - drain_state_list
> + */
> + refcount = atomic_read(&cmd->se_cmd.cmd_kref.refcount);
> + ql_dbg(ql_dbg_tgt_mgt, vha, 0x,
> + "multiple abort. %p refcount %lx"
> + "transport_state %x, t_state %x, se_cmd_flags %x \n",
> + cmd, refcount, cmd->se_cmd.transport_state,
> + cmd->se_cmd.t_state, cmd->se_cmd.se_cmd_flags);
>
> -return EIO;
> -}
> + return EIO;
> + }
>
> cmd->aborted = 1;
> cmd->cmd_flags |= BIT_6;
> -spin_unlock_irqrestore(&cmd->cmd_lock, flags);
> -
> - qlt_send_term_exchange(vha, cmd, &cmd->atio, 0, 1);
> + spin_unlock_irqrestore(&cmd->cmd_lock, flags);
>
> return 0;
> }
> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> index 842fcca..2e9c194 100644
> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> @@ -617,6 +617,26 @@ static int tcm_qla2xxx_queue_status(struct se_cmd
> *se_cmd)
> struct qla_tgt_cmd, se_cmd);
> int xmit_type = QLA_TGT_XMIT_STATUS;
>
> + if (se_cmd->transport_state & CMD_T_ABORTED) {
> + /* For TCM TAS support n kernel >= 3.15:
> + * This cmd is attempting to respond with "Task Aborted Status".
> + */
> + if (cmd->aborted) {
> + return 0;
> + } else if ((cmd->state == QLA_TGT_STATE_NEED_DATA) &&
> + cmd->cmd_sent_to_fw) {
> + qlt_abort_cmd(cmd);
> + return 0;
> + } else if (cmd->state == QLA_TGT_STATE_PROCESSED) {
> + if (cmd->cmd_sent_to_fw) {
> + qlt_abort_cmd(cmd);
> + return 0;
> + } else {/* about to be free */
> + return 0;
> + }
> + }
> + }
> +
> cmd->bufflen = se_cmd->data_length;
> cmd->sg = NULL;
> cmd->sg_cnt = 0;
>
Reviewed-by: Hannes Reinecke
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 12/20] target/tmr: LUN reset cause cmd premature free.
On 12/08/2015 01:48 AM, Himanshu Madhani wrote:
> From: Quinn Tran
>
> During LUN/Target reset, the TMR code attempt to intercept
> cmds and try to aborted them. Current code assume cmds are
> always intercepted at the back end device. The cleanup code
> would issue a "queue_status() & check_stop_free()" to terminate
> the command. However, when a cmd is intercepted at the front
> end/Fabric layer, current code introduce premature free or
> cause Fabric to double free.
>
> When command is intercepted at Fabric layer, it means a
> check_stop_free(cmd_kref--) has been called. The extra
> check_stop_free in the Lun Reset cleanup code causes early
> free. When a cmd in the Fabric layer is completed, the normal
> free code adds another another free which introduce a double free.
>
> To fix the issue:
> - add a new flag/CMD_T_SENT_STATUS to track command that have
> made it down to fabric layer after back end good/bad completion.
> - if cmd reach Fabric Layer at Lun Reset time, add an extra
> cmd_kref count to prevent premature free.
>
> Signed-off-by: Quinn Tran
> Signed-off-by: Himanshu Madhani
> ---
> drivers/target/target_core_tmr.c | 33
> +++-
> drivers/target/target_core_transport.c | 30 +
> include/target/target_core_base.h |1 +
> 3 files changed, 63 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/target/target_core_tmr.c
> b/drivers/target/target_core_tmr.c
> index 28fb301..41f8b57 100644
> --- a/drivers/target/target_core_tmr.c
> +++ b/drivers/target/target_core_tmr.c
> @@ -243,7 +243,9 @@ static void core_tmr_drain_state_list(
> {
> LIST_HEAD(drain_task_list);
> struct se_cmd *cmd, *next;
> - unsigned long flags;
> + unsigned long flags, flags2;
> + int rmkref;
> + struct se_session *se_sess;
>
> /*
>* Complete outstanding commands with TASK_ABORTED SAM status.
> @@ -282,6 +284,16 @@ static void core_tmr_drain_state_list(
> if (prout_cmd == cmd)
> continue;
>
> + se_sess = cmd->se_sess;
> + /* take an extra kref to prevent cmd free race condition. */
> + spin_lock_irqsave(&se_sess->sess_cmd_lock, flags2);
> + if (!kref_get_unless_zero(&cmd->cmd_kref)) {
> + /* cmd is already in the free process */
> + spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags2);
> + continue;
> + }
> + spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags2);
> +
> list_move_tail(&cmd->state_list, &drain_task_list);
> cmd->state_active = false;
> }
> @@ -320,9 +332,28 @@ static void core_tmr_drain_state_list(
> target_stop_cmd(cmd, &flags);
>
> cmd->transport_state |= CMD_T_ABORTED;
> +
> + /* CMD_T_SENT_STATUS: cmd is down in fabric layer.
> + * A check stop has been called. Keep the extra kref
> + * from above because core_tmr_handle_tas_abort will
> + * generate another check_stop.
> + *
> + * !CMD_T_SENT_STATUS: cmd intercepted at back end.
> + * Remove the extra kref from above because only
> + * 1 check_stop is required or generated by
> + * core_tmr_handle_tas_abort()
> + */
> + rmkref = 0;
> + if (!((cmd->t_state == TRANSPORT_COMPLETE) &&
> + (cmd->transport_state & CMD_T_SENT_STATUS)))
> + rmkref = 1;
> +
> spin_unlock_irqrestore(&cmd->t_state_lock, flags);
>
> core_tmr_handle_tas_abort(tmr_nacl, cmd, tas);
> +
> + if (rmkref)
> + target_put_sess_cmd(cmd);
> }
> }
>
> diff --git a/drivers/target/target_core_transport.c
> b/drivers/target/target_core_transport.c
> index 4fdcee2..cdd18bf 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -639,9 +639,14 @@ void transport_cmd_finish_abort(struct se_cmd *cmd, int
> remove)
> static void target_complete_failure_work(struct work_struct *work)
> {
> struct se_cmd *cmd = container_of(work, struct se_cmd, work);
> + unsigned long flags;
>
> transport_generic_request_failure(cmd,
> TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE);
> +
> + spin_lock_irqsave(&cmd->t_state_lock, flags);
> + cmd->transport_state |= CMD_T_SENT_STATUS;
> + spin_unlock_irqrestore(&cmd->t_state_lock, flags);
> }
>
> /*
> @@ -1659,6 +1664,7 @@ void transport_generic_request_failure(struct se_cmd
> *cmd,
> sense_reason_t sense_reason)
> {
> int ret = 0, post_ret = 0;
> + unsigned long flags;
>
> pr_debug("-[ Storage Engine Exception for cmd: %p ITT: 0x%08llx"
> " CDB: 0x%02x\n", cmd, cmd->tag, cmd->t_task_cdb[0]);
> @
