Re: mpt2sas driver barfs when force removing a drive on 3.13.1

2014-02-13 Thread Matthew Thode
On 02/12/2014 01:41 PM, Richard Yao wrote:
> On 02/12/2014 02:25 PM, Dan Williams wrote:
>> On Wed, Feb 12, 2014 at 9:41 AM, Richard Yao  wrote:
>>> You should use `git send-email` to send the patch to all of the people
>>> listed by get_maintainer.pl:
>>
>> I see nothing wrong with the patch Joe originally submitted:
>> http://marc.info/?l=linux-scsi&m=138609455422179&w=2
>>
>>>
>>> richard@desktop ~/devel/linux $ scripts/get_maintainer.pl --file
>>> drivers/scsi/scsi_transport_sas.c
>>> "James E.J. Bottomley"  (maintainer:SCSI
>>> SUBSYSTEM)
>>> linux-scsi@vger.kernel.org (open list:SCSI SUBSYSTEM)
>>> linux-ker...@vger.kernel.org (open list)
>>>
>>> If I see that James E.J. Bottomley is already on CC for this discussion,
>>> but my recent (and first) experience submitting a patch for another
>>> subsystem was that they are ignored unless this strict procedure is
>>> followed.
>>>
>>> If the maintainer does not respond within a month after doing this, I
>>> suggest informing Linus Torvalds via email that the subsystem maintainer
>>> is non-responsive.
>>>
>>
>> Just re-ping James, he'll get to it...
>>
> 
> That was intended as generic advice. Sadly, I don't have a copy of the
> email headers for the original patch submission, so I can't inspect it.
> 
> That being said, I am receiving these emails because I suggested Matthew
> to include the Gentoo kernel team alias on the CC list when I advised
> him to send this report upstream.
> 
Confirmed that it now works (I luksClosed the device, removed it from
the system, waited a few seconds, readded it and checked dmesg).

-- 
-- Matthew Thode (prometheanfire)


0x40AC5AC3.asc
Description: application/pgp-keys


signature.asc
Description: OpenPGP digital signature


Haben Sie ein Geld Problem? ** Sofort langfristig zinsgünstig und ohne bankmässige Schikanen frisches Geld bekommen

2014-02-13 Thread Silke Weiss
Hallo,

hat Ihre Bank mal wieder einen Grund gefunden, Ihnen keinen Kredit zu geben? 

Gehen Sie doch dorthin, wo man Sie als Kunden noch anständig behandelt.

Auch bei laufenden Verpflichtungen und zuvor von Ihrer eigenen Hausbank 
zurückgewiesenem Antrag können wir Ihnen frisches Geld vermitteln zu niedrigen 
Zinsen -nur ab 1,8% Zinsen p.a.- und extrem langen Laufzeiten. Ihren Kredit 
bekommen Sie jetzt aus Belize ohne den üblichen Antragswust, ohne Bankauskunft 
und schnell. 
Wir arbeiten mit mehr als 30 Banken zusammen, und wir kümmern uns um alle 
Details.
Also ideal für Selbständige, Angestellte und Pensionäre. Lassen Sie sich nicht 
länger bevormunden von Ihrer Bank. 
Wenn Sie nicht nur auf Ihre Bank angewiesen sein wollen, ist das jetzt die 
Chance.

Weitere Infos auf:   NIEDRIGZINSKREDIT und ergänzen Sie ohne die Füllzeichen:
c+0+m

Holen Sie sich frisches Geld dort, wo die Geldspeicher voll sind, die Zinsen 
extrem günstig und die Laufzeiten lang sind. 

Wir wünschen noch einen schönen Tag

Alfred Wagner


--
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 v10 0/4] ata: Add APM X-Gene SoC SATA host controller support

2014-02-13 Thread Loc Ho
Hi Tejun,

With regard to make use of the standard ahci_platform and recent Hans'
PHY works, the original version of the driver was an ahci_platform
driver. Unfortunately, the APM X-Gene SATA controller is more
complicate than an standard AHCI controller. In order to make it works
properly, the follow additional programming sequences are required:

1. There are a number of errata that require workaround. Some can be
fixed by adding broken flags while others are better to just wrap
around the existent libahci library routines and not overly polluting
the libahci routines.
2. There are additional controller programming sequences to configure.
2a. By default, RAM are powered down and require brought out of shutdown.
2b. The controller has an additional corresponding PHY part that needs
to be programmed after PHY configuration.
2c. The controller requires extra programming sequence for the
hardreset due to errata.
2d. For the IO flush, it requires additional memory resources.
3. With the current AHCI platform data handing and ARM64 arch support,
if I were to add custom AHCI platform data routine, I would need
equivalent functions and that would make the ARCH support having
knowledge of the SATA code which I don't want. Also, the DTS
processing/scanning in ARCH64 is NULL for platform data and would have
make X-Gene to have its own version which I also don't want.

-Loc

> On Thu, Jan 16, 2014 at 8:11 AM, Loc Ho  wrote:
>> This patch adds support for the APM X-Gene SoC SATA host controller. In
>> order for the host controller to work, the corresponding PHY driver
>> musts also be available.
>>
>> v10:
>>  * Update binding documentation
>>
>> v9:
>>  * Remove ACPI/EFI include files
>>  * Remove the IO flush support, interrupt routine, and DTS resources
>>  * Remove function xgene_rd, xgene_wr, and xgene_wr_flush
>>  * Remove PMP support (function xgene_ahci_qc_issue, xgene_ahci_qc_prep,
>>xgene_ahci_qc_fill_rtf, xgene_ahci_softreset, and xgene_ahci_do_softreset)
>>  * Rename function xgene_ahci_enable_phy to xgene_ahci_force_phy_rdy
>>  * Clean up hardreset functions
>>  * Require v7 of the PHY driver
>>
>> v8:
>>  * Remove _ADDR from defines
>>  * Remove define MSTAWAUX_COHERENT_BYPASS_SET and
>>STARAUX_COHERENT_BYPASS_SET and use direct coding
>>  * Remove the un-necessary check for DTS boot with built in ACPI table
>>  * Switch to use dma_set_mask_and_coherent for setting DMA mask
>>  * Remove ACPI table matching code
>>  * Update clock-names for sata01clk, sata23clk, and sata45clk
>>
>> v7:
>>  * Update the clock code by toggle the clock
>>  * Update the DTS clock mask values due to the clock spilt between host and
>>v5 of the PHY drivers
>>
>> v6:
>>  * Update binding documentation
>>  * Change select PHY_XGENE_SATA to PHY_XGENE
>>  * Add ULL to constants
>>  * Change indentation and comments
>>  * Clean up the probe functions a bit more
>>  * Remove xgene_ahci_remove function
>>  * Add the flush register to DTS
>>  * Remove the interrupt-parent from DTS
>>
>> v5:
>>  * Sync up to v3 of the PHY driver
>>  * Remove MSLIM wrapper functions
>>  * Change the memory shutdown loop to use usleep_range
>>  * Use devm_ioremap_resource instead devm_ioremap
>>  * Remove suspend/resume functions as not needed
>>
>> v4:
>>  * Remove the ID property in DT
>>  * Remove the temporary PHY direct function call and use PHY function
>>  * Change printk to pr_debug
>>  * Move the IOB flush addresses into the DT
>>  * Remove the parameters retrieval function as no longer needed
>>  * Remove the header file as no longer needed
>>  * Require v2 patch of the SATA PHY driver. Require slightly modification
>>in the Kconfig as it is moved to folder driver/phy and use Kconfig
>>PHY_XGENE_SATA instead SATA_XGENE_PHY.
>>
>> v3:
>>  * Move out the SATA PHY to another driver
>>  * Remove the clock-cells entry from DTS
>>  * Remove debug wrapper
>>  * Remove delay functions wrapper
>>  * Clean up resource and IRQ query
>>  * Remove query clock name
>>  * Switch to use dma_set_mask/dma_coherent_mask
>>  * Remove un-necessary devm_kfree
>>  * Update GPL license header to v2
>>  * Spilt up function xgene_ahci_hardreset
>>  * Spilt up function xgene_ahci_probe
>>  * Remove all reference of CONFIG_ARCH_MSLIM
>>  * Clean up chip revision code
>>
>> v2:
>>  * Clean up file sata_xgene.c with Lindent and etc
>>  * Clean up file sata_xgene_serdes.c with Lindent and etc
>>  * Add description to each patch
>>
>> v1:
>>  * inital version
>>
>> Signed-off-by: Loc Ho 
>> Signed-off-by: Tuan Phan 
>> Signed-off-by: Suman Tripathi 
>> ---
>> Loc Ho (4):
>>   ata: Export required functions by APM X-Gene SATA driver
>>   Documentation: Add documentation for APM X-Gene SoC SATA host
>> controller DTS binding
>>   ata: Add APM X-Gene SoC SATA host controller driver
>>   arm64: Add APM X-Gene SoC SATA host controller DTS entries
>>
>>  .../devicetree/bindings/ata/apm-xgene.txt  |   70 +++
>>  arch/arm64/boot/dts/apm-storm.dtsi 

Re: [Lsf-pc] [LSF/MM TOPIC] SMR: Disrupting recording technology meriting a new class of storage device

