Re: [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation

From: Ivan Warren
Date: Mon Aug 18 2008 - 13:08:37 EST


Stefan Weinhuber wrote:
Hi Ivan,

while I agree that your patch is technically correct, I think I'd
rather like to fix the problematic data length instead of making
the storage server ignore it. There is no need for the
dasd_psf_prssd_data structure to be 16 bytes long as it is only used
for suborders that require 12 bytes. The current layout of that
structure was probably just a mistake. If we ever go to use a
suborder that requires more data, we should either define a further
structure specific to that purpose or find some other way to make
sure that each suborder is called with the appropriate data length.

The following patch works fine on current hardware. Can you please
verify that it indeed fixes your problem as well?

Signed-off-by: Stefan Weinhuber <wein@xxxxxxxxxx>
---
drivers/s390/block/dasd_eckd.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.26/drivers/s390/block/dasd_eckd.h
===================================================================
--- linux-2.6.26.orig/drivers/s390/block/dasd_eckd.h
+++ linux-2.6.26/drivers/s390/block/dasd_eckd.h
@@ -379,7 +379,7 @@ struct dasd_psf_prssd_data {
unsigned char flags;
unsigned char reserved[4];
unsigned char suborder;
- unsigned char varies[9];
+ unsigned char varies[5];
} __attribute__ ((packed));
/*


Stefan,

Sorry for taking such a long time to respond..

I like the patch. You are right, it's probably better to set the right length then hide it ! My initial rationale to SLI instead of altering the length was that I couldn't know if there was any obscure reason for setting the length to 16 instead of 12 (I thought it could have been related to DS8K PAV/HyperPAV support)..

I also tried it on my rig, and it does seem to take care of the problem !

So I guess :

Reviewed-by: Ivan Warren <ivan@xxxxxxxxxxxxx>
Tested-by: Ivan Warren <ivan@xxxxxxxxxxxxx>

Thanks again !

--Ivan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/