Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer
On Thu, Jun 21, 2007 at 09:40:35PM -0700, Mike Anderson wrote: > James Bottomley <[EMAIL PROTECTED]> wrote: > > A proposal to display the correct form of the LUN would be useful if you > > wish to make it? ... The problem is really that SAM specifies a > > possible 4 level structure with 4 possible address methods per level. > > > > The well known LUNs should be simple; there are only three: Report Lun, > > Access Controls and Target Log Pages. The rest we really need input on. > > For instance, I could see the vendors wishing us to combine a > > multi-level flat addressing space into a single logical unit number, > > whereas I could see them wanting us to supply some sort of hierarchy for > > the peripheral and logical unit methods of addressing. > > > > Since you're already using 2 level flat addressing, how do you want to > > see that represented? > > James, why would we would not want to display the lun per SAM4 4.6.2 as > suggested by Stefan in a previous comment. I would also agree to display the LUN in sysfs as 64 bit hex number as advised in SAM4. While the SAM describes different formats and addressing schemes, i don't see how this affects the SCSI layer in the kernel. The kernel gets the 64 bit LUNs and uses them as unique ids. 4.6.1 in SAM4 says in the last sentence, that different values for the 64 bits represent different LUNs, so the assumption of the LUNs being unique is guaranteed. My point of view is mainly from the device driver's (zfcp) and user's point of view. It is only that the disk systems i deal with use the 2 level addressing in the LUNs. The user interface of the disk system displays something like: Volume F000 is exported as FCP LUN 40F04000. When the SCSI midlayer uses and exports this value in sysfs, it should be displayed in the same format (hex). I don't know if the multi level scheme is actually used as hierarchy somewhere. If somebody would be interested in using this information, i think a userspace tool could grab the value from sysfs and display the detailed information according to SAM. -- 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] dmx3191d: Coding style police
Signed-off-by: Alan Cox <[EMAIL PROTECTED]> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc4-mm2/drivers/scsi/dmx3191d.c linux-2.6.22-rc4-mm2/drivers/scsi/dmx3191d.c --- linux.vanilla-2.6.22-rc4-mm2/drivers/scsi/dmx3191d.c2007-06-07 14:24:28.0 +0100 +++ linux-2.6.22-rc4-mm2/drivers/scsi/dmx3191d.c2007-06-14 13:48:18.0 +0100 @@ -113,13 +113,13 @@ scsi_scan_host(shost); return 0; - out_free_irq: +out_free_irq: free_irq(shost->irq, shost); - out_release_region: +out_release_region: release_region(io, DMX3191D_REGION_LEN); - out_disable_device: +out_disable_device: pci_disable_device(pdev); - out: +out: return error; } - 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] dtc: Coding police and printk levels
Seems printk levels hadn't been invented last time this driver was tidied up. Signed-off-by: Alan Cox <[EMAIL PROTECTED]> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc4-mm2/drivers/scsi/dtc.c linux-2.6.22-rc4-mm2/drivers/scsi/dtc.c --- linux.vanilla-2.6.22-rc4-mm2/drivers/scsi/dtc.c 2007-06-07 14:24:28.0 +0100 +++ linux-2.6.22-rc4-mm2/drivers/scsi/dtc.c 2007-06-14 13:43:37.0 +0100 @@ -137,11 +137,9 @@ #ifdef OVERRIDE [] __initdata = OVERRIDE; #else -[4] __initdata = { { -0, IRQ_AUTO}, { -0, IRQ_AUTO}, { -0, IRQ_AUTO}, { -0, IRQ_AUTO}}; +[4] __initdata = { + { 0, IRQ_AUTO }, { 0, IRQ_AUTO }, { 0, IRQ_AUTO }, { 0, IRQ_AUTO } +}; #endif #define NO_OVERRIDES ARRAY_SIZE(overrides) @@ -176,7 +174,7 @@ * Inputs : str - unused, ints - array of integer parameters with ints[0] * equal to the number of ints. * -*/ + */ static void __init dtc_setup(char *str, int *ints) { @@ -233,7 +231,7 @@ } else for (; !addr && (current_base < NO_BASES); ++current_base) { #if (DTCDEBUG & DTCDEBUG_INIT) - printk("scsi-dtc : probing address %08x\n", bases[current_base].address); + printk(KERN_DEBUG "scsi-dtc : probing address %08x\n", bases[current_base].address); #endif if (bases[current_base].noauto) continue; @@ -244,7 +242,7 @@ if (check_signature(base + signatures[sig].offset, signatures[sig].string, strlen(signatures[sig].string))) { addr = bases[current_base].address; #if (DTCDEBUG & DTCDEBUG_INIT) - printk("scsi-dtc : detected board.\n"); + printk(KERB_DEBUG "scsi-dtc : detected board.\n"); #endif goto found; } @@ -253,7 +251,7 @@ } #if defined(DTCDEBUG) && (DTCDEBUG & DTCDEBUG_INIT) - printk("scsi-dtc : base = %08x\n", addr); + printk(KERN_DEBUG "scsi-dtc : base = %08x\n", addr); #endif if (!addr) - 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] ppa: Coding police and printk levels
Add printk levels Clean up some oddities of formatting Fix goto labels Signed-off-by: Alan Cox <[EMAIL PROTECTED]> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc4-mm2/drivers/scsi/ppa.c linux-2.6.22-rc4-mm2/drivers/scsi/ppa.c --- linux.vanilla-2.6.22-rc4-mm2/drivers/scsi/ppa.c 2007-06-07 14:24:28.0 +0100 +++ linux-2.6.22-rc4-mm2/drivers/scsi/ppa.c 2007-06-14 13:36:48.0 +0100 @@ -129,11 +129,11 @@ if ((length > 10) && (strncmp(buffer, "recon_tmo=", 10) == 0)) { x = simple_strtoul(buffer + 10, NULL, 0); dev->recon_tmo = x; - printk("ppa: recon_tmo set to %ld\n", x); + printk(KERN_INFO "ppa: recon_tmo set to %ld\n", x); return length; } - printk("ppa /proc: invalid variable\n"); - return (-EINVAL); + printk(KERN_WARNING "ppa /proc: invalid variable\n"); + return -EINVAL; } static int ppa_proc_info(struct Scsi_Host *host, char *buffer, char **start, off_t offset, int length, int inout) @@ -216,7 +216,7 @@ /* Counter expired - Time out occurred */ ppa_fail(dev, DID_TIME_OUT); - printk("ppa timeout in ppa_wait\n"); + printk(KERN_WARNING "ppa timeout in ppa_wait\n"); return 0; /* command timed out */ } @@ -248,7 +248,7 @@ return; udelay(5); } - printk("ppa: ECP sync failed as data still present in FIFO.\n"); + printk(KERN_WARNING "ppa: ECP sync failed as data still present in FIFO.\n"); } } @@ -328,7 +328,7 @@ break; default: - printk("PPA: bug in ppa_out()\n"); + printk(KERN_ERR "PPA: bug in ppa_out()\n"); r = 0; } return r; @@ -381,7 +381,7 @@ break; default: - printk("PPA: bug in ppa_ins()\n"); + printk(KERN_ERR "PPA: bug in ppa_ins()\n"); r = 0; break; } @@ -633,7 +633,7 @@ struct scsi_cmnd *cmd = dev->cur_cmd; if (!cmd) { - printk("PPA: bug in ppa_interrupt\n"); + printk(KERN_ERR "PPA: bug in ppa_interrupt\n"); return; } if (ppa_engine(dev, cmd)) { @@ -646,31 +646,31 @@ case DID_OK: break; case DID_NO_CONNECT: - printk("ppa: no device at SCSI ID %i\n", cmd->device->target); + printk(KERN_DEBUG "ppa: no device at SCSI ID %i\n", cmd->device->target); break; case DID_BUS_BUSY: - printk("ppa: BUS BUSY - EPP timeout detected\n"); + printk(KERN_DEBUG "ppa: BUS BUSY - EPP timeout detected\n"); break; case DID_TIME_OUT: - printk("ppa: unknown timeout\n"); + printk(KERN_DEBUG "ppa: unknown timeout\n"); break; case DID_ABORT: - printk("ppa: told to abort\n"); + printk(KERN_DEBUG "ppa: told to abort\n"); break; case DID_PARITY: - printk("ppa: parity error (???)\n"); + printk(KERN_DEBUG "ppa: parity error (???)\n"); break; case DID_ERROR: - printk("ppa: internal driver error\n"); + printk(KERN_DEBUG "ppa: internal driver error\n"); break; case DID_RESET: - printk("ppa: told to reset device\n"); + printk(KERN_DEBUG "ppa: told to reset device\n"); break; case DID_BAD_INTR: - printk("ppa: bad interrupt (???)\n"); + printk(KERN_WARNING "ppa: bad interrupt (???)\n"); break; default: - printk("ppa: bad return code (%02x)\n", + printk(KERN_WARNING "ppa: bad return code (%02x)\n", (cmd->result >> 16) & 0xff); } #endif @@ -724,8 +724,7 @@ if (retv) { if (time_after(jiffies, dev->jstart + (1 * HZ))) { - printk - ("ppa: Parallel port cable is unplugged!!\n"); + printk(KERN_ERR "ppa: Parallel port cable is unplugged.\n"); ppa_fail(dev, DID_BUS_BUSY); return 0; } else { @@ -755,11 +754,9 @@ case 4: /* Phase 4 - Setup scatter/gather buffers */ if (cmd->use_sg) { /* if many buffers are available, start filling the first */ - cmd->SCp.buffer = - (struct scatterlist *) cmd->request_buffer; + cmd->SCp.buffer = (struct scatte
Re: [PATCH] dmx3191d: Coding style police
On Fri, Jun 22, 2007 at 02:24:56PM +0100, Alan Cox wrote: > - out_free_irq: > +out_free_irq: Which CodingStyle mandates that goto labels start in column 0? It screws up diff -p to do this. - 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] dmx3191d: Coding style police
On Fri, 22 Jun 2007 07:26:23 -0600 Matthew Wilcox <[EMAIL PROTECTED]> wrote: > On Fri, Jun 22, 2007 at 02:24:56PM +0100, Alan Cox wrote: > > - out_free_irq: > > +out_free_irq: > > Which CodingStyle mandates that goto labels start in column 0? It > screws up diff -p to do this. The style being the fact almost all of the kernel keeps the label in column zero ? I'm not greatly fussed by the indent for goto labels, its the other stuff like missing printk labels that actually matters. However if your diff -p is buggy how about fixing your diff instead ? Alan - 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] zfcp: Report FCP LUN to SCSI midlayer
On Thu, 2007-06-21 at 21:40 -0700, Mike Anderson wrote: > James Bottomley <[EMAIL PROTECTED]> wrote: > > A proposal to display the correct form of the LUN would be useful if you > > wish to make it? ... The problem is really that SAM specifies a > > possible 4 level structure with 4 possible address methods per level. > > > > The well known LUNs should be simple; there are only three: Report Lun, > > Access Controls and Target Log Pages. The rest we really need input on. > > For instance, I could see the vendors wishing us to combine a > > multi-level flat addressing space into a single logical unit number, > > whereas I could see them wanting us to supply some sort of hierarchy for > > the peripheral and logical unit methods of addressing. > > > > Since you're already using 2 level flat addressing, how do you want to > > see that represented? > > James, why would we would not want to display the lun per SAM4 4.6.2 as > suggested by Stefan in a previous comment. Because in a two LUN system, what was LUN 1 would then become LUN 0x1 which looks a bit unpalatable. 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 3/3] Enable Aggressive Link Power management for AHCI controllers.
On Thu, 21 Jun 2007 15:08:32 +0200 Jens Axboe <[EMAIL PROTECTED]> wrote: > On Wed, Jun 20 2007, Kristen Carlson Accardi wrote: > > Enable Aggressive Link Power management for AHCI controllers. > > > > This patch will set the correct bits to turn on Aggressive > > Link Power Management (ALPM) for the ahci driver. This > > will cause the controller and disk to negotiate a lower > > power state for the link when there is no activity (see > > the AHCI 1.x spec for details). This feature is mutually > > exclusive with Hot Plug, so when ALPM is enabled, Hot Plug > > is disabled. ALPM will be enabled by default, but it is > > settable via the scsi host syfs interface. Possible > > settings for this feature are: > > > > Setting Effect > > -- > > min_power ALPM is enabled, and link set to enter > > lowest power state (SLUMBER) when idle > > Hot plug not allowed. > > > > max_performance ALPM is disabled, Hot Plug is allowed > > > > medium_powerALPM is enabled, and link set to enter > > second lowest power state (PARTIAL) when > > idle. Hot plug not allowed. > > > > Signed-off-by: Kristen Carlson Accardi <[EMAIL PROTECTED]> > > A suggestion (it comes with a patch!) - default to max_power/almp off, > not min_power. For two reasons: > > - There's such a big performance difference between the two, you really > want max_power when booting. > > - It's a lot better to default to no change, than default to enabling > something new. Sounds reasonable to me. Distros/users can decide if they want to have scripts that enable this after boot to run at min_power. Acked-by: Kristen Carlson Accardi <[EMAIL PROTECTED]> > > diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c > index 841cf0a..e7a2072 100644 > --- a/drivers/ata/ahci.c > +++ b/drivers/ata/ahci.c > @@ -786,8 +786,7 @@ static int ahci_disable_alpm(struct ata_port *ap) > return 0; > } > > -static int ahci_enable_alpm(struct ata_port *ap, > - enum scsi_host_link_pm policy) > +static int ahci_enable_alpm(struct ata_port *ap, enum scsi_host_link_pm > policy) > { > struct ahci_host_priv *hpriv = ap->host->private_data; > void __iomem *port_mmio = ahci_port_base(ap); > @@ -808,19 +807,19 @@ static int ahci_enable_alpm(struct ata_port *ap, > return -EINVAL; > } > > - switch(policy) { > + switch (policy) { > case SHOST_MAX_PERFORMANCE: > - ahci_disable_alpm(ap); > - ap->pm_policy = policy; > - return 0; > case SHOST_NOT_AVAILABLE: > - case SHOST_MIN_POWER: > /* >* if we came here with SHOST_NOT_AVAILABLE, >* it just means this is the first time we > - * have tried to enable - so try to do > - * min_power > + * have tried to enable - default to max performance, > + * and let the user go to lower power modes on request. >*/ > + ahci_disable_alpm(ap); > + ap->pm_policy = SHOST_MAX_PERFORMANCE; > + return 0; > + case SHOST_MIN_POWER: > ap->pm_policy = SHOST_MIN_POWER; > > /* configure HBA to enter SLUMBER */ > > -- > Jens Axboe > - 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] zfcp: Report FCP LUN to SCSI midlayer
On Fri, 22 Jun 2007 09:11:39 CDT, James Bottomley wrote: > On Thu, 2007-06-21 at 21:40 -0700, Mike Anderson wrote: > > James Bottomley <[EMAIL PROTECTED]> wrote: > > > A proposal to display the correct form of the LUN would be useful if you > > > wish to make it? ... The problem is really that SAM specifies a > > > possible 4 level structure with 4 possible address methods per level. > > > > > > The well known LUNs should be simple; there are only three: Report Lun, > > > Access Controls and Target Log Pages. The rest we really need input on. > > > For instance, I could see the vendors wishing us to combine a > > > multi-level flat addressing space into a single logical unit number, > > > whereas I could see them wanting us to supply some sort of hierarchy for > > > the peripheral and logical unit methods of addressing. > > > > > > Since you're already using 2 level flat addressing, how do you want to > > > see that represented? > > > > James, why would we would not want to display the lun per SAM4 4.6.2 as > > suggested by Stefan in a previous comment. > > Because in a two LUN system, what was LUN 1 would then become LUN > 0x1 which looks a bit unpalatable. > Just my $.02. If you use a pseries, and have to specifiy the LUN to the OFW on the later releases, you must train the fingers to type the extra 12 chars. Iy you are watching an iSCSI target with tcpdump, you can observe the PDU fields with the lun encoded thusly. It may not be historical, but is certainly more accurate to use the 0x1 as 64 bit (or more correctly 8 bytes) encoded as hex. ++doug - 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] dtc: Coding police and printk levels
On Fri, Jun 22, 2007 at 02:26:29PM +0100, Alan Cox wrote: > @@ -244,7 +242,7 @@ > if (check_signature(base + > signatures[sig].offset, signatures[sig].string, > strlen(signatures[sig].string))) { > addr = > bases[current_base].address; > #if (DTCDEBUG & DTCDEBUG_INIT) > - printk("scsi-dtc : detected > board.\n"); > + printk(KERB_DEBUG "scsi-dtc : > detected board.\n"); I think you meant KERN_DEBUG ? --D - 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 3/3] Enable Aggressive Link Power management for AHCI controllers.
On Fri, Jun 22 2007, Kristen Carlson Accardi wrote: > On Thu, 21 Jun 2007 15:08:32 +0200 > Jens Axboe <[EMAIL PROTECTED]> wrote: > > > On Wed, Jun 20 2007, Kristen Carlson Accardi wrote: > > > Enable Aggressive Link Power management for AHCI controllers. > > > > > > This patch will set the correct bits to turn on Aggressive > > > Link Power Management (ALPM) for the ahci driver. This > > > will cause the controller and disk to negotiate a lower > > > power state for the link when there is no activity (see > > > the AHCI 1.x spec for details). This feature is mutually > > > exclusive with Hot Plug, so when ALPM is enabled, Hot Plug > > > is disabled. ALPM will be enabled by default, but it is > > > settable via the scsi host syfs interface. Possible > > > settings for this feature are: > > > > > > Setting Effect > > > -- > > > min_power ALPM is enabled, and link set to enter > > > lowest power state (SLUMBER) when idle > > > Hot plug not allowed. > > > > > > max_performance ALPM is disabled, Hot Plug is allowed > > > > > > medium_power ALPM is enabled, and link set to enter > > > second lowest power state (PARTIAL) when > > > idle. Hot plug not allowed. > > > > > > Signed-off-by: Kristen Carlson Accardi <[EMAIL PROTECTED]> > > > > A suggestion (it comes with a patch!) - default to max_power/almp off, > > not min_power. For two reasons: > > > > - There's such a big performance difference between the two, you really > > want max_power when booting. > > > > - It's a lot better to default to no change, than default to enabling > > something new. > > Sounds reasonable to me. Distros/users can decide if they want to have > scripts that enable this after boot to run at min_power. Exactly, it needs to be handled by some power management daemon anyway and be integrated with power savings in general. You could use io load to determine when to enable/disable alpm, for instance. > Acked-by: Kristen Carlson Accardi <[EMAIL PROTECTED]> Will you integrate it into the next posting? -- Jens Axboe - 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] zfcp: Report FCP LUN to SCSI midlayer
James Bottomley wrote: > H:C is really mid-layer defined (although I'd like to get rid of C > eventually). They really correspond to physical enumeration of the HBA > devices. T (or I) is the one we think could be abstracted and placed > within the gift of the transport, but so far there's been a lot of > debate with few actual concrete proposals. L is basically defined by > SAM for every transport, but I'm really unsure how it should be > represented in all its SAM specified glory. If we reduce this discussion to the sysfs representation, then there is one important point to understand: The sysfs device path is implementation-defined. It always has been, always will be. That's inherent to sysfs. So, having the current implementation-defined H:C:T:L in the sysfs device path is alright. Trying to squeeze the SAM LU identifiers and target port identifiers into the sysfs path might not be such a good idea, exactly because there is a number of different formats of these identifiers. We certainly want to find devices in sysfs in a transport-independent way. So these identifiers should go somewhere else. How about exporting sysfs attributes for each sdev with common names like logical_unit_identifier target_port_identifier initiator_port_identifier but with transport-dependent contents? The various transport-layer implementations can do this without hand-holding of the SCSI core; the only thing that's important is that all transports use the same names for these attributes. The attributes shall be human-readable and can be used by system management software like udev, hal, lsscsi, whatever. If SCSI mid-layer wants to control what attribute names are used but wants to leave the formatting of the contents to the transports, add for example something like: struct scsi_transport_template { ... int (* print_logical_unit_identifier)(char *buf, struct scsi_device *sdev); int (* print_target_port_identifier)(char *buf, struct scsi_device *sdev); int (* print_initiator_port_identifier)(char *buf, struct scsi_device *sdev); ... }; Then SCSI mid-layer creates and destroys the attributes and transport-layer implementations deliver contents for them. The print_ hooks can also be used for debug printk's or the like if need be. What's then left of the issues with H:C:T:L are the varying difficulties that some transports have to fill in some more or less sensible values in the mid-layer's variables corresponding to H:C:T:L. Well, actually the values don't have to make a lot of sense because they are implementation defined and not for users to draw conclusions from them. Or is there more to it? -- Stefan Richter -=-=-=== -==- =-==- http://arcgraph.de/sr/ - 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] dtc: Coding police and printk levels
On Fri, 22 Jun 2007 11:00:06 -0700 "Darrick J. Wong" <[EMAIL PROTECTED]> wrote: > On Fri, Jun 22, 2007 at 02:26:29PM +0100, Alan Cox wrote: > > @@ -244,7 +242,7 @@ > > if (check_signature(base + > > signatures[sig].offset, signatures[sig].string, > > strlen(signatures[sig].string))) { > > addr = > > bases[current_base].address; > > #if (DTCDEBUG & DTCDEBUG_INIT) > > - printk("scsi-dtc : detected > > board.\n"); > > + printk(KERB_DEBUG "scsi-dtc : > > detected board.\n"); > > I think you meant KERN_DEBUG ? Thanks - thats a path thats not compiled and I missed that. Will fix it. - 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