2014-02-13 Thread Theodore Ts'o
On Tue, Feb 11, 2014 at 09:57:40AM -0200, Carlos Maiolino wrote:
> 
> Thanks for clarification, and, this just enforce my concept that ZBC protocol
> should be integrated in the generic block layer not make it device-mapper
> dependent. So, make this available to any device that supports it with or
> without the help of DM.

The kernel interface which I have proposed[1] on the linux-fsdevel
list is indeed something that would be integrated in the generic block
device layer.  My hope is that in the near future (I'm waiting for ZBC
prototypes to show up in Mountain View ;-) we will have a patches that
will allow Linux to recognize ZBC drives, and export the ZBC
functionality via the this interface.

[1] http://thread.gmane.org/gmane.linux.file-systems/81970/focus=82309

I also hope that in the near-term future we will have at least one
device-mapper "SMR simulator" which take a standard block device, and
add write-pointer tracking, and then export the same interface.  This
would allow file systems (perhaps btrfs) to experiment with working
with SMR drives, without having to wait for getting a hold of one of
the ZBC drives.  It would also allow people who are interested in
writing a device mapper shim layer which takes a SMR drive, and
exports a host-assisted block device.

Of course, the stacking of the device mapper layers:

HDD <-> SMR_SIMULATOR <-> SMR_ADAPTER 

is basically a no-op except for introducing performance delays.  But
the idea here is not performance, but allow people to debug their
code.  So the use cases:

HDD <-> SMR_SIMULATOR <-> SMR_ADAPTER <-> stock linux file system
HDD <-> SMR_SIMULATOR <-> SMR_ADAPTER <-> ext4 modified to be 
SMR-friendly
HDD <-> SMR_SIMULATOR <-> modified btrfs that supports ZBC

would eventually become:

SMR HDD <-> SMR_ADAPTER <-> stock linux file system
SMR_HDD <-> SMR_ADAPTER <-> ext4 modified to be SMR-friendly
SMR_HDD <-> modified btrfs that supports ZBC

And while we wait for SMR_HDD's to become generally available to all
kernel developers, the existence of the device-mapper smr simulator
will enable us to start work on the device-mapper smr adapter, and
file systems that are either modified to be SMR-friendly, or modified
to work directly w/o any adapter layers with SMR drives.

Regards,

- Ted
--
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] Fix uses of dma_max_pfn() when converting to a limiting address

2014-02-13 Thread Russell King - ARM Linux
On Thu, Feb 13, 2014 at 10:07:01AM -0800, James Bottomley wrote:
> On Thu, 2014-02-13 at 17:11 +, Russell King - ARM Linux wrote:
> > On Thu, Feb 13, 2014 at 08:58:10AM -0800, James Bottomley wrote:
> > > This doesn't really look like the right fix.  You replaced dev->dma_mask
> > > with a calculation on dev_max_pfn().  Since dev->dma_mask is always u64
> > > and dev_max_pfn is supposed to be returning the pfn of the dma_mask, it
> > > should unconditionally be 64 bits as well.  Either that or it should
> > > return dma_addr_t.
> > 
> > My reasoning is that PFNs in the system are always of type "unsigned long"
> > and therefore a function returning a pfn should have that type.  If we
> > overflow a PFN fitting in an unsigned long, we have lots of places which
> > need fixing.
> 
> It's not intuitive to people who need the dma mask that they're supposed
> to use dma_max_pfn() << PAGE_SHIFT but now they have to worry about the
> casting and, if they don't get it right, nothing will warn or tell them.
> what about a new macro, say dma_max_mask(dev) that just returns
> (u64)dma_max_pfn() << PAGE_SHIFT?

This sounds like a good idea.

I've just been looking at places which do this << PAGE_SHIFT, and we
have other places which suffer from this same bug all over the kernel,
so maybe we actually need a pfn_to_addr() macro or similar so that
people get this right in these other places too?  It appears to be
quite a widespread problem.

I'm surprised none of the below haven't already caused a problem.

Thoughts?

int __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
 unsigned long nr_pages)
{
resource_size_t start, size;

start = phys_start_pfn << PAGE_SHIFT;

void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
unsigned long start_pfn, enum memmap_context context)
{
unsigned long end_pfn = start_pfn + size;
unsigned long pfn;

/* The shift won't overflow because ZONE_NORMAL is below 4G. */
if (!is_highmem_idx(zone))
set_page_address(page, __va(pfn << PAGE_SHIFT));

void __init free_area_init_nodes(unsigned long *max_zone_pfn)
{
unsigned long start_pfn, end_pfn;

for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid)
printk("  node %3d: [mem %#010lx-%#010lx]\n", nid,
   start_pfn << PAGE_SHIFT, (end_pfn << PAGE_SHIFT) - 1);
(thankfully, this one is just a printk).

int vb2_get_contig_userptr(unsigned long vaddr, unsigned long size,
   struct vm_area_struct **res_vma, dma_addr_t *res_pa)
{
unsigned long this_pfn, prev_pfn;
dma_addr_t pa = 0;

if (prev_pfn == 0)
pa = this_pfn << PAGE_SHIFT;

static pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 unsigned long size, pgprot_t vma_prot)
{
#ifdef pgprot_noncached
phys_addr_t offset = pfn << PAGE_SHIFT;

if (uncached_access(file, offset))
return pgprot_noncached(vma_prot);
#endif
return vma_prot;

static int i810_insert_dcache_entries(struct agp_memory *mem, off_t pg_start,
  int type)
{
int i;

for (i = pg_start; i < (pg_start + mem->page_count); i++) {
dma_addr_t addr = i << PAGE_SHIFT;


-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
--
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: [SCSI] bnx2fc: Broadcom FCoE offload driver

2014-02-13 Thread Eddie Wai
Hello Dan,

Thanks for bringing this to our attention.  We'll fix it upon our next
patchset submission.

Thanks,
Eddie

On Thu, 2014-02-13 at 12:52 +0300, Dan Carpenter wrote:
> Hello Bhanu Gollapudi,
> 
> The patch 853e2bd2103a: "[SCSI] bnx2fc: Broadcom FCoE offload driver"
> from Feb 4, 2011, leads to the following static checker warning:
> 
>   drivers/scsi/bnx2fc/bnx2fc_hwi.c:1660 bnx2fc_init_mp_task()
>   warn: we tested 'task_type == 3' before and it was 'false'
> 
> drivers/scsi/bnx2fc/bnx2fc_hwi.c
>   1586  void bnx2fc_init_mp_task(struct bnx2fc_cmd *io_req,
>   1587  struct fcoe_task_ctx_entry *task)
>   1588  {
>   1589  struct bnx2fc_mp_req *mp_req = &(io_req->mp_req);
>   1590  struct bnx2fc_rport *tgt = io_req->tgt;
>   1591  struct fc_frame_header *fc_hdr;
>   1592  struct fcoe_ext_mul_sges_ctx *sgl;
>   1593  u8 task_type = 0;
>^
> Should this be FCOE_TASK_TYPE_UNSOLICITED (3)?
UNSOLICITED task type is not being used by bnx2fc at the moment.  The
more preferable fix would be to un-initialize this and then change the
cmd_type to a switch statement and supply a default exit.  We don't want
to see the firmware getting initialized by uninitialized requests.
> 
>   1594  u64 *hdr;
>   1595  u64 temp_hdr[3];
>   1596  u32 context_id;
>   1597  
>   1598  
>   1599  /* Obtain task_type */
>   1600  if ((io_req->cmd_type == BNX2FC_TASK_MGMT_CMD) ||
>   1601  (io_req->cmd_type == BNX2FC_ELS)) {
>   1602  task_type = FCOE_TASK_TYPE_MIDPATH;
>   1603  } else if (io_req->cmd_type == BNX2FC_ABTS) {
>   1604  task_type = FCOE_TASK_TYPE_ABTS;
>   1605  }
>   1606  
>   1607  memset(task, 0, sizeof(struct fcoe_task_ctx_entry));
>   1608  
>   1609  /* Setup the task from io_req for easy reference */
>   1610  io_req->task = task;
>   1611  
>   1612  BNX2FC_IO_DBG(io_req, "Init MP task for cmd_type = %d 
> task_type = %d\n",
>   1613  io_req->cmd_type, task_type);
>   1614  
>   1615  /* Tx only */
>   1616  if ((task_type == FCOE_TASK_TYPE_MIDPATH) ||
>   1617  (task_type == FCOE_TASK_TYPE_UNSOLICITED)) {
>   ^^
> Not possible.
> 
>   1618  task->txwr_only.sgl_ctx.sgl.mul_sgl.cur_sge_addr.lo =
>   1619  (u32)mp_req->mp_req_bd_dma;
>   1620  task->txwr_only.sgl_ctx.sgl.mul_sgl.cur_sge_addr.hi =
>   1621  (u32)((u64)mp_req->mp_req_bd_dma >> 
> 32);
>   1622  task->txwr_only.sgl_ctx.sgl.mul_sgl.sgl_size = 1;
>   1623  }
> 
> regards,
> dan carpenter
> --
> 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


--
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] Fix uses of dma_max_pfn() when converting to a limiting address

2014-02-13 Thread James Bottomley
On Thu, 2014-02-13 at 17:11 +, Russell King - ARM Linux wrote:
> On Thu, Feb 13, 2014 at 08:58:10AM -0800, James Bottomley wrote:
> > This doesn't really look like the right fix.  You replaced dev->dma_mask
> > with a calculation on dev_max_pfn().  Since dev->dma_mask is always u64
> > and dev_max_pfn is supposed to be returning the pfn of the dma_mask, it
> > should unconditionally be 64 bits as well.  Either that or it should
> > return dma_addr_t.
> 
> My reasoning is that PFNs in the system are always of type "unsigned long"
> and therefore a function returning a pfn should have that type.  If we
> overflow a PFN fitting in an unsigned long, we have lots of places which
> need fixing.

It's not intuitive to people who need the dma mask that they're supposed
to use dma_max_pfn() << PAGE_SHIFT but now they have to worry about the
casting and, if they don't get it right, nothing will warn or tell them.
what about a new macro, say dma_max_mask(dev) that just returns
(u64)dma_max_pfn() << PAGE_SHIFT?

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 19/22] pmcraid: Use pci_enable_msix_range()

2014-02-13 Thread Alexander Gordeev
On Tue, Feb 04, 2014 at 12:17:05PM +0100, Alexander Gordeev wrote:
> As result of deprecation of MSI-X/MSI enablement functions
> pci_enable_msix() and pci_enable_msi_block() all drivers
> using these two interfaces need to be updated to use the
> new pci_enable_msi_range() and pci_enable_msix_range()
> interfaces.

Hi Anil,

Any feedback?

Thanks!

-- 
Regards,
Alexander Gordeev
agord...@redhat.com
--
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 21/22] qla4xxx: Use pci_enable_msix_range()

2014-02-13 Thread Alexander Gordeev
On Tue, Feb 04, 2014 at 12:17:07PM +0100, Alexander Gordeev wrote:
> As result of deprecation of MSI-X/MSI enablement functions
> pci_enable_msix() and pci_enable_msi_block() all drivers
> using these two interfaces need to be updated to use the
> new pci_enable_msi_range() and pci_enable_msix_range()
> interfaces.

Hi Vikas,

Any feedback on this?

Thanks!

-- 
Regards,
Alexander Gordeev
agord...@redhat.com
--
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 18/22] pmcraid: Get rid of a redundant assignment

2014-02-13 Thread Alexander Gordeev
On Tue, Feb 04, 2014 at 12:17:04PM +0100, Alexander Gordeev wrote:

Anil,

Any feedback?

Thanks!

-- 
Regards,
Alexander Gordeev
agord...@redhat.com
--
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 14/22] mpt2sas: Use pci_enable_msix_range()

