Re: [PATCH] block devices: validate block device capacity
On Fri, Jan 31, 2014 at 03:20:17AM -0500, Mikulas Patocka wrote: So if you think you can support 16TiB devices and leave pgoff_t 32-bit, send a patch that does it. Until you make it, you should apply the patch that I sent, that prevents kernel lockups or data corruption when the user uses 16TiB device on 32-bit kernel. Exactly. I had actually looked into support for 16TiB devices for a NAS use case a while ago, but when explaining the effort involves the idea was dropped quickly. The Linux block device is too deeply tied to the pagecache to make it easily feasible. -- 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] target: Fix missing length check in spc_emulate_evpd_83()
From: Roland Dreier rol...@purestorage.com Commit fbfe858fea2a (target_core_spc: Include target device descriptor in VPD page 83) added a new length variable, but (due to a cut and paste mistake?) just checks scsi_name_len against 256 twice. Fix this to check scsi_target_len for overflow too. Signed-off-by: Roland Dreier rol...@purestorage.com --- drivers/target/target_core_spc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c index 43c5ca9878bc..3bebc71ea033 100644 --- a/drivers/target/target_core_spc.c +++ b/drivers/target/target_core_spc.c @@ -440,8 +440,8 @@ check_scsi_name: padding = ((-scsi_target_len) 3); if (padding) scsi_target_len += padding; - if (scsi_name_len 256) - scsi_name_len = 256; + if (scsi_target_len 256) + scsi_target_len = 256; buf[off-1] = scsi_target_len; off += scsi_target_len; -- 1.9.rc1 -- 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] trivial: scsi: Fix warning on make htmldocs caused by scsi_transport_srp.c
This patch fixed following warnings on make htmldocs. Warning(/drivers/scsi/scsi_transport_srp.c:819): No description found for parameter 'rport' Signed-off-by: Masanari Iida standby2...@gmail.com --- drivers/scsi/scsi_transport_srp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c index d47ffc8..664eb38 100644 --- a/drivers/scsi/scsi_transport_srp.c +++ b/drivers/scsi/scsi_transport_srp.c @@ -811,6 +811,8 @@ EXPORT_SYMBOL_GPL(srp_remove_host); /** * srp_stop_rport_timers - stop the transport layer recovery timers * + * @rport: SRP remote port to be removed + * * Must be called after srp_remove_host() and scsi_remove_host(). The caller * must hold a reference on the rport (rport-dev) and on the SCSI host * (rport-dev.parent). -- 1.9.rc1 -- 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] st: Do not rewind for SG_IO
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 2/2/2014 5:42 AM, Hannes Reinecke wrote: This is due to the strictly sequential design udev has. Essentially udev spawns a worker for every device which gets created (= udev receives a uevent for). The part I fail to see in this explanation is why the nst/st/st*a/st*m/etc handles are being treated as separate devices. They aren't. They are all the same physical tape device, so why are the nst devices being handled separately from the st ones? Maybe the problem is that you have to many workers for the tape device. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJS76ykAAoJEL5i86xrzcy7MKMH/0upLuOBOJWzdOfKYq0WmIvZ eIaaZG2vhckYeS2zZtim5uVFQDp5eTOirmjxfwSGzSTSAmNrQJwzZvBO/vA2/Kqk wZSKXp/ZGZhw11+6Kg8f1EArQQwT/i3R6BKglLELFvZVvNOUg3KCnd6nE/4k7ysh H3f6+6/Jb1wUA6h7a65BG7VBQlJ3HqVe01vYTrkb3eYW7IWfN0tX2FMdqYt2zon2 Yo6TRxhTE/dmqJhg8nLB+fA8rUwW7CYU/IX8nsKNn9lPaDdoJ6g22ozpJRrtEZZ+ lt/qL3VxfWu38z0GWhuKuOqY969bMlyaphY7bOgf7LY4osiC7OgarVoxSIgfP9E= =pIPt -END PGP SIGNATURE- -- 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] trivial: scsi: Fix warning on make htmldocs caused by scsi_transport_srp.c
On 02/03/14 15:34, Masanari Iida wrote: This patch fixed following warnings on make htmldocs. Warning(/drivers/scsi/scsi_transport_srp.c:819): No description found for parameter 'rport' Signed-off-by: Masanari Iida standby2...@gmail.com --- drivers/scsi/scsi_transport_srp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c index d47ffc8..664eb38 100644 --- a/drivers/scsi/scsi_transport_srp.c +++ b/drivers/scsi/scsi_transport_srp.c @@ -811,6 +811,8 @@ EXPORT_SYMBOL_GPL(srp_remove_host); /** * srp_stop_rport_timers - stop the transport layer recovery timers * + * @rport: SRP remote port to be removed + * * Must be called after srp_remove_host() and scsi_remove_host(). The caller * must hold a reference on the rport (rport-dev) and on the SCSI host * (rport-dev.parent). The comment added by this patch is wrong. That is easy to see by having a look at the function name and function description. In case anyone is wondering why this kerneldoc warning was not addressed by commit 0c7f821: that patch was prepared against kernel v3.12-rc7 and the warning reported above is new in kernel v3.14-rc1. Bart. -- 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: mpt2sas driver barfs when force removing a drive on 3.13.1
On Fri, 31 Jan 2014 20:49:39 -0600 Matthew Thode prometheanf...@gentoo.org wrote: I decided to pull a drive while it was in use out of laziness (it was open via luks, but not in actual use). Got a fun trace as a result. Just thought you'd like to know :D Hi Matthew, The first trace looks a lot like what I see on mpt2sas driver removal [1]. I posted a suggested fix back in Dec [2], which you might try, however it is not reviewed at this point. [1] http://thread.gmane.org/gmane.linux.scsi/86237 [2] https://github.com/joe-lawrence/linux/compare/scsi_transport_sas_sysfs_warning.patch Regards, -- Joe -- 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] st: Do not rewind for SG_IO
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/03/2014 03:50 PM, Jeremy Linton wrote: On 2/2/2014 5:42 AM, Hannes Reinecke wrote: This is due to the strictly sequential design udev has. Essentially udev spawns a worker for every device which gets created (= udev receives a uevent for). The part I fail to see in this explanation is why the nst/st/st*a/st*m/etc handles are being treated as separate devices. They aren't. They are all the same physical tape device, so why are the nst devices being handled separately from the st ones? That's due to udev. Udev is getting events for each device it should create a device node for. So for 'st' it'll get a series of events for 'stX', and another series of events for 'nstX'. Udev will treat each of these events separately, with distinct worker programs handling them. Each of those workers run fully asynchronous and cannot access information from other workers. Udev cannot and should not make assumptions about any correlations between events; if you need this then you have to explicitly gate events via the 'collect' mechanism. But even then you can only access information from the currently running process/event; you cannot modify information from prior events. So when trying to generate unique IDs for tape devices you either have to a) modify the program generating the Unique ID to redirect the call to a different device node or b) to modify the driver to not rewind the tape We tried approach a) when using the scsi_id program. But as this program is rather old and doesn't have a maintainer I'm working on using 'sg_inq' as a replacement. And these kind of hacks really do not belong in a generic program. (It would involve create a temporary device node, call SG_IO on that, and remove the temporary device node again). So now we're shooting for b). Unless you have a better idea ... 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) -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJS77BsAAoJEGz4yi9OyKjPQz8P/1cY+k9KtEezXC2ovuzJmy9T BjQaoxcTgHbaXykFyEoJeiNQ09XrjAl7IlVOOF9jX3XqC4kYQiBZCThUnC8O3D69 vaZrA6+VR9AoTXbfNzBUdHzEKWTlz5X2ThLzrLPogXS6v5hiw0iBcLq5mvlJxlkc DBHJrV+ujlKOM/JibNwUNldeYAFnrrzCCH50arJ/Eqa6BcLZ2uXTksACqIVybUoI nW1yfVzJdsxI5YwPp0N5bnuAPIg3N7zBTcFCVYqTHUiAmrS4HYkHRrC501yuopCA PS9Gv1KznctEIGudoP5S0Lwn+g/aVSuzOTpo3uUWAa1BNyyvRMRUSzSIUfm2WdAj +o1mswhhX6NZMM9rFrM35dbiky2Nv/pvc2zcD4pvizsLiHkoQER7GQ9FCaBNXP7d eK0OF2jJeEPV6LSC97wxH27Vybv/c89wp0HGDaJc+kd8+WZFIvkLbvY7K3BSA2ut YvuebI59veIYNZEJVEdAGYQ3IEUG8uukbdpqf3Kdr17PQEms/gTW+xYWPo1+hBR7 V58ojgghBxPdVwGRyiSgKgjY/vHLFunm9H83LDrHKdQsKs8JGiTCflPhJEMwwsDo mhlmd5Ihw20Cyh46sd3Uu1onH0L5Rg0hfDqT913ShUFYi5Y+XvtHEezYn73OxYDr JoUo9HYfT843MabEympC =c45s -END PGP SIGNATURE- -- 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] st: Do not rewind for SG_IO
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 2/3/2014 9:06 AM, Hannes Reinecke wrote: That's due to udev. Udev is getting events for each device it should create a device node for. So for 'st' it'll get a series of events for 'stX', and another series of events for 'nstX'. Udev will treat each of these events separately, with distinct worker programs handling them. Each of those workers run fully asynchronous and cannot access information from other workers. So whats wrong with the simple solution? You throw the ones for st away, and create the st handles from the nst worker. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJS77DnAAoJEL5i86xrzcy7j+cH/24oncr+DN1yZ4ReXk3i8QHx LpI83UgP9EAvvcHJb5Op3IQtojccda1rYmecMS8qLYV0IX33lJg6UXbhkNS/skkR gFbPdsD/27JqJZvCU02U+ET0zfO5XH833UCRKOsqoA/GMeikLAaUKPV5t65eyCHh Qy4CYr4Gve9AxMhV9n00IdadOL5NoH/aO+Qb916zeJ2dUng5TUDqQ3WzdlQQdhD5 ObReHBnTBXlsWQdZL2VakP6gEX2ijiZ09GOIeSf1rKz/974OAudmzLsVjF4BwqTA 5JzqQXezYO2CZ8zkiuCCuiuXEwjv3f62rCuqzi5lQByFGDpvJNMZDAU9GjTn9Z8= =ELYk -END PGP SIGNATURE- -- 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/2] ipr: Use pci_enable_msi_range() and pci_enable_msix_range()
Acked-by: Brian King brk...@linux.vnet.ibm.com -- Brian King Power Linux I/O IBM Linux Technology Center -- 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/2] ipr: Get rid of superfluous call to pci_disable_msi/msix()
Acked-by: Brian King brk...@linux.vnet.ibm.com -- Brian King Power Linux I/O IBM Linux Technology Center -- 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] target: Fix missing length check in spc_emulate_evpd_83()
On Mon, 2014-02-03 at 00:35 -0800, Roland Dreier wrote: From: Roland Dreier rol...@purestorage.com Commit fbfe858fea2a (target_core_spc: Include target device descriptor in VPD page 83) added a new length variable, but (due to a cut and paste mistake?) just checks scsi_name_len against 256 twice. Fix this to check scsi_target_len for overflow too. Signed-off-by: Roland Dreier rol...@purestorage.com --- Applied. Thanks Roland! --nab drivers/target/target_core_spc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c index 43c5ca9878bc..3bebc71ea033 100644 --- a/drivers/target/target_core_spc.c +++ b/drivers/target/target_core_spc.c @@ -440,8 +440,8 @@ check_scsi_name: padding = ((-scsi_target_len) 3); if (padding) scsi_target_len += padding; - if (scsi_name_len 256) - scsi_name_len = 256; + if (scsi_target_len 256) + scsi_target_len = 256; buf[off-1] = scsi_target_len; off += scsi_target_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
[PATCH 10/12] scsi: use device_remove_file_self() instead of device_schedule_callback()
driver-core now supports synchrnous self-deletion of attributes and
the asynchrnous removal mechanism is scheduled for removal. Use it
instead of device_schedule_callback(). This makes delete behave
synchronously.
Signed-off-by: Tejun Heo t...@kernel.org
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/scsi_sysfs.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 9117d0b..8ead24c 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -649,23 +649,12 @@ store_rescan_field (struct device *dev, struct
device_attribute *attr,
}
static DEVICE_ATTR(rescan, S_IWUSR, NULL, store_rescan_field);
-static void sdev_store_delete_callback(struct device *dev)
-{
- scsi_remove_device(to_scsi_device(dev));
-}
-
static ssize_t
sdev_store_delete(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
- int rc;
-
- /* An attribute cannot be unregistered by one of its own methods,
-* so we have to use this roundabout approach.
-*/
- rc = device_schedule_callback(dev, sdev_store_delete_callback);
- if (rc)
- count = rc;
+ if (device_remove_file_self(dev, attr))
+ scsi_remove_device(to_scsi_device(dev));
return count;
};
static DEVICE_ATTR(delete, S_IWUSR, NULL, sdev_store_delete);
--
1.8.5.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] block devices: validate block device capacity
On Mon, 3 Feb 2014, Christoph Hellwig wrote: On Fri, Jan 31, 2014 at 03:20:17AM -0500, Mikulas Patocka wrote: So if you think you can support 16TiB devices and leave pgoff_t 32-bit, send a patch that does it. Until you make it, you should apply the patch that I sent, that prevents kernel lockups or data corruption when the user uses 16TiB device on 32-bit kernel. Exactly. I had actually looked into support for 16TiB devices for a NAS use case a while ago, but when explaining the effort involves the idea was dropped quickly. The Linux block device is too deeply tied to the pagecache to make it easily feasible. The memory management routines use pgoff_t, so we could define pgoff_t to be 64-bit type. But there is lib/radix_tree.c that uses unsigned long as an index into the radix tree - and pgoff_t is cast to unsigned long when calling the radix_tree routines - so we'd need to change lib/radix_tree to use pgoff_t. Then, there may be other places where pgoff_t is cast to unsigned long and they are not trivial to find (one could enable some extra compiler warnings about truncating values when casting them, but I suppose, this would trigger a lot of false positives). This needs some deep review by people who designed the memory management code. Mikulas -- 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] st: Do not rewind for SG_IO
On Mon, Feb 3, 2014 at 4:08 PM, Jeremy Linton jlin...@tributary.com wrote: On 2/3/2014 9:06 AM, Hannes Reinecke wrote: That's due to udev. Udev is getting events for each device it should create a device node for. So for 'st' it'll get a series of events for 'stX', and another series of events for 'nstX'. Udev will treat each of these events separately, with distinct worker programs handling them. Each of those workers run fully asynchronous and cannot access information from other workers. So whats wrong with the simple solution? You throw the ones for st away, and create the st handles from the nst worker. This is not simple and not going to happen. Sibling devices in /sys cannot have a relationship in udev, there are only parent/child dependencies. Hannes, can't you just drop the weird auto-rewinding device matches from the persistent rules, is that really useful today? Kay -- 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] st: Do not rewind for SG_IO
On Mon, 2014-02-03 at 21:51 +0100, Kay Sievers wrote: On Mon, Feb 3, 2014 at 4:08 PM, Jeremy Linton jlin...@tributary.com wrote: On 2/3/2014 9:06 AM, Hannes Reinecke wrote: That's due to udev. Udev is getting events for each device it should create a device node for. So for 'st' it'll get a series of events for 'stX', and another series of events for 'nstX'. Udev will treat each of these events separately, with distinct worker programs handling them. Each of those workers run fully asynchronous and cannot access information from other workers. So whats wrong with the simple solution? You throw the ones for st away, and create the st handles from the nst worker. This is not simple and not going to happen. Sibling devices in /sys cannot have a relationship in udev, there are only parent/child dependencies. Hannes, can't you just drop the weird auto-rewinding device matches from the persistent rules, is that really useful today? Regardless of the outcome of these discussions about adding a rewind ioctl, we can't just kill the auto-rewind devices because that would be an ABI break (the consequence would be people's backup scripts would break and nothing annoys sysadmins more than failed backups). 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] st: Do not rewind for SG_IO
On 14-02-03 10:08 AM, Jeremy Linton wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 2/3/2014 9:06 AM, Hannes Reinecke wrote: That's due to udev. Udev is getting events for each device it should create a device node for. So for 'st' it'll get a series of events for 'stX', and another series of events for 'nstX'. Udev will treat each of these events separately, with distinct worker programs handling them. Each of those workers run fully asynchronous and cannot access information from other workers. So whats wrong with the simple solution? You throw the ones for st away, and create the st handles from the nst worker. Doesn't seem to be any good solutions to this problem. If you can't discovery something without modifying its state, then what? For udev and/or sg_inq to know about the special relationship between sti nodes and nsti nodes seems unreasonable IMO. The cleanest way I can think of is for the st driver to show this relationship via sysfs. But then why should udev going mining for that relationship? A sysfs flag from the st driver indicating a node is undiscoverable might be a start. A possible hack inside the st driver is to notice that only the SG_IO ioctl is called on a file handle between st_open(/dev/st*, O_RDONLY|O_NONBLOCK) and st_release() then not auto-rewind it. BTW Recent versions of sg_inq in Linux use open(dev, O_RDONLY|O_NONBLOCK) unless 'sg_inq --block=1' is given in which case open(dev, O_RDONLY) is used. I just noticed that when scsi_debug makes a tape device, the st driver silently ignores it, probably because scsi_debug doesn't support a SSC command that st expects it to respond to. IMO the st driver should make some noise if it ever ignores a SCSI device with a PDT of 1 (i.e. a tape). For example: # lsscsi -g [0:0:0:0]diskATA INTEL SSDSC2CW12 400i /dev/sda /dev/sg0 [8:0:0:0]tapeLinuxscsi_debug 0004 - /dev/sg1 with the st module loaded and no indication in dmesg. Doug Gilbert -- 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] st: Do not rewind for SG_IO
On Mon, Feb 3, 2014 at 10:16 PM, Douglas Gilbert dgilb...@interlog.com wrote: On 14-02-03 10:08 AM, Jeremy Linton wrote: So whats wrong with the simple solution? You throw the ones for st away, and create the st handles from the nst worker. Doesn't seem to be any good solutions to this problem. If you can't discovery something without modifying its state, then what? For udev and/or sg_inq to know about the special relationship between sti nodes and nsti nodes seems unreasonable IMO. The cleanest way I can think of is for the st driver to show this relationship via sysfs. But then why should udev going mining for that relationship? A sysfs flag from the st driver indicating a node is undiscoverable might be a start. Udev rules explicitely match on device names, that should be fine. It can just not be included in the match if it does not provide the needed functionality, or if it is not needed. Kay -- 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] st: Do not rewind for SG_IO
On 2/3/2014 2:51 PM, Kay Sievers wrote: This is not simple and not going to happen. Sibling devices in /sys cannot have a relationship in udev, there are only parent/child dependencies. Ok.. so if we can't ignore all the spare device nodes in a given /sys entry for a given device. Why open the device to scan it? I've often wondered why the serial number isn't part of the data in /sys along with the manufacture/model. The last tape drive I saw that failed to respond to inquiry page 0x80 was over a decade ago (probably manufactured in the early 90s). So enabling it just for tape is pretty safe. Matching Manufacturer/model/serial is going to be better than anything your going to get out of 0x83 anyway. That data is guaranteed to be there, but its also guaranteed to be unreliable (every device, and every port has a slightly different set of descriptors they choose to support). Plus, your not going to have issues accidentally rewinding a device, or resetting a tape density, or accidentally turning compression off if you don't open the device. Hannes, can't you just drop the weird auto-rewinding device matches from the persistent rules, is that really useful today? The relationship between the st and nst devices is leveraged by a large number of backup applications in the field. If you change it, its likely lots of breakage will ensue. -- 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] target: Simplify command completion by removing CMD_T_FAILED flag
From: Roland Dreier rol...@purestorage.com
The CMD_T_FAILED flag is set used in one place to record the result of a
trivial test, and it is only tested once, few lines later. We might as
well make the code simpler and easier to read by directly doing the test
of success where we want to use it.
Signed-off-by: Roland Dreier rol...@purestorage.com
---
drivers/target/target_core_transport.c | 5 +
include/target/target_core_base.h | 1 -
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/target/target_core_transport.c
b/drivers/target/target_core_transport.c
index c50fd9f11aab..24b4f65d8777 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -669,9 +669,6 @@ void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
return;
}
- if (!success)
- cmd-transport_state |= CMD_T_FAILED;
-
/*
* Check for case where an explicit ABORT_TASK has been received
* and transport_wait_for_tasks() will be waiting for completion..
@@ -681,7 +678,7 @@ void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
spin_unlock_irqrestore(cmd-t_state_lock, flags);
complete(cmd-t_transport_stop_comp);
return;
- } else if (cmd-transport_state CMD_T_FAILED) {
+ } else if (!success) {
INIT_WORK(cmd-work, target_complete_failure_work);
} else {
INIT_WORK(cmd-work, target_complete_ok_work);
diff --git a/include/target/target_core_base.h
b/include/target/target_core_base.h
index c9c791209cd1..1772fadcff62 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -525,7 +525,6 @@ struct se_cmd {
#define CMD_T_COMPLETE (1 2)
#define CMD_T_SENT (1 4)
#define CMD_T_STOP (1 5)
-#define CMD_T_FAILED (1 6)
#define CMD_T_DEV_ACTIVE (1 7)
#define CMD_T_REQUEST_STOP (1 8)
#define CMD_T_BUSY (1 9)
--
1.9.rc1
--
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] st: Do not rewind for SG_IO
On Mon, Feb 3, 2014 at 10:58 PM, Jeremy Linton jlin...@tributary.com wrote: On 2/3/2014 2:51 PM, Kay Sievers wrote: This is not simple and not going to happen. Sibling devices in /sys cannot have a relationship in udev, there are only parent/child dependencies. Ok.. so if we can't ignore all the spare device nodes in a given /sys entry for a given device. Why open the device to scan it? I've often wondered why the serial number isn't part of the data in /sys along with the manufacture/model. The last tape drive I saw that failed to respond to inquiry page 0x80 was over a decade ago (probably manufactured in the early 90s). So enabling it just for tape is pretty safe. Matching Manufacturer/model/serial is going to be better than anything your going to get out of 0x83 anyway. That data is guaranteed to be there, but its also guaranteed to be unreliable (every device, and every port has a slightly different set of descriptors they choose to support). Plus, your not going to have issues accidentally rewinding a device, or resetting a tape density, or accidentally turning compression off if you don't open the device. Well, the predictable symlink for tapes stuff is all pretty old, and got through quite a few changes over the years. Not sure how much the /dev/tape/by-*/ links are really used in the field, and what people really expect here. (Mis-)using one of the various open() flags which do not make sense for SG_IO or tapes, which would prevent any state changes still sounds like the most convincing option to me. In the end it's always the nicer interface if the device node itself can be used to identify the device, instead of jumping through /sys or adding more proping caches to the kernel. Hannes, can't you just drop the weird auto-rewinding device matches from the persistent rules, is that really useful today? The relationship between the st and nst devices is leveraged by a large number of backup applications in the field. If you change it, its likely lots of breakage will ensue. Sure, but do they really expect the additional symlinks udev creates? We are not talking about the primary device nodes, only about the additional /dev/tape/by-*/ links. Kay -- 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] bio_integrity_add_page: check for BIO_POOL_NONE before determining nr_vecs on slab
David == David Milburn dmilb...@redhat.com writes:
David When enabling DIX T10-DIF-TYPE1-IP protection you can hit the
David bip_vec full condition which fails to attach the integrity
David metadata and returns 0 back to bio_integrity_prep()
Looks like Kent accidentally broke this when he changed the bvec pool
setup.
David - if (bip-bip_vcnt = bvec_nr_vecs(bip-bip_slab)) {
David + if (bip-bip_slab != BIO_POOL_NONE
David + bip-bip_vcnt = bvec_nr_vecs(bip-bip_slab)) {
David printk(KERN_ERR %s: bip_vec full\n, __func__);
David return 0;
David }
We still need to check that the page will actually fit, though:
block: Fix nr_vecs for inline integrity vectors
Commit 9f060e2231ca changed the way we handle allocations for the
integrity vectors. When the vectors are inline there is no associated
slab and consequently bvec_nr_vecs() returns 0. Ensure that we check
against BIP_INLINE_VECS in that case.
Reported-by: David Milburn dmilb...@redhat.com
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
index fc60b31453ee..6dea2b90b4d5 100644
--- a/fs/bio-integrity.c
+++ b/fs/bio-integrity.c
@@ -114,6 +114,14 @@ void bio_integrity_free(struct bio *bio)
}
EXPORT_SYMBOL(bio_integrity_free);
+static inline unsigned int bip_integrity_vecs(struct bio_integrity_payload
*bip)
+{
+ if (bip-bip_slab == BIO_POOL_NONE)
+ return BIP_INLINE_VECS;
+
+ return bvec_nr_vecs(bip-bip_slab);
+}
+
/**
* bio_integrity_add_page - Attach integrity metadata
* @bio: bio to update
@@ -129,7 +137,7 @@ int bio_integrity_add_page(struct bio *bio, struct page
*page,
struct bio_integrity_payload *bip = bio-bi_integrity;
struct bio_vec *iv;
- if (bip-bip_vcnt = bvec_nr_vecs(bip-bip_slab)) {
+ if (bip-bip_vcnt = bip_integrity_vecs(bip)) {
printk(KERN_ERR %s: bip_vec full\n, __func__);
return 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
Re: [PATCH v2 0/7] scsi_debug: several bug fixes and enable clustering support
Doug == Douglas Gilbert dgilb...@interlog.com writes: Doug The parts of this patch that I understand look fine. Perhaps Doug Martin should ack it, if he sees fit. Just had another look at the updated pieces. Patch series looks good to me. Reviewed-by: Martin K. Petersen martin.peter...@oracle.com -- Martin K. Petersen Oracle Linux Engineering -- 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
