Re: [PATCH] tgt: Use scsi_init_io instead of scsi_alloc_sgtable
On Sat, Jan 05 2008 at 1:02 +0200, James Bottomley [EMAIL PROTECTED] wrote:
On Thu, 2007-12-13 at 13:44 +0200, Boaz Harrosh wrote:
- If we export scsi_init_io()/scsi_release_buffers() instead of
scsi_{alloc,free}_sgtable() from scsi_lib than tgt code is
much more insulated from scsi_lib changes. As a bonus it will
also gain bidi capability when it comes.
Signed-off-by: Boaz Harrosh [EMAIL PROTECTED]
Acked-by: FUJITA Tomonori [EMAIL PROTECTED]
I'm afraid the reversion of -done removal broke the application of this
yet again ... could you respin.
Thanks,
James
It looks like the revert is to be reverted ;)
I have rebased, but should I wait a while to
see if they're needed?
Boaz
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tgt: Use scsi_init_io instead of scsi_alloc_sgtable
On Sun, 2008-01-06 at 19:08 +0200, Boaz Harrosh wrote:
On Sat, Jan 05 2008 at 1:02 +0200, James Bottomley [EMAIL PROTECTED] wrote:
On Thu, 2007-12-13 at 13:44 +0200, Boaz Harrosh wrote:
- If we export scsi_init_io()/scsi_release_buffers() instead of
scsi_{alloc,free}_sgtable() from scsi_lib than tgt code is
much more insulated from scsi_lib changes. As a bonus it will
also gain bidi capability when it comes.
Signed-off-by: Boaz Harrosh [EMAIL PROTECTED]
Acked-by: FUJITA Tomonori [EMAIL PROTECTED]
I'm afraid the reversion of -done removal broke the application of this
yet again ... could you respin.
Thanks,
James
It looks like the revert is to be reverted ;)
I have rebased, but should I wait a while to
see if they're needed?
Yes, let's just wait a while to see what the outcome is ...
James
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][SCSI] megaraid: Convert from scsi.h to scsi.h (and friends)
On Sun, 2008-01-06 at 20:03 +0100, Richard Knutsson wrote: Convert glue-include scsi.h to scsi.h (and friends). (binary sizes) allyesconfig: before: 260132 after: 260048 allmodconfig: before: 261740 after: 261656 Signed-off-by: Richard Knutsson [EMAIL PROTECTED] --- Do not have the hardware, but since it compiles I hope it is alright. diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c index 66c6520..9f1e2c5 100644 --- a/drivers/scsi/megaraid.c +++ b/drivers/scsi/megaraid.c @@ -48,8 +48,9 @@ #include linux/dma-mapping.h #include scsi/scsicam.h -#include scsi.h +#include scsi/scsi_cmnd.h #include scsi/scsi_host.h +#include scsi.h I'm afraid this is pretty much wrong. The scsi.h being referred to here is the local scsi.h in the build directory, so it should be included as a string. For #include, ... means begin the search in the local directory (what is wanted here) and ... means begin the search starting with the predefined include paths. The rest of the patch looks like a spurious (unrelated and undescribed) downcasing of TRUE and FALSE. James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][SCSI] megaraid: Convert from scsi.h to scsi.h (and friends)
Citerar James Bottomley [EMAIL PROTECTED]:
On Sun, 2008-01-06 at 20:03 +0100, Richard Knutsson wrote:
Convert glue-include scsi.h to scsi.h (and friends).
(binary sizes)
allyesconfig: before: 260132
after: 260048
allmodconfig: before: 261740
after: 261656
Signed-off-by: Richard Knutsson [EMAIL PROTECTED]
---
Do not have the hardware, but since it compiles I hope it is alright.
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 66c6520..9f1e2c5 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -48,8 +48,9 @@
#include linux/dma-mapping.h
#include scsi/scsicam.h
-#include scsi.h
+#include scsi/scsi_cmnd.h
#include scsi/scsi_host.h
+#include scsi.h
I'm afraid this is pretty much wrong. The scsi.h being referred to here
is the local scsi.h in the build directory, so it should be included as
a string. For #include, ... means begin the search in the local
directory (what is wanted here) and ... means begin the search
starting with the predefined include paths.
Oops, it were suppose to read +#include scsi/scsi.h. I thought it always
went to include/ if you used ... (which then would had given an error). Oh
well, it compiles again when also adding #include scsi/scsi_device.h (used,
at least, in the scsi_cmnd-structure).
The rest of the patch looks like a spurious (unrelated and undescribed)
downcasing of TRUE and FALSE.
Since TRUE/FALSE are defined in scsi.h, they were converted to the generic
names.
Do this looks alright? Forgot it were dependent on the patch-set who converted
all Scsi_Cmnd to struct scsi_cmnd.
---
Convert compatibility-glue-include scsi.h to scsi/scsi.h and friends.
Signed-off-by: Richard Knutsson [EMAIL PROTECTED]
---
Do not have the hardware, but since it compiles I hope it is alright.
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 66c6520..5e9edfd 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -48,8 +48,10 @@
#include linux/dma-mapping.h
#include scsi/scsicam.h
-#include scsi.h
+#include scsi/scsi_cmnd.h
+#include scsi/scsi_device.h
#include scsi/scsi_host.h
+#include scsi/scsi.h
#include megaraid.h
@@ -359,7 +361,7 @@ mega_runpendq(adapter_t *adapter)
* The command queuing entry point for the mid-layer.
*/
static int
-megaraid_queue(Scsi_Cmnd *scmd, void (*done)(Scsi_Cmnd *))
+megaraid_queue(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
{
adapter_t *adapter;
scb_t *scb;
@@ -411,7 +413,7 @@ megaraid_queue(Scsi_Cmnd *scmd, void (*done)(Scsi_Cmnd *))
* commands.
*/
static inline scb_t *
-mega_allocate_scb(adapter_t *adapter, Scsi_Cmnd *cmd)
+mega_allocate_scb(adapter_t *adapter, struct scsi_cmnd *cmd)
{
struct list_head *head = adapter-free_list;
scb_t *scb;
@@ -443,7 +445,7 @@ mega_allocate_scb(adapter_t *adapter, Scsi_Cmnd *cmd)
* and the channel number.
*/
static inline int
-mega_get_ldrv_num(adapter_t *adapter, Scsi_Cmnd *cmd, int channel)
+mega_get_ldrv_num(adapter_t *adapter, struct scsi_cmnd *cmd, int channel)
{
int tgt;
int ldrv_num;
@@ -506,7 +508,7 @@ mega_get_ldrv_num(adapter_t *adapter, Scsi_Cmnd *cmd, int
channel)
* boot settings.
*/
static scb_t *
-mega_build_cmd(adapter_t *adapter, Scsi_Cmnd *cmd, int *busy)
+mega_build_cmd(adapter_t *adapter, struct scsi_cmnd *cmd, int *busy)
{
mega_ext_passthru *epthru;
mega_passthru *pthru;
@@ -944,7 +946,7 @@ mega_build_cmd(adapter_t *adapter, Scsi_Cmnd *cmd, int
*busy)
* prepare a command for the scsi physical devices.
*/
static mega_passthru *
-mega_prepare_passthru(adapter_t *adapter, scb_t *scb, Scsi_Cmnd *cmd,
+mega_prepare_passthru(adapter_t *adapter, scb_t *scb, struct scsi_cmnd *cmd,
int channel, int target)
{
mega_passthru *pthru;
@@ -1008,7 +1010,7 @@ mega_prepare_passthru(adapter_t *adapter, scb_t *scb,
Scsi_Cmnd *cmd,
* commands for devices which can take extended CDBs (10 bytes)
*/
static mega_ext_passthru *
-mega_prepare_extpassthru(adapter_t *adapter, scb_t *scb, Scsi_Cmnd *cmd,
+mega_prepare_extpassthru(adapter_t *adapter, scb_t *scb, struct scsi_cmnd *cmd,
int channel, int target)
{
mega_ext_passthru *epthru;
@@ -1410,7 +1412,7 @@ mega_cmd_done(adapter_t *adapter, u8 completed[], int
nstatus, int status)
{
mega_ext_passthru *epthru = NULL;
struct scatterlist *sgl;
- Scsi_Cmnd *cmd = NULL;
+ struct scsi_cmnd*cmd = NULL;
mega_passthru *pthru = NULL;
mbox_t *mbox = NULL;
u8 c;
@@ -1662,14 +1664,14 @@ mega_cmd_done(adapter_t *adapter, u8 completed[], int
nstatus, int status)
static void
mega_rundoneq (adapter_t *adapter)
{
- Scsi_Cmnd *cmd;
+ struct scsi_cmnd *cmd;
struct list_head *pos;
Re: [PATCH 0/33][SCSI] Arrange for removal of 'scsi_typedefs.h'
James Bottomley wrote: On Sun, 2008-01-06 at 04:02 +0100, Richard Knutsson wrote: Hi all drivers/scsi/scsi_typedefs.h is about ready to be removed, only the 'struct scsi_cmnd' typedef Scsi_Cmnd is left. This set converts all the Scsi_Cmnd's, except in: Changelogs Documentation/scsi/scsi_mid_low_api.txt So if all these gets merged, in one form or the other, we can: un'include scsi_typedefs.h from drivers/scsi/scsi.h fix the text in Documentation/scsi/scsi_mid_low_api.txt (finally) remove the file in question Left the over 80 characters wide-warnings alone, until requested otherwise. Am not subscribed on linux-scsi@vger.kernel.org, so if replying, please Cc: me. Actually, a lot of this is superfluous ... the drivers are already removed in the scsi-misc-2.6 tree. When are they be expected to be removed in the mainstream? The ideal is to remove this line: #include scsi.h Because that file (and scsi_typedefs.h) should be expendable by the end of all of this (their whole purpose was so we didn't have to convert all the typedefs), and then check the thing still compiles---you probably have to add in some of the standard #include scsi/scsi_.. files. Sounds reasonable. Please also don't bother with downcasing Scsi_Cmnd capitalisation in the comments, that's a quirk I'm happy to leave, since it's unambiguous in the doc book. Ok, not like it is a real bother can't imagine this would be a possible source for collision with other patches, but I don't mind either way. Thanks Richard Knutsson - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] convert semaphore to mutex in struct class
On Sun, Jan 06, 2008 at 07:41:29PM +0100, Stefan Richter wrote:
Dave Young wrote:
Convert semaphore to mutex in struct class.
All the patches in this series should be applyed simultaneously
Therefore you eventually need to repost it as a single patch. It can't
go into one of the maintainer trees otherwise, because they only take
fully bijectable patches. (Kernel must build and run at any point in
between any patch series.)
toc:
---
1-driver-core-struct-class-convert-semaphore-to-mutex.patch
2-i2c-struct-class-convert-semaphore-to-mutex.patch
3-ieee1394-struct-class-convert-semaphore-to-mutex.patch
4-power-struct-class-convert-semaphore-to-mutex.patch
5-rtc-struct-class-convert-semaphore-to-mutex.patch
6-scsi-struct-class-convert-semaphore-to-mutex.patch
7-spi-struct-class-convert-semaphore-to-mutex.patch
I was going to test it at runtime on top of 2.6.24-rc6, but the first
and second patch depend on stuff in -mm. So, my laziness wins for now,
as -mm is not my cup of tea.
About your changelog:
xyz: convert semaphore to mutex in struct class
Use mutex instead of semaphore in struct class.
Signed-off-by: Dave Young [EMAIL PROTECTED]
You don't need the second line because it says the same as the first
line. Either kill it, or replace it by an explanation _why_ the
semaphore is to be replaced by mutex. (I guess you do it because they
are lighter-weight, both in semantics and in implementation, and because
there are better facilities to debug mutexes...)
Thanks for your comment, I rewrite it for 2.6.24-rc7 as a all-in-one patch,
please see following. Drop i2c maintainer and list in cc because there's no
changes about i2c in this patch:
Convert semaphore to mutex in struct class
Signed-off-by: Dave Young [EMAIL PROTECTED]
---
drivers/base/class.c | 22 ++--
drivers/base/core.c | 18 +++--
drivers/ieee1394/nodemgr.c| 40 +++---
drivers/power/apm_power.c |6 ++---
drivers/power/power_supply_core.c |8 +++
drivers/rtc/interface.c |4 +--
drivers/scsi/hosts.c |4 +--
drivers/spi/spi.c |4 +--
include/linux/device.h|3 +-
9 files changed, 54 insertions(+), 55 deletions(-)
diff -upr a/linux-2.6.24-rc7/drivers/base/class.c
linux-2.6.24-rc7/drivers/base/class.c
--- a/linux-2.6.24-rc7/drivers/base/class.c 2008-01-07 08:56:05.0
+0800
+++ linux-2.6.24-rc7/drivers/base/class.c 2008-01-07 09:12:49.0
+0800
@@ -144,7 +144,7 @@ int class_register(struct class * cls)
INIT_LIST_HEAD(cls-devices);
INIT_LIST_HEAD(cls-interfaces);
kset_init(cls-class_dirs);
- init_MUTEX(cls-sem);
+ mutex_init(cls-mutex);
error = kobject_set_name(cls-subsys.kobj, %s, cls-name);
if (error)
return error;
@@ -617,13 +617,13 @@ int class_device_add(struct class_device
kobject_uevent(class_dev-kobj, KOBJ_ADD);
/* notify any interfaces this device is now here */
- down(parent_class-sem);
+ mutex_lock_nested(parent_class-mutex, SINGLE_DEPTH_NESTING);
list_add_tail(class_dev-node, parent_class-children);
list_for_each_entry(class_intf, parent_class-interfaces, node) {
if (class_intf-add)
class_intf-add(class_dev, class_intf);
}
- up(parent_class-sem);
+ mutex_unlock(parent_class-mutex);
goto out1;
@@ -725,12 +725,12 @@ void class_device_del(struct class_devic
struct class_interface *class_intf;
if (parent_class) {
- down(parent_class-sem);
+ mutex_lock(parent_class-mutex);
list_del_init(class_dev-node);
list_for_each_entry(class_intf, parent_class-interfaces, node)
if (class_intf-remove)
class_intf-remove(class_dev, class_intf);
- up(parent_class-sem);
+ mutex_unlock(parent_class-mutex);
}
if (class_dev-dev) {
@@ -772,14 +772,14 @@ void class_device_destroy(struct class *
struct class_device *class_dev = NULL;
struct class_device *class_dev_tmp;
- down(cls-sem);
+ mutex_lock(cls-mutex);
list_for_each_entry(class_dev_tmp, cls-children, node) {
if (class_dev_tmp-devt == devt) {
class_dev = class_dev_tmp;
break;
}
}
- up(cls-sem);
+ mutex_unlock(cls-mutex);
if (class_dev)
class_device_unregister(class_dev);
@@ -812,7 +812,7 @@ int class_interface_register(struct clas
if (!parent)
return -EINVAL;
- down(parent-sem);
+ mutex_lock(parent-mutex);
list_add_tail(class_intf-node, parent-interfaces);
if
Re: [PATCH 0/7] sg_ring: a ring of scatterlist arrays
On Sunday 06 January 2008 02:31:12 James Bottomley wrote: On Wed, 2007-12-19 at 17:31 +1100, Rusty Russell wrote: This patch series is the start of my attempt to simplify and make explicit the chained scatterlist logic. It's not complete: my SATA box boots and seems happy, but all the other users of SCSI need to be updated and checked. But I've gotten far enough to believe it's worth persuing. Sorry for the delay in looking at this, I was busy with Holidays and things. Thankyou for your consideration. When I compare sg_ring with the current sg_chain (and later sg_table) implementations, I'm actually struck by how similar they are. I agree, they're solving the same problem. It is possible that the pain of change is no longer worthwhile, but I hate to see us give up on this. We're adding complexity without making it harder to misuse. The other thing I note is that the problem you're claiming to solve with sg_ring (the ability to add extra scatterlists to the front or the back of an existing one) is already solved with sg_chain, so the only real advantage of sg_ring was that it contains explicit counts, which sg_table (in -mm) also introduces. I just converted virtio using latest Linus for fair comparison, and it's still pretty ugly. sg_ring needs more work to de-scsi it. sg_table is almost sg_ring except it retains all the chaining warts. But we hit the same problems: 1) sg_chain loses information. The clever chain packaging makes reading easy, but manipulation is severely limited. You can append to your own chains by padding, but not someone elses. This works for SCSI, but what about the rest of us? And don't even think of joining mapped chains: it will almost work. 2) sg_chain's end marker is only for reading non-dma elements, not for mapped chains, nor for writing into chains. Plus it's a duplicate of the num arg, which is still passed around for efficiency. (virtio had to implement count_sg() because I didn't want those two redundant num args). 3) By overloading struct scatterlist, it's invisible what code has been converted to chains, and which hasn't. Too clever by half! Look at sg_chain(): it claims to join two scatterlists, but it doesn't. It assumes that prv is an array, not a chain. Because you've overloaded an existing type, this happens everywhere. Try passing skb_to_sgvec a chained skb. sg_ring would not have required any change to existing drivers, just those that want to use long sg lists. And then it's damn obvious which are which. 4) sg_chain missed a chance to combine all the relevent information (max, num, num_dma and the sg array). sg_table comes close, but you still can't join them (no max information, and even if there were, what if it's full?). Unlike sg_ring, it's unlikely to reduce bugs. 5) (A little moot now) sg_ring didn't require arch changes. The other differences are that sg_ring only allows adding at the front or back of an existing sg_ring, it doesn't allow splicing at any point like sg_chain does, so I'd say it's less functional (not that I actually want anyone ever to do this, of course ...) Well it's just as possible, but you might have to copy more elements (with sg chaining you need only copy 1 sg element to make room for the chain elem). Agreed that it's a little out there... The final point is that sg_ring requires a two level traversal: ring list then scatterlist, whereas sg_chain only requires a single level traversal. I grant that we can abstract out the traversal into something that would make users think they're only doing a single level, but I don't see what the extra level really buys us. We hide the real complexity from users and it makes it less safe for non-trivial cases. Hence the introduction of YA datastructure: sg_table. This is getting close: just hang the array off it and you'll have sg_ring and no requirement for dynamic alloc all the time. And once you have a header, no need for chaining tricks... The only thing missing from sg_chain perhaps is an accessor function that does the splicing, which I can easily construct if you want to try it out in virtio. I don't need that (I prepend and append), but it'd be nice if sg_next took a const struct scatterlist *. Cheers, Rusty. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] sg_ring: a ring of scatterlist arrays
Hello, Rusty Russell wrote: The other thing I note is that the problem you're claiming to solve with sg_ring (the ability to add extra scatterlists to the front or the back of an existing one) is already solved with sg_chain, so the only real advantage of sg_ring was that it contains explicit counts, which sg_table (in -mm) also introduces. I just converted virtio using latest Linus for fair comparison, and it's still pretty ugly. sg_ring needs more work to de-scsi it. sg_table is almost sg_ring except it retains all the chaining warts. I agree it's ugly. But we hit the same problems: 1) sg_chain loses information. The clever chain packaging makes reading easy, but manipulation is severely limited. You can append to your own chains by padding, but not someone elses. This works for SCSI, but what about the rest of us? And don't even think of joining mapped chains: it will almost work. You can append by allocating one more element on the chain to be appended and moving the last element of the first chain to it while using the last element for chaining. Join can be done by similar technique using three element extra chain in the middle. But, yeah, it's ugly as hell. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Don't blatt first element of prv in sg_chain()
Rusty Russell wrote: I realize that sg chaining is a ploy to make the rest of the kernel devs feel the pain of the SCSI subsystem. But this was a little unsubtle. Signed-off-by: Rusty Russell [EMAIL PROTECTED] Embarrassingly Acked-by: Tejun Heo [EMAIL PROTECTED] Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] sg_ring: a ring of scatterlist arrays
On Monday 07 January 2008 16:01:40 Tejun Heo wrote: But we hit the same problems: 1) sg_chain loses information. The clever chain packaging makes reading easy, but manipulation is severely limited. You can append to your own chains by padding, but not someone elses. This works for SCSI, but what about the rest of us? And don't even think of joining mapped chains: it will almost work. You can append by allocating one more element on the chain to be appended and moving the last element of the first chain to it while using the last element for chaining. Hi Tejun, Nice try! Even ignoring the ugliness of undoing such an operation if the caller doesn't expect you to mangle their chains, consider a one-element sg array. :( Cheers, Rusty. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Remove of old NCR53C9x/esp family of drivers
On do, 2008-01-03 at 20:05 +0100, Geert Uytterhoeven wrote: On Thu, 3 Jan 2008, James Bottomley wrote: On Thu, 2008-01-03 at 17:40 +0200, Boaz Harrosh wrote: As recommended by Christoph Hellwig. There is no use of Fixing these drivers, since there is a much simpler and modern esp infrastructure with David Miller's esp_scsi - Remove all driver files dependent on NCR53C9x.c deleted:drivers/scsi/NCR53C9x.c deleted:drivers/scsi/NCR53C9x.h deleted:drivers/scsi/blz1230.c deleted:drivers/scsi/blz2060.c deleted:drivers/scsi/cyberstorm.c deleted:drivers/scsi/cyberstormII.c deleted:drivers/scsi/dec_esp.c deleted:drivers/scsi/fastlane.c deleted:drivers/scsi/mac_esp.c deleted:drivers/scsi/mca_53c9x.c deleted:drivers/scsi/oktagon_esp.c deleted:drivers/scsi/oktagon_io.S deleted:drivers/scsi/sun3x_esp.c - Remove above list from drivers/scsi/Kconfig drivers/scsi/Makefile OK, I'll split this into four pieces for scsi-pending, since there are three separate interest groups with signoffs to collect (MCA, m68k and alpha) plus the core removal. Anybody who can look into converting the m68k NCR53C9x drivers and has hardware to test (some of) them? I don't think we can afford losing one third of our SCSI drivers... I'll have a look at this. I can only test it on Blizzard 1260 hardware though. Kind regards, Kars. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] scsi: Use new __dma_buffer to align sense buffer in scsi_cmnd
On Sun, 23 Dec 2007 13:09:05 +0200
Boaz Harrosh [EMAIL PROTECTED] wrote:
On Fri, Dec 21 2007 at 4:30 +0200, Benjamin Herrenschmidt [EMAIL PROTECTED]
wrote:
The sense buffer ins scsi_cmnd can nowadays be DMA'ed into directly
by some low level drivers (that typically happens with USB mass
storage).
This is a problem on non cache coherent architectures such as
embedded PowerPCs where the sense buffer can share cache lines with
other structure members, which leads to various forms of corruption.
This uses the newly defined __dma_buffer annotation to enforce that
on such platforms, the sense_buffer is contained within its own
cache line. This has no effect on cache coherent architectures.
Signed-off-by: Benjamin Herrenschmidt [EMAIL PROTECTED]
---
include/scsi/scsi_cmnd.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-merge.orig/include/scsi/scsi_cmnd.h 2007-12-21
13:07:14.0 +1100
+++ linux-merge/include/scsi/scsi_cmnd.h2007-12-21 13:07:29.0
+1100
@@ -88,7 +88,7 @@ struct scsi_cmnd {
working on */
#define SCSI_SENSE_BUFFERSIZE 96
- unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE];
+ unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE] __dma_buffer;
/* obtained by REQUEST SENSE when
* CHECK CONDITION is received on original
* command (auto-sense) */
This has the potential of leaving a big fat ugly hole in the middle of
scsi_cmnd. I would suggest of *just* moving the sense_buffer array to be
the *first member* of struct scsi_cmnd. The command itself is already cache
aligned, allocated by the proper flags to it's slab. And put a fat comment
near it's definition.
This is until a proper fix is sent. I have in my Q a proposition for a
more prominent solution, which I will send next month. Do to short comings
in the sense handling and optimizations, but should definitely cover this
problem.
The code should have time to be discussed and tested, so it is only 2.6.26
material. For the duration of the 2.6.25 kernel we can live with a reorder
of scsi_cmnd members, if it solves such a grave bug for some ARCHs.
Boaz
[RFD below]
My proposed solution will be has follows:
1. Since the scsi protocol mandates an immediate REQUEST_SENSE after an error
in effect the Q is frozen until the REQUEST_SENSE command returns.
2. The scsi-ml needs the sense buffer for its normal operation, independent
from the ULD's request of the sence_buffer or not at request-sense. But
in effect, 90% of all scsi-requests come with ULD's allocated buffer for
sense, that is copied to, on command completion.
3. 99% of all commands complete successfully, so if an optimization is
proposed for the successful case, sacrificing a few cycles for the error
case than thats a good thing.
My suggestion is to have a per Q, driver-overridable, sense buffer that is
DMAed/written to by drivers. At the end of the REQUEST_SENSE command one
of 2 things will be done. Either copy the sense to the ULD's supplied buffer,
or if not available, allocate one from a dedicated mem_cache pool.
So we are completely saving 92 bytes from scsi_cmnd by replacing the buffer
with a pointer. We can always read the sense into a per Q buffer. And 10% of
%1 of the times we will need to allocate a sense buffer from a dedicated
mem_cache
I would say thats a nice optimization.
The changes to scsi_error/scsi_cmnd and friends, is pretty strait forward.
But
it depends on a conversion of 4/5 drivers to the new scsi_eh API for
REQUEST_SENSE. I have only converted these drivers that interfered with the
accessors
effort + 1 other places. But there are a few more places that are not
converted.
Once done. The implementation can easily change with no affect on drivers.
I think that removing the sense_buffer array from scsi_cmnd effects
lots of LLDs. As I wrote in other mail, many LLDs assume that
scsi_cmnd:sense_buffer is always available. Another big task is to
take care about auto sense.
Have you already had some patches? I've just started to work on this
and I'd like to push that fix into 2.6.25.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 0/1] Add blktrace interface to sg device files
There was no response on this patch last time, so i am resubmitting it here. It applies on the current linux-2.6 git tree. -- Christof Schmitt - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/1] blktrace: Add blktrace ioctls to SCSI generic devices
From: Christof Schmitt [EMAIL PROTECTED]
Since the SCSI layer uses the request queues from the block layer, blktrace can
also be used to trace the requests to all SCSI devices (like SCSI tape drives),
not only disks. The only missing part is the ioctl interface to start and stop
tracing.
This patch adds the SETUP, START, STOP and TEARDOWN ioctls from blktrace to the
sg device files. With this change, blktrace can be used for SCSI devices like
for disks, e.g.: blktrace -d /dev/sg1 -o - | blkparse -i -
Signed-off-by: Christof Schmitt [EMAIL PROTECTED]
---
block/blktrace.c | 24 ++--
block/compat_ioctl.c |5 -
drivers/scsi/sg.c| 12
include/linux/blktrace_api.h | 12 ++--
4 files changed, 40 insertions(+), 13 deletions(-)
--- a/block/blktrace.c 2007-12-17 21:51:45.0 +0100
+++ b/block/blktrace.c 2007-12-17 21:51:45.0 +0100
@@ -236,7 +236,7 @@ static void blk_trace_cleanup(struct blk
kfree(bt);
}
-static int blk_trace_remove(struct request_queue *q)
+int blk_trace_remove(struct request_queue *q)
{
struct blk_trace *bt;
@@ -250,6 +250,7 @@ static int blk_trace_remove(struct reque
return 0;
}
+EXPORT_SYMBOL_GPL(blk_trace_remove);
static int blk_dropped_open(struct inode *inode, struct file *filp)
{
@@ -317,18 +318,17 @@ static struct rchan_callbacks blk_relay_
/*
* Setup everything required to start tracing
*/
-int do_blk_trace_setup(struct request_queue *q, struct block_device *bdev,
+int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
struct blk_user_trace_setup *buts)
{
struct blk_trace *old_bt, *bt = NULL;
struct dentry *dir = NULL;
- char b[BDEVNAME_SIZE];
int ret, i;
if (!buts-buf_size || !buts-buf_nr)
return -EINVAL;
- strcpy(buts-name, bdevname(bdev, b));
+ strcpy(buts-name, name);
/*
* some device names have larger paths - convert the slashes
@@ -353,7 +353,7 @@ int do_blk_trace_setup(struct request_qu
goto err;
bt-dir = dir;
- bt-dev = bdev-bd_dev;
+ bt-dev = dev;
atomic_set(bt-dropped, 0);
ret = -EIO;
@@ -400,8 +400,8 @@ err:
return ret;
}
-static int blk_trace_setup(struct request_queue *q, struct block_device *bdev,
- char __user *arg)
+int blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
+ char __user *arg)
{
struct blk_user_trace_setup buts;
int ret;
@@ -410,7 +410,7 @@ static int blk_trace_setup(struct reques
if (ret)
return -EFAULT;
- ret = do_blk_trace_setup(q, bdev, buts);
+ ret = do_blk_trace_setup(q, name, dev, buts);
if (ret)
return ret;
@@ -419,8 +419,9 @@ static int blk_trace_setup(struct reques
return 0;
}
+EXPORT_SYMBOL_GPL(blk_trace_setup);
-static int blk_trace_startstop(struct request_queue *q, int start)
+int blk_trace_startstop(struct request_queue *q, int start)
{
struct blk_trace *bt;
int ret;
@@ -453,6 +454,7 @@ static int blk_trace_startstop(struct re
return ret;
}
+EXPORT_SYMBOL_GPL(blk_trace_startstop);
/**
* blk_trace_ioctl: - handle the ioctls associated with tracing
@@ -465,6 +467,7 @@ int blk_trace_ioctl(struct block_device
{
struct request_queue *q;
int ret, start = 0;
+ char b[BDEVNAME_SIZE];
q = bdev_get_queue(bdev);
if (!q)
@@ -474,7 +477,8 @@ int blk_trace_ioctl(struct block_device
switch (cmd) {
case BLKTRACESETUP:
- ret = blk_trace_setup(q, bdev, arg);
+ strcpy(b, bdevname(bdev, b));
+ ret = blk_trace_setup(q, b, bdev-bd_dev, arg);
break;
case BLKTRACESTART:
start = 1;
--- a/drivers/scsi/sg.c 2007-12-17 21:51:45.0 +0100
+++ b/drivers/scsi/sg.c 2007-12-17 22:36:30.0 +0100
@@ -48,6 +48,7 @@ static int sg_version_num = 30534;/* 2
#include linux/blkdev.h
#include linux/delay.h
#include linux/scatterlist.h
+#include linux/blktrace_api.h
#include scsi.h
#include scsi/scsi_dbg.h
@@ -1063,6 +1064,17 @@ sg_ioctl(struct inode *inode, struct fil
case BLKSECTGET:
return put_user(sdp-device-request_queue-max_sectors * 512,
ip);
+ case BLKTRACESETUP:
+ return blk_trace_setup(sdp-device-request_queue,
+ sdp-disk-disk_name,
+ sdp-device-sdev_gendev.devt,
+ (char *)arg);
+ case BLKTRACESTART:
+ return blk_trace_startstop(sdp-device-request_queue, 1);
+ case BLKTRACESTOP:
+ return blk_trace_startstop(sdp-device-request_queue, 0);
+ case BLKTRACETEARDOWN:
+