2014-02-13 Thread Alexander Gordeev
On Tue, Feb 04, 2014 at 12:17:00PM +0100, Alexander Gordeev wrote:
> As result of deprecation of MSI-X/MSI enablement functions
> pci_enable_msix() and pci_enable_msi_block() all drivers
> using these two interfaces need to be updated to use the
> new pci_enable_msi_range() and pci_enable_msix_range()
> interfaces.

Nagalakshmi, Sreekanth,

Any feedback?

Thanks!

-- 
Regards,
Alexander Gordeev
agord...@redhat.com
--
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 11/22] lpfc: Remove superfluous call to pci_disable_msix()

2014-02-13 Thread Alexander Gordeev
On Tue, Feb 04, 2014 at 12:16:57PM +0100, Alexander Gordeev wrote:

Hi James,

Any feedback?

Thanks!

-- 
Regards,
Alexander Gordeev
agord...@redhat.com
--
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/22] bfa: Do not call pci_enable_msix() once it failed

2014-02-13 Thread Alexander Gordeev
On Tue, Feb 04, 2014 at 12:16:48PM +0100, Alexander Gordeev wrote:
> Function pci_enable_msix() should not be called again in
> case it threw a negative errno.

Anil, Vijaya,

Any feedback on bfa patches?

Thanks!

-- 
Regards,
Alexander Gordeev
agord...@redhat.com
--
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] Fix uses of dma_max_pfn() when converting to a limiting address

2014-02-13 Thread Russell King - ARM Linux
On Thu, Feb 13, 2014 at 08:58:10AM -0800, James Bottomley wrote:
> This doesn't really look like the right fix.  You replaced dev->dma_mask
> with a calculation on dev_max_pfn().  Since dev->dma_mask is always u64
> and dev_max_pfn is supposed to be returning the pfn of the dma_mask, it
> should unconditionally be 64 bits as well.  Either that or it should
> return dma_addr_t.

My reasoning is that PFNs in the system are always of type "unsigned long"
and therefore a function returning a pfn should have that type.  If we
overflow a PFN fitting in an unsigned long, we have lots of places which
need fixing.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
--
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] Fix uses of dma_max_pfn() when converting to a limiting address

2014-02-13 Thread James Bottomley
On Tue, 2014-02-11 at 17:28 +, Russell King wrote:
> We must use a 64-bit for this, otherwise overflowed bits get lost, and
> that can result in a lower than intended value set.
> 
> Fixes: 8e0cb8a1f6ac ("ARM: 7797/1: mmc: Use dma_max_pfn(dev) helper for 
> bounce_limit calculations")
> Fixes: 7d35496dd982 ("ARM: 7796/1: scsi: Use dma_max_pfn(dev) helper for 
> bounce_limit calculations")
> Signed-off-by: Russell King 
> ---
> While poking about with the Cubox-i4 and investigating why my UHS-1
> SD card wasn't achieving its full potential, I came across a slight
> problem... the SDHCI host sets a mask of 0x, but with the
> start of memory at pfn 0x1, the blk code sees this when setting
> the bounce limit:
> 
>   max addr 0x0000 bounce limit 0x dma 1
> 
> and this results in the bounce functions appearing in the profile:
> 
> c00f8b70 copy_to_high_bio_irq  1139 2.5886
> c00f8d28 bounce_end_io   12 0.0714
> c00f8dd0 bounce_end_io_read_isa   8 0.1053
> 
> which, compared to the cost of copying the data to userland and
> request handling, this is quite significant:
> 
> c04b1794 sdhci_request  268 0.5447
> c02d0740 __copy_to_user_std 398 0.4252
> 
> With this calculation fixed, we avoid the bouncing code entirely.
> 
>   max addr 0x10000 bounce limit 0x10 dma 0
> 
>  drivers/mmc/card/queue.c | 2 +-
>  drivers/scsi/scsi_lib.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index 357bbc54fe4b..3e049c13429c 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -197,7 +197,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card 
> *card,
>   struct mmc_queue_req *mqrq_prev = &mq->mqrq[1];
>  
>   if (mmc_dev(host)->dma_mask && *mmc_dev(host)->dma_mask)
> - limit = dma_max_pfn(mmc_dev(host)) << PAGE_SHIFT;
> + limit = (u64)dma_max_pfn(mmc_dev(host)) << PAGE_SHIFT;
>  
>   mq->card = card;
>   mq->queue = blk_init_queue(mmc_request_fn, lock);
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 7bd7f0d5f050..62ec84b42e31 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1684,7 +1684,7 @@ u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
>  
>   host_dev = scsi_get_device(shost);
>   if (host_dev && host_dev->dma_mask)
> - bounce_limit = dma_max_pfn(host_dev) << PAGE_SHIFT;
> + bounce_limit = (u64)dma_max_pfn(host_dev) << PAGE_SHIFT;

This doesn't really look like the right fix.  You replaced dev->dma_mask
with a calculation on dev_max_pfn().  Since dev->dma_mask is always u64
and dev_max_pfn is supposed to be returning the pfn of the dma_mask, it
should unconditionally be 64 bits as well.  Either that or it should
return dma_addr_t.

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] Fix uses of dma_max_pfn() when converting to a limiting address

2014-02-13 Thread Santosh Shilimkar
On Tuesday 11 February 2014 12:28 PM, Russell King wrote:
> We must use a 64-bit for this, otherwise overflowed bits get lost, and
> that can result in a lower than intended value set.
> 
> Fixes: 8e0cb8a1f6ac ("ARM: 7797/1: mmc: Use dma_max_pfn(dev) helper for 
> bounce_limit calculations")
> Fixes: 7d35496dd982 ("ARM: 7796/1: scsi: Use dma_max_pfn(dev) helper for 
> bounce_limit calculations")
> Signed-off-by: Russell King 
> ---
> While poking about with the Cubox-i4 and investigating why my UHS-1
> SD card wasn't achieving its full potential, I came across a slight
> problem... the SDHCI host sets a mask of 0x, but with the
> start of memory at pfn 0x1, the blk code sees this when setting
> the bounce limit:
> 
>   max addr 0x0000 bounce limit 0x dma 1
> 
> and this results in the bounce functions appearing in the profile:
> 
> c00f8b70 copy_to_high_bio_irq  1139 2.5886
> c00f8d28 bounce_end_io   12 0.0714
> c00f8dd0 bounce_end_io_read_isa   8 0.1053
> 
> which, compared to the cost of copying the data to userland and
> request handling, this is quite significant:
> 
> c04b1794 sdhci_request  268 0.5447
> c02d0740 __copy_to_user_std 398 0.4252
> 
> With this calculation fixed, we avoid the bouncing code entirely.
> 
>   max addr 0x10000 bounce limit 0x10 dma 0
>
You are right. We were seeing an issue with SCSI HDD over PCI and
the patch fixes that issue as well.

I think we should send this patch to stable as well. At least 3.13
should get this patch.

Thanks RMK for the bug fix. Feel free to add, in case the patch is
not committed already

Tested-Acked-by: Santosh Shilimkar 
 
>  drivers/mmc/card/queue.c | 2 +-
>  drivers/scsi/scsi_lib.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index 357bbc54fe4b..3e049c13429c 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -197,7 +197,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card 
> *card,
>   struct mmc_queue_req *mqrq_prev = &mq->mqrq[1];
>  
>   if (mmc_dev(host)->dma_mask && *mmc_dev(host)->dma_mask)
> - limit = dma_max_pfn(mmc_dev(host)) << PAGE_SHIFT;
> + limit = (u64)dma_max_pfn(mmc_dev(host)) << PAGE_SHIFT;
>  
>   mq->card = card;
>   mq->queue = blk_init_queue(mmc_request_fn, lock);
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 7bd7f0d5f050..62ec84b42e31 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1684,7 +1684,7 @@ u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
>  
>   host_dev = scsi_get_device(shost);
>   if (host_dev && host_dev->dma_mask)
> - bounce_limit = dma_max_pfn(host_dev) << PAGE_SHIFT;
> + bounce_limit = (u64)dma_max_pfn(host_dev) << PAGE_SHIFT;
>  
>   return bounce_limit;
>  }
> 

--
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] modify sg_inq to read from hexdump

2014-02-13 Thread Hannes Reinecke
Hi Doug,

and here's now the follow-up to my patchset 'Add EVPD pages to
sysfs'; with this small tweak sg_inq can read the EVPD dumps from
sysfs without any I/O.

So I guess we're good to go with just having the raw data for
the EVPD pages in sysfs, James.

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)
>From e95f4f11f2453c77204258b11beeb33adac53dc5 Mon Sep 17 00:00:00 2001
From: Hannes Reinecke 
Date: Thu, 13 Feb 2014 13:43:31 +0100
Subject: [PATCH] sg_inq: Read hexdump instead of using SG_IO

Implement a flag '-D' which will try to read a file containing
a hexdump instead of a 'normal' file descriptor.

Signed-off-by: Hannes Reinecke 
---
 include/sg_lib.h |  1 +
 lib/sg_lib.c | 25 +
 src/sg_inq.c | 32 +++-
 3 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/include/sg_lib.h b/include/sg_lib.h
index 03eb03c..722bdb9 100644
--- a/include/sg_lib.h
+++ b/include/sg_lib.h
@@ -293,6 +293,7 @@ int sg_vpd_dev_id_iter(const unsigned char * initial_desig_desc, int page_len,
  * If errnum is negative, flip its sign. */
 char * safe_strerror(int errnum);
 
+int sg_ll_read_hex(int sg_fd, unsigned char *str, int len);
 
 /* Print (to stdout) 'str' of bytes in hex, 16 bytes per line optionally
  * followed at the right hand side of the line with an ASCII interpretation.
diff --git a/lib/sg_lib.c b/lib/sg_lib.c
index 1f69980..5fe8be2 100644
--- a/lib/sg_lib.c
+++ b/lib/sg_lib.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #define __STDC_FORMAT_MACROS 1
 #include 
 
@@ -1404,6 +1405,30 @@ safe_strerror(int errnum)
 return errstr;
 }
 
+int
+sg_ll_read_hex(int sg_fd, unsigned char *str, int len)
+{
+char buf[48];
+unsigned int n;
+int r = 0, off, bytes;
+
+do {
+off = 0;
+bytes = read(sg_fd, buf, 48);
+if (bytes < 0)
+return -1;
+while (off < bytes) {
+if (sscanf(buf + off, "%x ", &n) != 1)
+break;
+if (n > 255)
+break;
+str[r] = n;
+r++;
+off += 3;
+}
+} while (bytes > 0 && r < len);
+return 0;
+}
 
 /* Note the ASCII-hex output goes to stdout. [Most other output from functions
in this file go to sg_warnings_strm (default stderr).]
diff --git a/src/sg_inq.c b/src/sg_inq.c
index 26bf3c3..cee8f5f 100644
--- a/src/sg_inq.c
+++ b/src/sg_inq.c
@@ -186,6 +186,7 @@ static struct option long_options[] = {
 {"block", required_argument, 0, 'B'},
 {"cmddt", no_argument, 0, 'c'},
 {"descriptors", no_argument, 0, 'd'},
+{"hexdump", no_argument, 0, 'D'},
 {"export", no_argument, 0, 'u'},
 {"extended", no_argument, 0, 'x'},
 {"help", no_argument, 0, 'h'},
@@ -220,6 +221,7 @@ struct opts_t {
 int do_version;
 int do_decode;
 int do_vpd;
+int hexdump;
 int resp_len;
 int page_num;
 int num_pages;
@@ -392,18 +394,18 @@ cl_new_process(struct opts_t * optsp, int argc, char * argv[])
 
 #ifdef SG_LIB_LINUX
 #ifdef SG_SCSI_STRINGS
-c = getopt_long(argc, argv, "aB:cdeEhHil:m:NOp:rsuvVx", long_options,
+c = getopt_long(argc, argv, "aB:cdDeEhHil:m:NOp:rsuvVx", long_options,
 &option_index);
 #else
-c = getopt_long(argc, argv, "B:cdeEhHil:m:p:rsuvVx", long_options,
+c = getopt_long(argc, argv, "B:cdDeEhHil:m:p:rsuvVx", long_options,
 &option_index);
 #endif /* SG_SCSI_STRINGS */
 #else  /* SG_LIB_LINUX */
 #ifdef SG_SCSI_STRINGS
-c = getopt_long(argc, argv, "B:cdeEhHil:m:NOp:rsuvVx", long_options,
+c = getopt_long(argc, argv, "B:cdDeEhHil:m:NOp:rsuvVx", long_options,
 &option_index);
 #else
-c = getopt_long(argc, argv, "B:cdeEhHil:m:p:rsuvVx", long_options,
+c = getopt_long(argc, argv, "B:cdDeEhHil:m:p:rsuvVx", long_options,
 &option_index);
 #endif /* SG_SCSI_STRINGS */
 #endif /* SG_LIB_LINUX */
@@ -436,6 +438,9 @@ cl_new_process(struct opts_t * optsp, int argc, char * argv[])
 case 'd':
 ++optsp->do_descriptors;
 break;
+case 'D':
+++optsp->hexdump;
+break;
 case 'e':
 ++optsp->do_vpd;
 break;
@@ -526,7 +531,7 @@ cl_new_process(struct opts_t * optsp, int argc, char * argv[])
 static int
 cl_old_process(struct opts_t * optsp, int argc, char * argv[])
 {
-int k, jmp_out, plen, num, n;
+int k, jmp_out, plen, n, num = 0;
 const char * cp;
 
 for (k = 1; k < argc; ++k) {
@@ -572,6 +577,9 @@ cl_old_process(struct opts_t * optsp, int argc, char * argv[])
 ++optsp->do_descriptors;
 ++optsp->do_decode

[PATCHv7 0/3][Resend] Display EVPD pages in sysfs

2014-02-13 Thread Hannes Reinecke
Hi all,

After discussion with jejb I've dropped the EVPD parsing.
So with this version we're just displaying the EVPD page
0x80 and 0x83 as hexdumps; no parsing is attempted.
This drastically simplifies the patch, and we don't
have to worry about any parsing errors in kernel space.
Of course we'll need a parser in userspace, but that
doesn't need to do any I/O. So it's still a very nice
gain.

(I had to redo the patchset as it no longer applied to
 -rc2. Hence the resend. Sorry for that.)

Hannes Reinecke (3):
  scsi_sysfs: Implement 'is_visible' callback
  Add EVPD page 0x83 to sysfs
  Add EVPD page 0x80 to sysfs

 drivers/scsi/scsi.c| 104 ++--
 drivers/scsi/scsi_scan.c   |   3 +
 drivers/scsi/scsi_sysfs.c  | 229 +++--
 include/scsi/scsi_device.h |   5 +
 4 files changed, 242 insertions(+), 99 deletions(-)

-- 
1.7.12.4

--
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 2/3] Add EVPD page 0x83 to sysfs

2014-02-13 Thread Hannes Reinecke
EVPD page 0x83 is used to uniquely identify the device.
So instead of having each and every program issue a separate
SG_IO call to retrieve this information it does make far more
sense to display it in sysfs.

Cc: Jeremy Linton 
Cc: Kay Sievers 
Cc: Doug Gilbert 
Cc: Kai Makisara 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/scsi.c| 79 ++
 drivers/scsi/scsi_scan.c   |  3 ++
 drivers/scsi/scsi_sysfs.c  | 39 ++-
 include/scsi/scsi_device.h |  3 ++
 4 files changed, 116 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index d8afec8..57105e0 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -954,7 +954,7 @@ EXPORT_SYMBOL(scsi_track_queue_full);
  * This is an internal helper function.  You probably want to use
  * scsi_get_vpd_page instead.
  *
- * Returns 0 on success or a negative error number.
+ * Returns size of the vpg page on success or a negative error number.
  */
 static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char *buffer,
u8 page, unsigned len)
@@ -962,6 +962,9 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, 
unsigned char *buffer,
int result;
unsigned char cmd[16];
 
+   if (len < 4)
+   return -EINVAL;
+
cmd[0] = INQUIRY;
cmd[1] = 1; /* EVPD */
cmd[2] = page;
@@ -982,7 +985,7 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, 
unsigned char *buffer,
if (buffer[1] != page)
return -EIO;
 
-   return 0;
+   return (buffer[2] << 8) + buffer[3] + 4;
 }
 
 /**
@@ -1009,18 +1012,18 @@ int scsi_get_vpd_page(struct scsi_device *sdev, u8 
page, unsigned char *buf,
 
/* Ask for all the pages supported by this device */
result = scsi_vpd_inquiry(sdev, buf, 0, buf_len);
-   if (result)
+   if (result < 4)
goto fail;
 
/* If the user actually wanted this page, we can skip the rest */
if (page == 0)
return 0;
 
-   for (i = 0; i < min((int)buf[3], buf_len - 4); i++)
-   if (buf[i + 4] == page)
+   for (i = 4; i < min(result, buf_len); i++)
+   if (buf[i] == page)
goto found;
 
-   if (i < buf[3] && i >= buf_len - 4)
+   if (i < result && i >= buf_len)
/* ran off the end of the buffer, give us benefit of doubt */
goto found;
/* The device claims it doesn't support the requested page */
@@ -1028,7 +1031,7 @@ int scsi_get_vpd_page(struct scsi_device *sdev, u8 page, 
unsigned char *buf,
 
  found:
result = scsi_vpd_inquiry(sdev, buf, page, buf_len);
-   if (result)
+   if (result < 0)
goto fail;
 
return 0;
@@ -1039,6 +1042,68 @@ int scsi_get_vpd_page(struct scsi_device *sdev, u8 page, 
unsigned char *buf,
 EXPORT_SYMBOL_GPL(scsi_get_vpd_page);
 
 /**
+ * scsi_attach_vpd - Attach Vital Product Data to a SCSI device structure
+ * @sdev: The device to ask
+ *
+ * Attach the 'Device Identification' VPD page (0x83) to a SCSI device
+ * structure. This information can be used to identify the device
+ * uniquely.
+ */
+void scsi_attach_vpd(struct scsi_device *sdev)
+{
+   int result, i;
+   int vpd_len = 255;
+   int pg83_supported = 0;
+   unsigned char *vpd_buf;
+
+   if (sdev->skip_vpd_pages)
+   return;
+retry_pg0:
+   vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
+   if (!vpd_buf)
+   return;
+
+   /* Ask for all the pages supported by this device */
+   result = scsi_vpd_inquiry(sdev, vpd_buf, 0, vpd_len);
+   if (result < 0) {
+   kfree(vpd_buf);
+   return;
+   }
+   if (result > vpd_len) {
+   vpd_len = result;
+   kfree(vpd_buf);
+   goto retry_pg0;
+   }
+
+   for (i = 4; i < result; i++) {
+   if (vpd_buf[i] == 0x83) {
+   pg83_supported = 1;
+   }
+   }
+   kfree(vpd_buf);
+
+   if (pg83_supported) {
+retry_pg83:
+   vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
+   if (!vpd_buf)
+   return;
+
+   result = scsi_vpd_inquiry(sdev, vpd_buf, 0x83, vpd_len);
+   if (result < 0) {
+   kfree(vpd_buf);
+   return;
+   }
+   if (result > vpd_len) {
+   vpd_len = result;
+   kfree(vpd_buf);
+   goto retry_pg83;
+   }
+   sdev->vpd_pg83_len = result;
+   sdev->vpd_pg83 = vpd_buf;
+   }
+}
+
+/**
  * scsi_report_opcode - Find out if a given command opcode is supported
  * @sdev:  scsi device to query
  * @buffer:scratch buffer (must be at least 20 bytes long)
diff --git a/drivers/scsi/s

[PATCH 3/3] Add EVPD page 0x80 to sysfs

2014-02-13 Thread Hannes Reinecke
Some older devices (most notably tapes) will only report reliable
information in page 0x80 (Unit Serial Number). So export this
in the sysfs attribute 'vpd_pg80'.

Cc: Doug Gilbert 
Cc: Jeremy Linton 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/scsi.c| 27 ++-
 drivers/scsi/scsi_sysfs.c  |  6 ++
 include/scsi/scsi_device.h |  2 ++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 57105e0..defa62d 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -1045,7 +1045,8 @@ EXPORT_SYMBOL_GPL(scsi_get_vpd_page);
  * scsi_attach_vpd - Attach Vital Product Data to a SCSI device structure
  * @sdev: The device to ask
  *
- * Attach the 'Device Identification' VPD page (0x83) to a SCSI device
+ * Attach the 'Device Identification' VPD page (0x83) and the
+ * 'Unit Serial Number' VPD page (0x80) to a SCSI device
  * structure. This information can be used to identify the device
  * uniquely.
  */
@@ -1053,6 +1054,7 @@ void scsi_attach_vpd(struct scsi_device *sdev)
 {
int result, i;
int vpd_len = 255;
+   int pg80_supported = 0;
int pg83_supported = 0;
unsigned char *vpd_buf;
 
@@ -1076,12 +1078,35 @@ retry_pg0:
}
 
for (i = 4; i < result; i++) {
+   if (vpd_buf[i] == 0x80) {
+   pg80_supported = 1;
+   }
if (vpd_buf[i] == 0x83) {
pg83_supported = 1;
}
}
kfree(vpd_buf);
 
+   if (pg80_supported) {
+retry_pg80:
+   vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
+   if (!vpd_buf)
+   return;
+
+   result = scsi_vpd_inquiry(sdev, vpd_buf, 0x80, vpd_len);
+   if (result < 0) {
+   kfree(vpd_buf);
+   return;
+   }
+   if (result > vpd_len) {
+   vpd_len = result;
+   kfree(vpd_buf);
+   goto retry_pg80;
+   }
+   sdev->vpd_pg80_len = result;
+   sdev->vpd_pg80 = vpd_buf;
+   }
+
if (pg83_supported) {
 retry_pg83:
vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index ca19448..03f97ab 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -784,6 +784,7 @@ show_vpd_##page(struct device *dev, struct device_attribute 
*attr, \
 static DEVICE_ATTR(vpd_##page, S_IRUGO, show_vpd_##page, NULL);
 
 sdev_vpd_pg_attr(pg83);
+sdev_vpd_pg_attr(pg80);
 
 static ssize_t
 show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
@@ -941,6 +942,10 @@ static umode_t scsi_sdev_attr_is_visible(struct kobject 
*kobj,
!sdev->vpd_pg83)
return 0;
 
+   if (attr == &dev_attr_vpd_pg80.attr &&
+   !sdev->vpd_pg80)
+   return 0;
+
return attr->mode;
 }
 
@@ -966,6 +971,7 @@ static struct attribute *scsi_sdev_attrs[] = {
&dev_attr_queue_depth.attr,
&dev_attr_queue_type.attr,
&dev_attr_queue_ramp_up_period.attr,
+   &dev_attr_vpd_pg80.attr,
&dev_attr_vpd_pg83.attr,
REF_EVT(media_change),
REF_EVT(inquiry_change_reported),
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index d209f04..2ea8212 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -115,6 +115,8 @@ struct scsi_device {
const char * rev;   /* ... "nullnullnullnull" before scan */
unsigned char vpd_pg83_len;
unsigned char *vpd_pg83;
+   unsigned char vpd_pg80_len;
+   unsigned char *vpd_pg80;
unsigned char current_tag;  /* current tag */
struct scsi_target  *sdev_target;   /* used only for single_lun */
 
-- 
1.7.12.4

--
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 1/3] scsi_sysfs: Implement 'is_visible' callback

2014-02-13 Thread Hannes Reinecke
Instead of modifying attributes after the device has been created
we should be using the 'is_visible' callback to avoid races.

Signed-off-by: Hannes Reinecke 
Reviewed-by: Christoph Hellwig 
---
 drivers/scsi/scsi_sysfs.c | 184 +++---
 1 file changed, 93 insertions(+), 91 deletions(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 9117d0b..196e59a 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -579,7 +579,6 @@ static int scsi_sdev_check_buf_bit(const char *buf)
  * Create the actual show/store functions and data structures.
  */
 sdev_rd_attr (device_blocked, "%d\n");
-sdev_rd_attr (queue_depth, "%d\n");
 sdev_rd_attr (device_busy, "%d\n");
 sdev_rd_attr (type, "%d\n");
 sdev_rd_attr (scsi_level, "%d\n");
@@ -723,7 +722,37 @@ show_queue_type_field(struct device *dev, struct 
device_attribute *attr,
return snprintf(buf, 20, "%s\n", name);
 }
 
-static DEVICE_ATTR(queue_type, S_IRUGO, show_queue_type_field, NULL);
+static ssize_t
+store_queue_type_field(struct device *dev, struct device_attribute *attr,
+  const char *buf, size_t count)
+{
+   struct scsi_device *sdev = to_scsi_device(dev);
+   struct scsi_host_template *sht = sdev->host->hostt;
+   int tag_type = 0, retval;
+   int prev_tag_type = scsi_get_tag_type(sdev);
+
+   if (!sdev->tagged_supported || !sht->change_queue_type)
+   return -EINVAL;
+
+   if (strncmp(buf, "ordered", 7) == 0)
+   tag_type = MSG_ORDERED_TAG;
+   else if (strncmp(buf, "simple", 6) == 0)
+   tag_type = MSG_SIMPLE_TAG;
+   else if (strncmp(buf, "none", 4) != 0)
+   return -EINVAL;
+
+   if (tag_type == prev_tag_type)
+   return count;
+
+   retval = sht->change_queue_type(sdev, tag_type);
+   if (retval < 0)
+   return retval;
+
+   return count;
+}
+
+static DEVICE_ATTR(queue_type, S_IRUGO | S_IWUSR, show_queue_type_field,
+  store_queue_type_field);
 
 static ssize_t
 show_iostat_counterbits(struct device *dev, struct device_attribute *attr, 
char *buf)
@@ -797,46 +826,9 @@ DECLARE_EVT(soft_threshold_reached, 
SOFT_THRESHOLD_REACHED_REPORTED)
 DECLARE_EVT(mode_parameter_change_reported, MODE_PARAMETER_CHANGE_REPORTED)
 DECLARE_EVT(lun_change_reported, LUN_CHANGE_REPORTED)
 
-/* Default template for device attributes.  May NOT be modified */
-static struct attribute *scsi_sdev_attrs[] = {
-   &dev_attr_device_blocked.attr,
-   &dev_attr_type.attr,
-   &dev_attr_scsi_level.attr,
-   &dev_attr_device_busy.attr,
-   &dev_attr_vendor.attr,
-   &dev_attr_model.attr,
-   &dev_attr_rev.attr,
-   &dev_attr_rescan.attr,
-   &dev_attr_delete.attr,
-   &dev_attr_state.attr,
-   &dev_attr_timeout.attr,
-   &dev_attr_eh_timeout.attr,
-   &dev_attr_iocounterbits.attr,
-   &dev_attr_iorequest_cnt.attr,
-   &dev_attr_iodone_cnt.attr,
-   &dev_attr_ioerr_cnt.attr,
-   &dev_attr_modalias.attr,
-   REF_EVT(media_change),
-   REF_EVT(inquiry_change_reported),
-   REF_EVT(capacity_change_reported),
-   REF_EVT(soft_threshold_reached),
-   REF_EVT(mode_parameter_change_reported),
-   REF_EVT(lun_change_reported),
-   NULL
-};
-
-static struct attribute_group scsi_sdev_attr_group = {
-   .attrs =scsi_sdev_attrs,
-};
-
-static const struct attribute_group *scsi_sdev_attr_groups[] = {
-   &scsi_sdev_attr_group,
-   NULL
-};
-
 static ssize_t
-sdev_store_queue_depth_rw(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count)
+sdev_store_queue_depth(struct device *dev, struct device_attribute *attr,
+  const char *buf, size_t count)
 {
int depth, retval;
struct scsi_device *sdev = to_scsi_device(dev);
@@ -859,10 +851,10 @@ sdev_store_queue_depth_rw(struct device *dev, struct 
device_attribute *attr,
 
return count;
 }
+sdev_show_function(queue_depth, "%d\n");
 
-static struct device_attribute sdev_attr_queue_depth_rw =
-   __ATTR(queue_depth, S_IRUGO | S_IWUSR, sdev_show_queue_depth,
-  sdev_store_queue_depth_rw);
+static DEVICE_ATTR(queue_depth, S_IRUGO | S_IWUSR, sdev_show_queue_depth,
+  sdev_store_queue_depth);
 
 static ssize_t
 sdev_show_queue_ramp_up_period(struct device *dev,
@@ -890,40 +882,73 @@ sdev_store_queue_ramp_up_period(struct device *dev,
return period;
 }
 
-static struct device_attribute sdev_attr_queue_ramp_up_period =
-   __ATTR(queue_ramp_up_period, S_IRUGO | S_IWUSR,
-  sdev_show_queue_ramp_up_period,
-  sdev_store_queue_ramp_up_period);
+static DEVICE_ATTR(queue_ramp_up_period, S_IRUGO | S_IWUSR,
+  sdev_show_queue_ramp_up_period,
+  sdev_store_queue_ramp_up_period);
 
-static ssize_t
-sdev

Re: [PATCH 06/16] scsi_dh_alua: use local buffer for VPD inquiry

2014-02-13 Thread Hannes Reinecke
On 02/13/2014 10:51 AM, Maurizio Lombardi wrote:
> Hi,
> 
> On 01/31/2014 10:29 AM, Hannes Reinecke wrote:
>>  static int alua_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data 
>> *h)
>>  {
>> +unsigned char *buff;
>> +unsigned char bufflen = 36;
>>  int len, timeout = ALUA_FAILOVER_TIMEOUT;
> [...]
>> +len = (buff[2] << 8) + buff[3] + 4;
>> +if (len > bufflen) {
> [...]
>> +bufflen = len;
> 
> just a nit: is it safe to use char as the type of bufflen? Isn't better
> to declare it as int just in case len is > than 255 ?
> 
Yes, true.

However, this whole section needs to be reworked anyway, as there's
a fair chance we're getting the VPD page 0x83 for free.
(Cf my latest patchset 'Display EVPD pages in sysfs').

And jejb made some positive noises that, so I'll be reworking the
scsi_dh_alua patchset to take advantage of the new fields.

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


[PATCH 1/3] scsi_sysfs: Implement 'is_visible' callback

2014-02-13 Thread Hannes Reinecke
Instead of modifying attributes after the device has been created
we should be using the 'is_visible' callback to avoid races.

Signed-off-by: Hannes Reinecke 
Reviewed-by: Christoph Hellwig 
---
 drivers/scsi/scsi_sysfs.c | 184 +++---
 1 file changed, 93 insertions(+), 91 deletions(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 9117d0b..196e59a 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -579,7 +579,6 @@ static int scsi_sdev_check_buf_bit(const char *buf)
  * Create the actual show/store functions and data structures.
  */
 sdev_rd_attr (device_blocked, "%d\n");
-sdev_rd_attr (queue_depth, "%d\n");
 sdev_rd_attr (device_busy, "%d\n");
 sdev_rd_attr (type, "%d\n");
 sdev_rd_attr (scsi_level, "%d\n");
@@ -723,7 +722,37 @@ show_queue_type_field(struct device *dev, struct 
device_attribute *attr,
return snprintf(buf, 20, "%s\n", name);
 }
 
-static DEVICE_ATTR(queue_type, S_IRUGO, show_queue_type_field, NULL);
+static ssize_t
+store_queue_type_field(struct device *dev, struct device_attribute *attr,
+  const char *buf, size_t count)
+{
+   struct scsi_device *sdev = to_scsi_device(dev);
+   struct scsi_host_template *sht = sdev->host->hostt;
+   int tag_type = 0, retval;
+   int prev_tag_type = scsi_get_tag_type(sdev);
+
+   if (!sdev->tagged_supported || !sht->change_queue_type)
+   return -EINVAL;
+
+   if (strncmp(buf, "ordered", 7) == 0)
+   tag_type = MSG_ORDERED_TAG;
+   else if (strncmp(buf, "simple", 6) == 0)
+   tag_type = MSG_SIMPLE_TAG;
+   else if (strncmp(buf, "none", 4) != 0)
+   return -EINVAL;
+
+   if (tag_type == prev_tag_type)
+   return count;
+
+   retval = sht->change_queue_type(sdev, tag_type);
+   if (retval < 0)
+   return retval;
+
+   return count;
+}
+
+static DEVICE_ATTR(queue_type, S_IRUGO | S_IWUSR, show_queue_type_field,
+  store_queue_type_field);
 
 static ssize_t
 show_iostat_counterbits(struct device *dev, struct device_attribute *attr, 
char *buf)
@@ -797,46 +826,9 @@ DECLARE_EVT(soft_threshold_reached, 
SOFT_THRESHOLD_REACHED_REPORTED)
 DECLARE_EVT(mode_parameter_change_reported, MODE_PARAMETER_CHANGE_REPORTED)
 DECLARE_EVT(lun_change_reported, LUN_CHANGE_REPORTED)
 
-/* Default template for device attributes.  May NOT be modified */
-static struct attribute *scsi_sdev_attrs[] = {
-   &dev_attr_device_blocked.attr,
-   &dev_attr_type.attr,
-   &dev_attr_scsi_level.attr,
-   &dev_attr_device_busy.attr,
-   &dev_attr_vendor.attr,
-   &dev_attr_model.attr,
-   &dev_attr_rev.attr,
-   &dev_attr_rescan.attr,
-   &dev_attr_delete.attr,
-   &dev_attr_state.attr,
-   &dev_attr_timeout.attr,
-   &dev_attr_eh_timeout.attr,
-   &dev_attr_iocounterbits.attr,
-   &dev_attr_iorequest_cnt.attr,
-   &dev_attr_iodone_cnt.attr,
-   &dev_attr_ioerr_cnt.attr,
-   &dev_attr_modalias.attr,
-   REF_EVT(media_change),
-   REF_EVT(inquiry_change_reported),
-   REF_EVT(capacity_change_reported),
-   REF_EVT(soft_threshold_reached),
-   REF_EVT(mode_parameter_change_reported),
-   REF_EVT(lun_change_reported),
-   NULL
-};
-
-static struct attribute_group scsi_sdev_attr_group = {
-   .attrs =scsi_sdev_attrs,
-};
-
-static const struct attribute_group *scsi_sdev_attr_groups[] = {
-   &scsi_sdev_attr_group,
-   NULL
-};
-
 static ssize_t
-sdev_store_queue_depth_rw(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count)
+sdev_store_queue_depth(struct device *dev, struct device_attribute *attr,
+  const char *buf, size_t count)
 {
int depth, retval;
struct scsi_device *sdev = to_scsi_device(dev);
@@ -859,10 +851,10 @@ sdev_store_queue_depth_rw(struct device *dev, struct 
device_attribute *attr,
 
return count;
 }
+sdev_show_function(queue_depth, "%d\n");
 
-static struct device_attribute sdev_attr_queue_depth_rw =
-   __ATTR(queue_depth, S_IRUGO | S_IWUSR, sdev_show_queue_depth,
-  sdev_store_queue_depth_rw);
+static DEVICE_ATTR(queue_depth, S_IRUGO | S_IWUSR, sdev_show_queue_depth,
+  sdev_store_queue_depth);
 
 static ssize_t
 sdev_show_queue_ramp_up_period(struct device *dev,
@@ -890,40 +882,73 @@ sdev_store_queue_ramp_up_period(struct device *dev,
return period;
 }
 
-static struct device_attribute sdev_attr_queue_ramp_up_period =
-   __ATTR(queue_ramp_up_period, S_IRUGO | S_IWUSR,
-  sdev_show_queue_ramp_up_period,
-  sdev_store_queue_ramp_up_period);
+static DEVICE_ATTR(queue_ramp_up_period, S_IRUGO | S_IWUSR,
+  sdev_show_queue_ramp_up_period,
+  sdev_store_queue_ramp_up_period);
 
-static ssize_t
-sdev

[PATCHv7 0/3] Display EVPD pages in sysfs

2014-02-13 Thread Hannes Reinecke
Hi all,

After discussion with jejb I've dropped the EVPD parsing.
So with this version we're just displaying the EVPD page
0x80 and 0x83 as hexdumps; no parsing is attempted.
This drastically simplifies the patch, and we don't
have to worry about any parsing errors in kernel space.

Hannes Reinecke (3):
  scsi_sysfs: Implement 'is_visible' callback
  Add EVPD page 0x83 to sysfs
  Add EVPD page 0x80 to sysfs

 drivers/scsi/scsi.c| 104 ++--
 drivers/scsi/scsi_scan.c   |   3 +
 drivers/scsi/scsi_sysfs.c  | 229 +++--
 include/scsi/scsi.h|   4 +-
 include/scsi/scsi_device.h |   5 +
 5 files changed, 245 insertions(+), 100 deletions(-)

-- 
1.7.12.4

--
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 3/3] Add EVPD page 0x80 to sysfs

2014-02-13 Thread Hannes Reinecke
Some older devices (most notably tapes) will only report reliable
information in page 0x80 (Unit Serial Number). So export this
in the sysfs attribute 'vpd_pg80'.

Cc: Doug Gilbert 
Cc: Jeremy Linton 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/scsi.c| 27 ++-
 drivers/scsi/scsi_sysfs.c  |  6 ++
 include/scsi/scsi_device.h |  2 ++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index e4cd88d..0615d94 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -1045,7 +1045,8 @@ EXPORT_SYMBOL_GPL(scsi_get_vpd_page);
  * scsi_attach_vpd - Attach Vital Product Data to a SCSI device structure
  * @sdev: The device to ask
  *
- * Attach the 'Device Identification' VPD page (0x83) to a SCSI device
+ * Attach the 'Device Identification' VPD page (0x83) and the
+ * 'Unit Serial Number' VPD page (0x80) to a SCSI device
  * structure. This information can be used to identify the device
  * uniquely.
  */
@@ -1053,6 +1054,7 @@ void scsi_attach_vpd(struct scsi_device *sdev)
 {
int result, i;
int vpd_len = 255;
+   int pg80_supported = 0;
int pg83_supported = 0;
unsigned char *vpd_buf;
 
@@ -1076,12 +1078,35 @@ retry_pg0:
}
 
for (i = 4; i < result; i++) {
+   if (vpd_buf[i] == 0x80) {
+   pg80_supported = 1;
+   }
if (vpd_buf[i] == 0x83) {
pg83_supported = 1;
}
}
kfree(vpd_buf);
 
+   if (pg80_supported) {
+retry_pg80:
+   vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
+   if (!vpd_buf)
+   return;
+
+   result = scsi_vpd_inquiry(sdev, vpd_buf, 0x80, vpd_len);
+   if (result < 0) {
+   kfree(vpd_buf);
+   return;
+   }
+   if (result > vpd_len) {
+   vpd_len = result;
+   kfree(vpd_buf);
+   goto retry_pg80;
+   }
+   sdev->vpd_pg80_len = result;
+   sdev->vpd_pg80 = vpd_buf;
+   }
+
if (pg83_supported) {
 retry_pg83:
vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index ca19448..03f97ab 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -784,6 +784,7 @@ show_vpd_##page(struct device *dev, struct device_attribute 
*attr, \
 static DEVICE_ATTR(vpd_##page, S_IRUGO, show_vpd_##page, NULL);
 
 sdev_vpd_pg_attr(pg83);
+sdev_vpd_pg_attr(pg80);
 
 static ssize_t
 show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
@@ -941,6 +942,10 @@ static umode_t scsi_sdev_attr_is_visible(struct kobject 
*kobj,
!sdev->vpd_pg83)
return 0;
 
+   if (attr == &dev_attr_vpd_pg80.attr &&
+   !sdev->vpd_pg80)
+   return 0;
+
return attr->mode;
 }
 
@@ -966,6 +971,7 @@ static struct attribute *scsi_sdev_attrs[] = {
&dev_attr_queue_depth.attr,
&dev_attr_queue_type.attr,
&dev_attr_queue_ramp_up_period.attr,
+   &dev_attr_vpd_pg80.attr,
&dev_attr_vpd_pg83.attr,
REF_EVT(media_change),
REF_EVT(inquiry_change_reported),
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index b99ed35..96e63c6 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -115,6 +115,8 @@ struct scsi_device {
const char * rev;   /* ... "nullnullnullnull" before scan */
unsigned char vpd_pg83_len;
unsigned char *vpd_pg83;
+   unsigned char vpd_pg80_len;
+   unsigned char *vpd_pg80;
struct scsi_target  *sdev_target;   /* used only for single_lun */
 
unsigned intsdev_bflags; /* black/white flags as also found in
-- 
1.7.12.4

--
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 2/3] Add EVPD page 0x83 to sysfs

2014-02-13 Thread Hannes Reinecke
EVPD page 0x83 is used to uniquely identify the device.
So instead of having each and every program issue a separate
SG_IO call to retrieve this information it does make far more
sense to display it in sysfs.

Cc: Jeremy Linton 
Cc: Kay Sievers 
Cc: Doug Gilbert 
Cc: Kai Makisara 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/scsi.c| 79 ++
 drivers/scsi/scsi_scan.c   |  3 ++
 drivers/scsi/scsi_sysfs.c  | 39 ++-
 include/scsi/scsi.h|  4 ++-
 include/scsi/scsi_device.h |  3 ++
 5 files changed, 119 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index d8afec8..e4cd88d 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -954,7 +954,7 @@ EXPORT_SYMBOL(scsi_track_queue_full);
  * This is an internal helper function.  You probably want to use
  * scsi_get_vpd_page instead.
  *
- * Returns 0 on success or a negative error number.
+ * Returns size of the vpg page on success or a negative error number.
  */
 static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char *buffer,
u8 page, unsigned len)
@@ -962,6 +962,9 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, 
unsigned char *buffer,
int result;
unsigned char cmd[16];
 
+   if (len < 4)
+   return -EINVAL;
+
cmd[0] = INQUIRY;
cmd[1] = 1; /* EVPD */
cmd[2] = page;
@@ -982,7 +985,7 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, 
unsigned char *buffer,
if (buffer[1] != page)
return -EIO;
 
-   return 0;
+   return (buffer[2] << 8) + buffer[3] + 4;
 }
 
 /**
@@ -1009,18 +1012,18 @@ int scsi_get_vpd_page(struct scsi_device *sdev, u8 
page, unsigned char *buf,
 
/* Ask for all the pages supported by this device */
result = scsi_vpd_inquiry(sdev, buf, 0, buf_len);
-   if (result)
+   if (result < 4)
goto fail;
 
/* If the user actually wanted this page, we can skip the rest */
if (page == 0)
return 0;
 
-   for (i = 0; i < min((int)buf[3], buf_len - 4); i++)
-   if (buf[i + 4] == page)
+   for (i = 4; i < min(result, buf_len); i++)
+   if (buf[i] == page)
goto found;
 
-   if (i < buf[3] && i >= buf_len - 4)
+   if (i < result && i >= buf_len)
/* ran off the end of the buffer, give us benefit of doubt */
goto found;
/* The device claims it doesn't support the requested page */
@@ -1028,7 +1031,7 @@ int scsi_get_vpd_page(struct scsi_device *sdev, u8 page, 
unsigned char *buf,
 
  found:
result = scsi_vpd_inquiry(sdev, buf, page, buf_len);
-   if (result)
+   if (result < 0)
goto fail;
 
return 0;
@@ -1039,6 +1042,68 @@ int scsi_get_vpd_page(struct scsi_device *sdev, u8 page, 
unsigned char *buf,
 EXPORT_SYMBOL_GPL(scsi_get_vpd_page);
 
 /**
+ * scsi_attach_vpd - Attach Vital Product Data to a SCSI device structure
+ * @sdev: The device to ask
+ *
+ * Attach the 'Device Identification' VPD page (0x83) to a SCSI device
+ * structure. This information can be used to identify the device
+ * uniquely.
+ */
+void scsi_attach_vpd(struct scsi_device *sdev)
+{
+   int result, i;
+   int vpd_len = 255;
+   int pg83_supported = 0;
+   unsigned char *vpd_buf;
+
+   if (sdev->skip_vpd_pages)
+   return;
+retry_pg0:
+   vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
+   if (!vpd_buf)
+   return;
+
+   /* Ask for all the pages supported by this device */
+   result = scsi_vpd_inquiry(sdev, vpd_buf, 0, vpd_len);
+   if (result < 0) {
+   kfree(vpd_buf);
+   return;
+   }
+   if (result > vpd_len) {
+   vpd_len = result;
+   kfree(vpd_buf);
+   goto retry_pg0;
+   }
+
+   for (i = 4; i < result; i++) {
+   if (vpd_buf[i] == 0x83) {
+   pg83_supported = 1;
+   }
+   }
+   kfree(vpd_buf);
+
+   if (pg83_supported) {
+retry_pg83:
+   vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
+   if (!vpd_buf)
+   return;
+
+   result = scsi_vpd_inquiry(sdev, vpd_buf, 0x83, vpd_len);
+   if (result < 0) {
+   kfree(vpd_buf);
+   return;
+   }
+   if (result > vpd_len) {
+   vpd_len = result;
+   kfree(vpd_buf);
+   goto retry_pg83;
+   }
+   sdev->vpd_pg83_len = result;
+   sdev->vpd_pg83 = vpd_buf;
+   }
+}
+
+/**
  * scsi_report_opcode - Find out if a given command opcode is supported
  * @sdev:  scsi device to query
  * @buffer:scratch buffer (must be at least 20 by

re: [SCSI] bnx2fc: Broadcom FCoE offload driver

2014-02-13 Thread Dan Carpenter
Hello Bhanu Gollapudi,

The patch 853e2bd2103a: "[SCSI] bnx2fc: Broadcom FCoE offload driver"
from Feb 4, 2011, leads to the following static checker warning:

drivers/scsi/bnx2fc/bnx2fc_hwi.c:1660 bnx2fc_init_mp_task()
warn: we tested 'task_type == 3' before and it was 'false'

drivers/scsi/bnx2fc/bnx2fc_hwi.c
  1586  void bnx2fc_init_mp_task(struct bnx2fc_cmd *io_req,
  1587  struct fcoe_task_ctx_entry *task)
  1588  {
  1589  struct bnx2fc_mp_req *mp_req = &(io_req->mp_req);
  1590  struct bnx2fc_rport *tgt = io_req->tgt;
  1591  struct fc_frame_header *fc_hdr;
  1592  struct fcoe_ext_mul_sges_ctx *sgl;
  1593  u8 task_type = 0;
   ^
Should this be FCOE_TASK_TYPE_UNSOLICITED (3)?

  1594  u64 *hdr;
  1595  u64 temp_hdr[3];
  1596  u32 context_id;
  1597  
  1598  
  1599  /* Obtain task_type */
  1600  if ((io_req->cmd_type == BNX2FC_TASK_MGMT_CMD) ||
  1601  (io_req->cmd_type == BNX2FC_ELS)) {
  1602  task_type = FCOE_TASK_TYPE_MIDPATH;
  1603  } else if (io_req->cmd_type == BNX2FC_ABTS) {
  1604  task_type = FCOE_TASK_TYPE_ABTS;
  1605  }
  1606  
  1607  memset(task, 0, sizeof(struct fcoe_task_ctx_entry));
  1608  
  1609  /* Setup the task from io_req for easy reference */
  1610  io_req->task = task;
  1611  
  1612  BNX2FC_IO_DBG(io_req, "Init MP task for cmd_type = %d task_type 
= %d\n",
  1613  io_req->cmd_type, task_type);
  1614  
  1615  /* Tx only */
  1616  if ((task_type == FCOE_TASK_TYPE_MIDPATH) ||
  1617  (task_type == FCOE_TASK_TYPE_UNSOLICITED)) {
  ^^
Not possible.

  1618  task->txwr_only.sgl_ctx.sgl.mul_sgl.cur_sge_addr.lo =
  1619  (u32)mp_req->mp_req_bd_dma;
  1620  task->txwr_only.sgl_ctx.sgl.mul_sgl.cur_sge_addr.hi =
  1621  (u32)((u64)mp_req->mp_req_bd_dma >> 32);
  1622  task->txwr_only.sgl_ctx.sgl.mul_sgl.sgl_size = 1;
  1623  }

regards,
dan carpenter
--
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/16] scsi_dh_alua: use local buffer for VPD inquiry

2014-02-13 Thread Maurizio Lombardi
Hi,

On 01/31/2014 10:29 AM, Hannes Reinecke wrote:
>  static int alua_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)
>  {
> + unsigned char *buff;
> + unsigned char bufflen = 36;
>   int len, timeout = ALUA_FAILOVER_TIMEOUT;
[...]
>+  len = (buff[2] << 8) + buff[3] + 4;
>+  if (len > bufflen) {
[...]
>+  bufflen = len;

just a nit: is it safe to use char as the type of bufflen? Isn't better
to declare it as int just in case len is > than 255 ?

Regards,
Maurizio Lombardi
--
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/16] scsi_dh_alua: Make stpg synchronous

2014-02-13 Thread Hannes Reinecke
On 02/12/2014 06:31 PM, Mike Christie wrote:
> On 2/12/14 10:26 AM, Mike Christie wrote:
>> On 2/12/14 10:11 AM, Mike Christie wrote:
>>> On 2/12/14 9:29 AM, Hannes Reinecke wrote:
 On 02/07/2014 02:54 AM, Mike Christie wrote:
> On 02/06/2014 07:24 PM, Mike Christie wrote:
>> On 01/31/2014 03:29 AM, Hannes Reinecke wrote:
>>> We should be issuing STPG synchronously as we need to
>>> evaluate the return code on failure.
>>>
>>> Signed-off-by: Hannes Reinecke 
>>
>> I think we need to also make dm-mpath.c use a non-ordered
>> workqueue
>> for
>> kmpath_handlerd. With this patch and the current ordered
>> workqueue in
>> dm-mpath I think we will only be able to do one STPG at a time. I
>> think
>> if we use a normal old non-ordered workqueue then we would be
>> limited to
>> have max_active STPGs executing.
>
> I goofed and commented in the order I saw the patches :) I take
> this
> comment back for this patch, because I see in 16/16 you added a
> new
> workqueue to scsi_dh_alua to do rtpgs and stpgs.
>
> For 16/16 though, do we want to make kmpath_aluad a non single
> threaded
> workqueue? It looks like max_active for single threaded is only
> one
> work
> at a time too.
>
 Well, that was by intention.

 The workqueue will be triggered very infrequently (basically for
 every path switch).
 For implicit ALUA we just need to issue a RTPG to get the new path
 status; there we might be suffering from single threaded behaviour.
 But we need to issue it only once and it should be processed
 reasonably fast.
 For explicit ALUA we'll have to send an STPG, which has potentially
 system-wide implications. So sending several to (supposedly)
 different targets might actually be contraproductive, as the first
 might have already set the status for the second call.
 Here we most definitely _want_ serialisation to avoid
 superfluous STPGs.

>>>
>>> What target is this?
>>>
>>> For our target it adds a regression. It only affects devices on
>>> the same
>>> port group. We then have multiple groups. Before the patch, we could
>>> failover/failback multiple devices in parallel. To do 64 devices
>>> it took
>>> about 3 seconds. With your patch it takes around 3 minutes.
>>>
>>
>> Also, with your pg change patch, I think we can serialize based on
>> group
>> and it will do what you want and also allow us to do STPGs to
>> different
>> groups in parallel.
> 
> I guess that would work for me, but I think if you had the same
> target port in multiple port groups then you could hit the issue you
> described, right?
> 
Yes, we might. But it's worth a shot anyway.

So to alleviate all this, this patch:

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
b/drivers/scsi/device_ha
ndler/scsi_dh_alua.c
index 569af9d..46cc1ef 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -1353,7 +1353,7 @@ static int __init alua_init(void)
 {
int r;

-   kmpath_aluad = create_singlethread_workqueue("kmpath_aluad");
+   kmpath_aluad = create_workqueue("kmpath_aluad");
if (!kmpath_aluad) {
printk(KERN_ERR "kmpath_aluad creation failed.\n");
return -EINVAL;

should be sufficient, right?

Could you test and see if it makes a difference?

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