Re: [PATCH v4] sd: Check for unaligned partial completion

[Ram Pai]

On Wed, Feb 15, 2017 at 03:48:52PM +0900, Damien Le Moal wrote:
> Christoph,
> 
> On 2/15/17 15:34, Christoph Hellwig wrote:
> > this looks reasonable, but we should ask Guilherme and Ram to confirm
> > it fixes their originally reported issue.  I've added them to Cc.
> 
> Thank you.
> 
> Guilherme, Ram,
> 
> Please test ! The original fix breaks the zoned block device support
> that was newly added to 4.10. So we need a fix for that and your issue
> combined before Linus releases 4.10 stable this weekend.

Yes logically it looks correct and should fix our issue. 

I doubt Guilherme has access to the same adapter firmware that exposed
the original problem. We will probably have to induce the erroneous
behavior in the driver to reproduce the issue and verify the fix.

Will let you know,
RP

Re: [PATCH] sg: protect access to to 'reserved' page array

[Hannes Reinecke]

On 02/14/2017 09:48 PM, Dmitry Vyukov wrote:
> On Wed, Feb 1, 2017 at 2:21 PM, Hannes Reinecke  wrote:
>> On 02/01/2017 02:12 PM, Christoph Hellwig wrote:
>>> On Wed, Feb 01, 2017 at 12:22:15PM +0100, Hannes Reinecke wrote:
 The 'reserved' page array is used as a short-cut for mapping
 data, saving us to allocate pages per request.
 However, the 'reserved' array is only capable of holding one
 request, so we need to protect it against concurrent accesses.

 Cc: sta...@vger.kernel.org
 Reported-by: Dmitry Vyukov 
 Link: http://www.spinics.net/lists/linux-scsi/msg104326.html
 Signed-off-by: Hannes Reinecke 
 Tested-by: Johannes Thumshirn 
 ---
  drivers/scsi/sg.c | 30 --
  1 file changed, 12 insertions(+), 18 deletions(-)

 diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
 index 652b934..6a8601c 100644
 --- a/drivers/scsi/sg.c
 +++ b/drivers/scsi/sg.c
 @@ -155,6 +155,8 @@
  unsigned char next_cmd_len; /* 0: automatic, >0: use on next write() 
 */
  char keep_orphan;   /* 0 -> drop orphan (def), 1 -> keep for 
 read() */
  char mmap_called;   /* 0 -> mmap() never called on this fd */
 +unsigned long flags;
 +#define SG_RESERVED_IN_USE 1
  struct kref f_ref;
  struct execute_work ew;
  } Sg_fd;
 @@ -198,7 +200,6 @@ static int sg_common_write(Sg_fd * sfp, Sg_request * 
 srp,
  static Sg_request *sg_get_rq_mark(Sg_fd * sfp, int pack_id);
  static Sg_request *sg_add_request(Sg_fd * sfp);
  static int sg_remove_request(Sg_fd * sfp, Sg_request * srp);
 -static int sg_res_in_use(Sg_fd * sfp);
  static Sg_device *sg_get_dev(int dev);
  static void sg_device_destroy(struct kref *kref);

 @@ -721,7 +722,7 @@ static int sg_allow_access(struct file *filp, unsigned 
 char *cmd)
  sg_remove_request(sfp, srp);
  return -EINVAL; /* either MMAP_IO or DIRECT_IO (not 
 both) */
  }
 -if (sg_res_in_use(sfp)) {
 +if (test_bit(SG_RESERVED_IN_USE, >flags)) {
  sg_remove_request(sfp, srp);
  return -EBUSY;  /* reserve buffer already being used 
 */
  }
 @@ -963,10 +964,14 @@ static int max_sectors_bytes(struct request_queue *q)
  val = min_t(int, val,
  max_sectors_bytes(sdp->device->request_queue));
  if (val != sfp->reserve.bufflen) {
 -if (sg_res_in_use(sfp) || sfp->mmap_called)
 +if (sfp->mmap_called)
 +return -EBUSY;
 +if (test_and_set_bit(SG_RESERVED_IN_USE, >flags))
  return -EBUSY;
 +
  sg_remove_scat(sfp, >reserve);
  sg_build_reserve(sfp, val);
 +clear_bit(SG_RESERVED_IN_USE, >flags);
>>>
>>>
>>> This seems to be abusing an atomic bitflag as a lock.
>>
>> Hmm. I wouldn't call it 'abusing'; the driver can proceed quite happily
>> without the 'reserved' buffer, so taking a lock would be overkill.
>> I could modify it to use a mutex if you insist ...
>>
>>>  And I think
>>> in general we have two different things here that this patch conflates:
>>>
>>>  a) a lock to protect building and using the reserve lists
>>>  b) a flag is a reservations is in use
>>>
>> No. This is not about reservations, this is about the internal
>> 'reserved' page buffer array.
>> (Just in case to avoid any misunderstandings).
>> We need to have an atomic / protected check in the 'sfp' structure if
>> the 'reserved' page buffer array is in use; there's an additional check
>> in the 'sg_request' structure (res_in_use) telling us which of the
>> requests is using it.
> 
> 
> So, how should we proceed here?
> We could use a mutex with only trylock, but it would be effectively the same.
> 
> There is one missed user of sg_res_in_use in "case SG_SET_FORCE_LOW_DMA".
> 
I've played around using a spinlock (can't use a mutex as one access in
done from softirq context), but always ended up in lockdep hell.

And in the end, we really only need to have a marker whether the
reserved array is in use or not. Which should be atomic, so we _could_
be using an atomic variable here.
Which would only ever have a value of '0' or '1', hence the use of a
bitfield here.
But if that fall foul of some style guide I could modify it to use an
atomic counter.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v4] sd: Check for unaligned partial completion

[Damien Le Moal]

Christoph,

On 2/15/17 15:34, Christoph Hellwig wrote:
> this looks reasonable, but we should ask Guilherme and Ram to confirm
> it fixes their originally reported issue.  I've added them to Cc.

Thank you.

Guilherme, Ram,

Please test ! The original fix breaks the zoned block device support
that was newly added to 4.10. So we need a fix for that and your issue
combined before Linus releases 4.10 stable this weekend.

Thanks.

Best regards.

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Research Group,
Western Digital Corporation
damien.lem...@wdc.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com

Re: [PATCH v4] sd: Check for unaligned partial completion

[Christoph Hellwig]

Hi Damien,

this looks reasonable, but we should ask Guilherme and Ram to confirm
it fixes their originally reported issue.  I've added them to Cc.

On Wed, Feb 15, 2017 at 11:12:30AM +0900, Damien Le Moal wrote:
> Commit "mpt3sas: Force request partial completion alignment" was not
> considering the case of REQ_TYPE_FS commands not operating on sector
> size units (e.g. REQ_OP_ZONE_REPORT and its 64B aligned partial
> replies). This could result is incorrectly retrying (forever) those
> commands.
> 
> Move the partial completion alignement check of mpt3sas to a generic
> implementation in sd_done so that the check comes after good_bytes and
> resid corrections done in that function depending on the request
> command. The check is added to the initial command switch so that
> commands with special payload size handling are ignored.
> 
> Signed-off-by: Damien Le Moal 
> 
> ---
> 
> Changes from v3:
> - Moved check to initial switch-case so that commands with special payload
>   handling are ignored.
> 
> Changes from v2:
> - Fixed good_bytes calculation after correction of unaligned resid
>   It should be good_bytes=scsi_buflen() - resid, and not good_bytes-=resid
> 
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 ---
>  drivers/scsi/sd.c| 20 
>  2 files changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 0b5b423..1961535 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -4658,7 +4658,6 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, 
> u8 msix_index, u32 reply)
>   struct MPT3SAS_DEVICE *sas_device_priv_data;
>   u32 response_code = 0;
>   unsigned long flags;
> - unsigned int sector_sz;
>  
>   mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
>   scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
> @@ -4717,20 +4716,6 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, 
> u8 msix_index, u32 reply)
>   }
>  
>   xfer_cnt = le32_to_cpu(mpi_reply->TransferCount);
> -
> - /* In case of bogus fw or device, we could end up having
> -  * unaligned partial completion. We can force alignment here,
> -  * then scsi-ml does not need to handle this misbehavior.
> -  */
> - sector_sz = scmd->device->sector_size;
> - if (unlikely(scmd->request->cmd_type == REQ_TYPE_FS && sector_sz &&
> -  xfer_cnt % sector_sz)) {
> - sdev_printk(KERN_INFO, scmd->device,
> - "unaligned partial completion avoided (xfer_cnt=%u, 
> sector_sz=%u)\n",
> - xfer_cnt, sector_sz);
> - xfer_cnt = round_down(xfer_cnt, sector_sz);
> - }
> -
>   scsi_set_resid(scmd, scsi_bufflen(scmd) - xfer_cnt);
>   if (ioc_status & MPI2_IOCSTATUS_FLAG_LOG_INFO_AVAILABLE)
>   log_info =  le32_to_cpu(mpi_reply->IOCLogInfo);
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 1f5d92a..d05a328 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1790,6 +1790,8 @@ static int sd_done(struct scsi_cmnd *SCpnt)
>  {
>   int result = SCpnt->result;
>   unsigned int good_bytes = result ? 0 : scsi_bufflen(SCpnt);
> + unsigned int sector_size = SCpnt->device->sector_size;
> + unsigned int resid;
>   struct scsi_sense_hdr sshdr;
>   struct scsi_disk *sdkp = scsi_disk(SCpnt->request->rq_disk);
>   struct request *req = SCpnt->request;
> @@ -1820,6 +1822,24 @@ static int sd_done(struct scsi_cmnd *SCpnt)
>   scsi_set_resid(SCpnt, blk_rq_bytes(req));
>   }
>   break;
> + default:
> + /*
> +  * In case of bogus fw or device, we could end up having
> +  * an unaligned partial completion. Check this here and force
> +  * alignment.
> +  */
> + resid = scsi_get_resid(SCpnt);
> + if (resid & (sector_size - 1)) {
> + SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt,
> + "Unaligned partial completion (resid=%u, 
> sector_sz=%u)\n",
> + resid, sector_size));
> + resid = round_up(resid, sector_size);
> + if (resid < good_bytes)
> + good_bytes -= resid;
> + else
> + good_bytes = 0;
> + scsi_set_resid(SCpnt, resid);
> + }
>   }
>  
>   if (result) {
> -- 
> 2.9.3
> 
> Western Digital Corporation (and its subsidiaries) E-mail Confidentiality 
> Notice & Disclaimer:
> 
> This e-mail and any files transmitted with it may contain confidential or 
> legally privileged information of WDC and/or its affiliates, and are intended 
> solely for the use of the individual or entity to which they are 

RE: [PATCH] scsi: megaraid_sas: handle dma_addr_t right on 32-bit

[Sumit Saxena]

>-Original Message-
>From: Arnd Bergmann [mailto:a...@arndb.de]
>Sent: Wednesday, February 15, 2017 2:52 AM
>To: James E.J. Bottomley; Martin K. Petersen
>Cc: Arnd Bergmann; Kashyap Desai; Sumit Saxena; Shivasharan S; Tomas
Henzl;
>Hannes Reinecke; Sasikumar Chandrasekaran;
>megaraidlinux@broadcom.com; linux-scsi@vger.kernel.org; linux-
>ker...@vger.kernel.org
>Subject: [PATCH] scsi: megaraid_sas: handle dma_addr_t right on 32-bit
>
>When building with a dma_addr_t that is different from pointer size, we
get this
>warning:
>
>drivers/scsi/megaraid/megaraid_sas_fusion.c: In function
>'megasas_make_prp_nvme':
>drivers/scsi/megaraid/megaraid_sas_fusion.c:1654:17: error: cast to
pointer
>from integer of different size [-Werror=int-to-pointer-cast]
>
>It's better to not pretend that the dma address is a pointer and instead
use a
>dma_addr_t consistently.

Patch looks good from review but we need to have some test runs before
acking this.
I will get back after some test runs.

Thanks,
Sumit
>
>Fixes: 33203bc4d61b ("scsi: megaraid_sas: NVME fast path io support")
>Signed-off-by: Arnd Bergmann 
>---
> drivers/scsi/megaraid/megaraid_sas_fusion.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c
>b/drivers/scsi/megaraid/megaraid_sas_fusion.c
>index 750090119f81..29650ba669da 100644
>--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
>+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
>@@ -1619,7 +1619,8 @@ megasas_make_prp_nvme(struct megasas_instance
>*instance, struct scsi_cmnd *scmd,  {
>   int sge_len, offset, num_prp_in_chain = 0;
>   struct MPI25_IEEE_SGE_CHAIN64 *main_chain_element, *ptr_first_sgl;
>-  u64 *ptr_sgl, *ptr_sgl_phys;
>+  u64 *ptr_sgl;
>+  dma_addr_t ptr_sgl_phys;
>   u64 sge_addr;
>   u32 page_mask, page_mask_result;
>   struct scatterlist *sg_scmd;
>@@ -1651,14 +1652,14 @@ megasas_make_prp_nvme(struct megasas_instance
>*instance, struct scsi_cmnd *scmd,
>*/
>   page_mask = mr_nvme_pg_size - 1;
>   ptr_sgl = (u64 *)cmd->sg_frame;
>-  ptr_sgl_phys = (u64 *)cmd->sg_frame_phys_addr;
>+  ptr_sgl_phys = cmd->sg_frame_phys_addr;
>   memset(ptr_sgl, 0, instance->max_chain_frame_sz);
>
>   /* Build chain frame element which holds all prps except first*/
>   main_chain_element = (struct MPI25_IEEE_SGE_CHAIN64 *)
>   ((u8 *)sgl_ptr + sizeof(struct MPI25_IEEE_SGE_CHAIN64));
>
>-  main_chain_element->Address =
cpu_to_le64((uintptr_t)ptr_sgl_phys);
>+  main_chain_element->Address = cpu_to_le64(ptr_sgl_phys);
>   main_chain_element->NextChainOffset = 0;
>   main_chain_element->Flags = IEEE_SGE_FLAGS_CHAIN_ELEMENT |
>   IEEE_SGE_FLAGS_SYSTEM_ADDR |
>@@ -1696,16 +1697,15 @@ megasas_make_prp_nvme(struct megasas_instance
>*instance, struct scsi_cmnd *scmd,
>   scmd_printk(KERN_NOTICE,
>   scmd, "page boundary ptr_sgl: 0x%p\n",
>   ptr_sgl);
>-  ptr_sgl_phys++;
>-  *ptr_sgl =
>-  cpu_to_le64((uintptr_t)ptr_sgl_phys);
>+  ptr_sgl_phys += 8;
>+  *ptr_sgl = cpu_to_le64(ptr_sgl_phys);
>   ptr_sgl++;
>   num_prp_in_chain++;
>   }
>
>   *ptr_sgl = cpu_to_le64(sge_addr);
>   ptr_sgl++;
>-  ptr_sgl_phys++;
>+  ptr_sgl_phys += 8;
>   num_prp_in_chain++;
>
>   sge_addr += mr_nvme_pg_size;
>--
>2.9.0

RE: [patch] scsi: megaraid_sas: array overflow in megasas_dump_frame()

[Sumit Saxena]

>-Original Message-
>From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
>Sent: Tuesday, February 14, 2017 10:09 PM
>To: Kashyap Desai; Shivasharan S
>Cc: Sumit Saxena; James E.J. Bottomley; Martin K. Petersen;
>megaraidlinux@broadcom.com; linux-scsi@vger.kernel.org; kernel-
>janit...@vger.kernel.org
>Subject: [patch] scsi: megaraid_sas: array overflow in
megasas_dump_frame()
>
>The "sz" variable is in terms of bytes, but we're treating the buffer as
an array of
>__le32 so we have to divide by 4.
>
>Fixes: def0eab3af86 ("scsi: megaraid_sas: enhance debug logs in OCR
context")
>Signed-off-by: Dan Carpenter 
>
>diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
>b/drivers/scsi/megaraid/megaraid_sas_base.c
>index dc9f42e135bb..7ac9a9ee9bd4 100644
>--- a/drivers/scsi/megaraid/megaraid_sas_base.c
>+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
>@@ -2754,7 +2754,7 @@ megasas_dump_frame(void *mpi_request, int sz)
>   __le32 *mfp = (__le32 *)mpi_request;
>
>   printk(KERN_INFO "IO request frame:\n\t");
>-  for (i = 0; i < sz; i++) {
>+  for (i = 0; i < sz / sizeof(__le32); i++) {
>   if (i && ((i % 8) == 0))
>   printk("\n\t");
>   printk("%08x ", le32_to_cpu(mfp[i]));

Thanks for fixing this.
Acked-by: Sumit Saxena

RE: [bug report] megaraid_sas: Make PI enabled VD 8 byte DMA aligned

[Sumit Saxena]

>-Original Message-
>From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
>Sent: Tuesday, February 14, 2017 9:54 PM
>To: sumit.sax...@avagotech.com
>Cc: megaraidlinux@broadcom.com; linux-scsi@vger.kernel.org
>Subject: [bug report] megaraid_sas: Make PI enabled VD 8 byte DMA aligned
>
>Hello sumit.sax...@avagotech.com,
>
>The patch 0b48d12d0365: "megaraid_sas: Make PI enabled VD 8 byte DMA
>aligned" from Oct 15, 2015, leads to the following static checker
>warning:
>
>   drivers/scsi/megaraid/megaraid_sas_base.c:1784
>megasas_set_dynamic_target_properties()
>   warn: if statement not indented

I will post patch to fix this indentation issue.
>
>drivers/scsi/megaraid/megaraid_sas_base.c
>  1757  void megasas_set_dynamic_target_properties(struct scsi_device
*sdev)
>  1758  {
>  1759  u16 pd_index = 0, ld;
>  1760  u32 device_id;
>  1761  struct megasas_instance *instance;
>  1762  struct fusion_context *fusion;
>  1763  struct MR_PRIV_DEVICE *mr_device_priv_data;
>  1764  struct MR_PD_CFG_SEQ_NUM_SYNC *pd_sync;
>  1765  struct MR_LD_RAID *raid;
>  1766  struct MR_DRV_RAID_MAP_ALL *local_map_ptr;
>  1767
>  1768  instance = megasas_lookup_instance(sdev->host->host_no);
>  1769  fusion = instance->ctrl_context;
>  1770  mr_device_priv_data = sdev->hostdata;
>  1771
>  1772  if (!fusion || !mr_device_priv_data)
>  1773  return;
>  1774
>  1775  if (MEGASAS_IS_LOGICAL(sdev)) {
>  1776  device_id = ((sdev->channel % 2) *
>MEGASAS_MAX_DEV_PER_CHANNEL)
>  1777  + sdev->id;
>  1778  local_map_ptr =
fusion->ld_drv_map[(instance->map_id & 1)];
>  1779  ld = MR_TargetIdToLdGet(device_id,
local_map_ptr);
>  1780  if (ld >= instance->fw_supported_vd_count)
>  1781  return;
>  1782  raid = MR_LdRaidGet(ld, local_map_ptr);
>  1783
>  1784  if (raid->capability.ldPiMode ==
>MR_PROT_INFO_TYPE_CONTROLLER)
>  1785
blk_queue_update_dma_alignment(sdev->request_queue, 0x7);
>
>
>
>  1786
>  1787  mr_device_priv_data->is_tm_capable =
>  1788  raid->capability.tmCapable;
>  1789  } else if (instance->use_seqnum_jbod_fp) {
>  1790  pd_index = (sdev->channel *
>MEGASAS_MAX_DEV_PER_CHANNEL) +
>  1791  sdev->id;
>  1792  pd_sync = (void *)fusion->pd_seq_sync
>  1793  [(instance->pd_seq_map_id - 1) &
1];
>  1794  mr_device_priv_data->is_tm_capable =
>  1795
pd_sync->seq[pd_index].capability.tmCapable;
>  1796  }
>  1797  }
>
>
>regards,
>dan carpenter

Re: [PATCH RFC] target/user: Add double ring buffers support.

[Xiubo Li]

The struct tcmu_cmd_entry {} size is fixed 44 bytes without iovec[], and
the size of struct iovec[N] is about 16 bytes * N.

The cmd entry size will be [44B, N *16 + 44B], and the data size will be
[0, N * 4096], so the ratio of sizeof(cmd entry): sizeof(entry datas) ==
(N * 16 + 44)Bytes : (N * 4096)Bytes == (N * 16)/(N * 4096) +
44/(N*4096) == 1/256 + 11/(N * 1024).
When N is bigger, the ratio will be smaller. If N >= 1, the ratio will
be [15/1024, 4/1024).

So, that's right, 512M is a little bigger than actually needed, for the
safe ratio i think 16/1024 is enough, if the data area is fixed 1G, so
the cmd area could be 16M+.
Or cmd area(64M) + data area(960M) == 1G ?

Seems like a good ratio. You could look at growing the cmd ring when
deciding to allocate more pages for the data area. But, growing the cmd
ring is a little trickier (see below) so maybe have a different policy
for deciding when & how much to grow.

Changing the cmd ring size: Unlike the data area where you can just
allocate & map more pages, I think the cmd ring you probably want to
complete all I/O, get cmd_head and cmd_tail back to the top with a PAD
op, and *then* reallocate/remap the cmd ring and update
tcmu_mailbox.cmdr_size.

Yes, that sounds nice. Growing the cmd area will be much more
complex, and this could be implemented in the future.

I will try to implement the following simple new scheme firstly:
- Fixed cmd area using the vmalloc(), total size will be 64M or 128M(?)
- Growing data area using kmalloc(DATA_BLOCK_SIZE, GFP_ATOMIC),
  the maximum total size will be 1G.

Thanks,

BRs
Xiubo


Regards -- Andy

[PATCH 13/16] aacraid: Reorder Adapter status check

[Raghava Aditya Renukunta]

The driver currently checks the SELF_TEST_FAILED first and then
KERNEL_PANIC next. Under error conditions(boot code failure) both
SELF_TEST_FAILED and KERNEL_PANIC can be set at the same time.

The driver has the capability to reset the controller on an KERNEL_PANIC
, but not on SELF_TEST_FAILED.

Fixed by first checking KERNEL_PANIC and then the others.

Cc: sta...@vger.kernel.org
Fixes: e8b12f0fb835223752 ([SCSI] aacraid: Add new code for PMC-Sierra's SRC 
base controller family)
Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/src.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index 5bb9865..0990117 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -437,16 +437,23 @@ static int aac_src_check_health(struct aac_dev *dev)
u32 status = src_readl(dev, MUnit.OMR);
 
/*
+*  Check to see if the board panic'd.
+*/
+   if (unlikely(status & KERNEL_PANIC))
+   goto err_blink;
+
+   /*
 *  Check to see if the board failed any self tests.
 */
if (unlikely(status & SELF_TEST_FAILED))
-   return -1;
+   goto err_out;
 
/*
-*  Check to see if the board panic'd.
+*  Check to see if the board failed any self tests.
 */
-   if (unlikely(status & KERNEL_PANIC))
-   return (status >> 16) & 0xFF;
+   if (unlikely(status & MONITOR_PANIC))
+   goto err_out;
+
/*
 *  Wait for the adapter to be up and running.
 */
@@ -456,6 +463,12 @@ static int aac_src_check_health(struct aac_dev *dev)
 *  Everything is OK
 */
return 0;
+
+err_out:
+   return -1;
+
+err_blink:
+   return (status > 16) & 0xFF;
 }
 
 static inline u32 aac_get_vector(struct aac_dev *dev)
-- 
2.7.4

[PATCH 12/16] aacraid: Skip IOP reset on controller panic(SMART Family)

[Raghava Aditya Renukunta]

When the SMART family of controller panic (KERNEL_PANIC) , they do not
honor IOP resets. So better to skip it and directly perform a IWBR reset.

Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/src.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index b23c818..5bb9865 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -714,6 +714,12 @@ static int aac_src_restart_adapter(struct aac_dev *dev, 
int bled, u8 reset_type)
pr_err("%s%d: adapter kernel panic'd %x.\n",
dev->name, dev->id, bled);
 
+   /*
+* WHen there is a BlinkLED, IOP_RESET has not effect
+*/
+   if (bled >= 2 && dev->sa_firmware && (reset_type & HW_IOP_RESET))
+   reset_type &= ~HW_IOP_RESET;
+
dev->a_ops.adapter_enable_int = aac_src_disable_interrupt;
 
switch (reset_type) {
-- 
2.7.4

[PATCH 11/16] aacraid: Decrease adapter health check interval

[Raghava Aditya Renukunta]

Currently driver checks the health status of the adapter once every 24
hours. When that happens the driver becomes dependent on the kernel to
figure out if the  adapter is misbehaving. This might take some time
(when the adapter is idle). The driver currently has support to
restart/recover the controller when it fails, and decreasing the time
interval will help.

Fixed by decreasing check interval from 24 hours to 1 minute

Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/aachba.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 98d4ffd..3ede50f 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -311,7 +311,7 @@ module_param(update_interval, int, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(update_interval, "Interval in seconds between time sync"
" updates issued to adapter.");
 
-int check_interval = 24 * 60 * 60;
+int check_interval = 60;
 module_param(check_interval, int, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(check_interval, "Interval in seconds between adapter health"
" checks.");
-- 
2.7.4

[PATCH 04/16] aacraid: Prevent E3 lockup when deleting units

[Raghava Aditya Renukunta]

Arrconf management utility at times sends fibs with AdapterProcessed set
in its fibs. This causes the controller to panic and lockup.

Fixed by failing the commands that have AdapterProcessed set in its flag.

Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/commsup.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 6220b47..f7a3bcb 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -522,6 +522,10 @@ int aac_fib_send(u16 command, struct fib *fibptr, unsigned 
long size,
 
if (!(hw_fib->header.XferState & cpu_to_le32(HostOwned)))
return -EBUSY;
+
+   if (hw_fib->header.XferState & cpu_to_le32(AdapterProcessed))
+   return -EINVAL;
+
/*
 *  There are 5 cases with the wait and response requested flags.
 *  The only invalid cases are if the caller requests to wait and
-- 
2.7.4

[PATCH 10/16] aacraid: Terminate kthread on controller fw assert

[Raghava Aditya Renukunta]

When the command thread performs a periodic time sync and the firmware is
going through an assert during that time, the command thread waits for the
response that would never arrive. The SCSI Mid layer's error handler would
eventually reset the controller, but the eh_handler just issues a
"thread stop" to the command thread which is stuck on a semaphore and the
eh_thread would in turn goes to sleep waiting for the command_thread to
respond to the stop which never happens.

Fixed by allowing SIGTERM for the command thread, and during the eh_reset
call, sends termination signal to the command thread. As a follow-up, the
eh_reset handler would take care of the controller reset.

Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/commsup.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 78588e4..0ee91d0 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -1519,8 +1519,15 @@ static int _aac_reset_adapter(struct aac_dev *aac, int 
forced, u8 reset_type)
scsi_block_requests(host);
aac_adapter_disable_int(aac);
if (aac->thread->pid != current->pid) {
+   struct task_struct *tsk;
+
spin_unlock_irq(host->host_lock);
+   tsk = pid_task(find_vpid(aac->thread->pid), PIDTYPE_PID);
+   if (tsk)
+   send_sig(SIGTERM, tsk, 1);
kthread_stop(aac->thread);
+
+   dev_info(>pdev->dev, "Command Thread Terminated\n");
jafo = 1;
}
 
-- 
2.7.4

[PATCH 06/16] aacraid: Added sysfs for driver version

[Raghava Aditya Renukunta]

Added support to retrieve driver version from a new sysfs variable called
driver_version. It makes it easier for the user to figure out the driver
version that is currently running.

Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/linit.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index ab4f1e7..df02784 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1131,6 +1131,13 @@ static ssize_t aac_show_bios_version(struct device 
*device,
return len;
 }
 
+static ssize_t aac_show_driver_version(struct device *device,
+   struct device_attribute *attr,
+   char *buf)
+{
+   return snprintf(buf, PAGE_SIZE, "%s\n", aac_driver_version);
+}
+
 static ssize_t aac_show_serial_number(struct device *device,
   struct device_attribute *attr, char *buf)
 {
@@ -1241,6 +1248,13 @@ static struct device_attribute aac_bios_version = {
},
.show = aac_show_bios_version,
 };
+static struct device_attribute aac_lld_version = {
+   .attr = {
+   .name = "driver_version",
+   .mode = 0444,
+   },
+   .show = aac_show_driver_version,
+};
 static struct device_attribute aac_serial_number = {
.attr = {
.name = "serial_number",
@@ -1278,6 +1292,7 @@ static struct device_attribute *aac_attrs[] = {
_kernel_version,
_monitor_version,
_bios_version,
+   _lld_version,
_serial_number,
_max_channel,
_max_id,
-- 
2.7.4

[PATCH 14/16] aacraid: Save adapter fib log before an IOP reset

[Raghava Aditya Renukunta]

Currently  the adapter firmware does not save outstanding I/O's log
information  when an IOP reset is triggered. This is problematic when
trying to root cause and debug issues.

Fixed by adding sync command to trigger I/O log file save in the adapter
firmware before issuing an IOP reset.

Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/aachba.c  |  4 
 drivers/scsi/aacraid/aacraid.h |  6 ++
 drivers/scsi/aacraid/src.c | 17 +
 3 files changed, 27 insertions(+)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 3ede50f..e3e93de 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -294,6 +294,10 @@ MODULE_PARM_DESC(aif_timeout, "The duration of time in 
seconds to wait for"
"deregistering them. This is typically adjusted for heavily burdened"
" systems.");
 
+int aac_fib_dump;
+module_param(aac_fib_dump, int, 0644);
+MODULE_PARM_DESC(aac_fib_dump, "Dump controller fibs prior to IOP_RESET 0=off, 
1=on");
+
 int numacb = -1;
 module_param(numacb, int, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(numacb, "Request a limit to the number of adapter control"
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 9281e72..622fd69 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -1444,6 +1444,10 @@ struct aac_supplement_adapter_info
 #define AAC_OPTION_VARIABLE_BLOCK_SIZE cpu_to_le32(0x0004)
 /* 240 simple volume support */
 #define AAC_OPTION_SUPPORTED_240_VOLUMES cpu_to_le32(0x1000)
+/*
+ * Supports FIB dump sync command send prior to IOP_RESET
+ */
+#define AAC_OPTION_SUPPORTED3_IOP_RESET_FIB_DUMP   cpu_to_le32(0x4000)
 #define AAC_SIS_VERSION_V3 3
 #define AAC_SIS_SLOT_UNKNOWN   0xFF
 
@@ -2483,6 +2487,7 @@ struct aac_hba_info {
 #define GET_DRIVER_BUFFER_PROPERTIES   0x0023
 #define RCV_TEMP_READINGS  0x0025
 #define GET_COMM_PREFERRED_SETTINGS0x0026
+#define IOP_RESET_FW_FIB_DUMP  0x0034
 #define IOP_RESET  0x1000
 #define IOP_RESET_ALWAYS   0x1001
 #define RE_INIT_ADAPTER0x00ee
@@ -2686,4 +2691,5 @@ extern int aac_commit;
 extern int update_interval;
 extern int check_interval;
 extern int aac_check_reset;
+extern int aac_fib_dump;
 #endif
diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index 0990117..fa03cdc 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -679,10 +679,27 @@ void aac_set_intx_mode(struct aac_dev *dev)
}
 }
 
+static void aac_dump_fw_fib_iop_reset(struct aac_dev *dev)
+{
+   __le32 supported_options3;
+
+   if (!aac_fib_dump)
+   return;
+
+   supported_options3  = dev->supplement_adapter_info.supported_options3;
+   if (!(supported_options3 & AAC_OPTION_SUPPORTED3_IOP_RESET_FIB_DUMP))
+   return;
+
+   aac_adapter_sync_cmd(dev, IOP_RESET_FW_FIB_DUMP,
+   0, 0, 0,  0, 0, 0, NULL, NULL, NULL, NULL, NULL);
+}
+
 static void aac_send_iop_reset(struct aac_dev *dev, int bled)
 {
u32 var, reset_mask;
 
+   aac_dump_fw_fib_iop_reset(dev);
+
bled = aac_adapter_sync_cmd(dev, IOP_RESET_ALWAYS,
0, 0, 0, 0, 0, 0, ,
_mask, NULL, NULL, NULL);
-- 
2.7.4

[PATCH 05/16] aacraid: Fix memory leak in fib init path

[Raghava Aditya Renukunta]

aac_fib_map_free frees misaligned fib dma memory, additionally it does not
free up the whole memory.

Fixed by changing the  code to free up the correct and full memory
allocation.

Cc: sta...@vger.kernel.org
Fixes: e8b12f0fb835223 ([SCSI] aacraid: Add new code for PMC-Sierra's SRC based 
controller family)
Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/commsup.c | 20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index f7a3bcb..863c98d 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -97,8 +97,8 @@ void aac_fib_map_free(struct aac_dev *dev)
 {
if (dev->hw_fib_va && dev->max_cmd_size) {
pci_free_consistent(dev->pdev,
-   (dev->max_cmd_size *
-   (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)),
+   (dev->max_cmd_size + sizeof(struct aac_fib_xporthdr))
+   * (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB) + 31,
dev->hw_fib_va, dev->hw_fib_pa);
}
dev->hw_fib_va = NULL;
@@ -153,22 +153,20 @@ int aac_fib_setup(struct aac_dev * dev)
if (i<0)
return -ENOMEM;
 
-   /* 32 byte alignment for PMC */
-   hw_fib_pa = (dev->hw_fib_pa + (ALIGN32 - 1)) & ~(ALIGN32 - 1);
-   dev->hw_fib_va = (struct hw_fib *)((unsigned char *)dev->hw_fib_va +
-   (hw_fib_pa - dev->hw_fib_pa));
-   dev->hw_fib_pa = hw_fib_pa;
memset(dev->hw_fib_va, 0,
(dev->max_cmd_size + sizeof(struct aac_fib_xporthdr)) *
(dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB));
 
+   /* 32 byte alignment for PMC */
+   hw_fib_pa = (dev->hw_fib_pa + (ALIGN32 - 1)) & ~(ALIGN32 - 1);
+   hw_fib= (struct hw_fib *)((unsigned char *)dev->hw_fib_va +
+   (hw_fib_pa - dev->hw_fib_pa));
+
/* add Xport header */
-   dev->hw_fib_va = (struct hw_fib *)((unsigned char *)dev->hw_fib_va +
+   hw_fib = (struct hw_fib *)((unsigned char *)hw_fib +
sizeof(struct aac_fib_xporthdr));
-   dev->hw_fib_pa += sizeof(struct aac_fib_xporthdr);
+   hw_fib_pa += sizeof(struct aac_fib_xporthdr);
 
-   hw_fib = dev->hw_fib_va;
-   hw_fib_pa = dev->hw_fib_pa;
/*
 *  Initialise the fibs
 */
-- 
2.7.4

[PATCH 09/16] aacraid: Reload offlined drives after controller reset

[Raghava Aditya Renukunta]

During the IOP reset stress testing, it was found that the drives can be
marked offline when the adapter controller crashes and IO's are running
in parallel. When the controller  does come back from the reset, the drive
that is marked offline is not exposed.

Fixed by removing and adding drives that are marked offline. In addition
invoke a scsi host bus rescan to capture any additional configuration
changes.

Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/commsup.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index de4285d..78588e4 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -1628,11 +1628,29 @@ static int _aac_reset_adapter(struct aac_dev *aac, int 
forced, u8 reset_type)
command->SCp.phase = AAC_OWNER_ERROR_HANDLER;
command->scsi_done(command);
}
+   /*
+* Any Device that was already marked offline needs to be cleaned up
+*/
+   __shost_for_each_device(dev, host) {
+   if (!scsi_device_online(dev)) {
+   sdev_printk(KERN_INFO, dev, "Removing offline 
device\n");
+   scsi_remove_device(dev);
+   scsi_device_put(dev);
+   }
+   }
retval = 0;
 
 out:
aac->in_reset = 0;
scsi_unblock_requests(host);
+   /*
+* Issue bus rescan to catch any configuration that might have
+* occurred
+*/
+   if (!retval) {
+   dev_info(>pdev->dev, "Issuing bus rescan\n");
+   scsi_scan_host(host);
+   }
if (jafo) {
spin_lock_irq(host->host_lock);
}
-- 
2.7.4

[PATCH 16/16] aacraid: Update driver version

[Raghava Aditya Renukunta]

Updated driver version to 50792

Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/aacraid.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 622fd69..d036a80 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -97,7 +97,7 @@ enum {
 #definePMC_GLOBAL_INT_BIT0 0x0001
 
 #ifndef AAC_DRIVER_BUILD
-# define AAC_DRIVER_BUILD 50740
+# define AAC_DRIVER_BUILD 50792
 # define AAC_DRIVER_BRANCH "-custom"
 #endif
 #define MAXIMUM_NUM_CONTAINERS 32
-- 
2.7.4

[PATCH 15/16] aacraid: Fix a potential spinlock double unlock bug

[Raghava Aditya Renukunta]

The driver does not unlock the reply  queue spin lock after handling SMART
adapter events. Instead it might attempt to unlock an already unlocked
spin lock.

Fixed by making sure the driver locks the spin lock before freeing it.

Thank you dan for finding this issue out.

Fixes: 6223a39fe6fbbeef (scsi: aacraid: Added support for hotplug)
Reported-by: Dan Carpenter 
Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/commsup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 0ee91d0..4141de0 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -2213,7 +2213,7 @@ static void aac_process_events(struct aac_dev *dev)
/* Thor AIF */
aac_handle_sa_aif(dev, fib);
aac_fib_adapter_complete(fib, (u16)sizeof(u32));
-   continue;
+   goto free_fib;
}
/*
 *  We will process the FIB here or pass it to a
-- 
2.7.4

[PATCH 02/16] aacraid: Use correct channel number for raw srb

[Raghava Aditya Renukunta]

The channel being used for raw srb commands is retrieved from the utility
sent fibs and is converted into physical channel id. The driver does not
need to to do this since the management utility sends the correct channel
id in the first place and in addition the driver sets inaccurate
information in the cmd sent to the firmware and gets an invalid response.

Fixed by using channel id from srb command.

Cc: sta...@vger.kernel.org
Fixes: 423400e64d377c0 ("scsi: aacraid: Include HBA direct interface")
Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/commctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 614842a..f6afd50 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -580,7 +580,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
goto cleanup;
}
 
-   chn = aac_logical_to_phys(user_srbcmd->channel);
+   chn = user_srbcmd->channel;
if (chn < AAC_MAX_BUSES && user_srbcmd->id < AAC_MAX_TARGETS &&
dev->hba_map[chn][user_srbcmd->id].devtype ==
AAC_DEVTYPE_NATIVE_RAW) {
-- 
2.7.4

[PATCH 01/16] aacraid: Fix camel case

[Raghava Aditya Renukunta]

Replaced camel case with snake case for init supported options.

Suggested-by: Johannes Thumshirn 
Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/aachba.c  | 53 ---
 drivers/scsi/aacraid/aacraid.h | 98 +-
 drivers/scsi/aacraid/commsup.c |  6 +--
 drivers/scsi/aacraid/linit.c   | 32 +++---
 drivers/scsi/aacraid/rx.c  |  2 +-
 drivers/scsi/aacraid/src.c |  2 +-
 6 files changed, 100 insertions(+), 93 deletions(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 907f1e8..98d4ffd 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -483,7 +483,7 @@ int aac_get_containers(struct aac_dev *dev)
if (status >= 0) {
dresp = (struct aac_get_container_count_resp *)fib_data(fibptr);
maximum_num_containers = 
le32_to_cpu(dresp->ContainerSwitchEntries);
-   if (fibptr->dev->supplement_adapter_info.SupportedOptions2 &
+   if (fibptr->dev->supplement_adapter_info.supported_options2 &
AAC_OPTION_SUPPORTED_240_VOLUMES) {
maximum_num_containers =
le32_to_cpu(dresp->MaxSimpleVolumes);
@@ -639,13 +639,16 @@ static void _aac_probe_container2(void * context, struct 
fib * fibptr)
fsa_dev_ptr = fibptr->dev->fsa_dev;
if (fsa_dev_ptr) {
struct aac_mount * dresp = (struct aac_mount *) 
fib_data(fibptr);
+   __le32 sup_options2;
+
fsa_dev_ptr += scmd_id(scsicmd);
+   sup_options2 =
+   fibptr->dev->supplement_adapter_info.supported_options2;
 
if ((le32_to_cpu(dresp->status) == ST_OK) &&
(le32_to_cpu(dresp->mnt[0].vol) != CT_NONE) &&
(le32_to_cpu(dresp->mnt[0].state) != FSCS_HIDDEN)) {
-   if 
(!(fibptr->dev->supplement_adapter_info.SupportedOptions2 &
-   AAC_OPTION_VARIABLE_BLOCK_SIZE)) {
+   if (!(sup_options2 & AAC_OPTION_VARIABLE_BLOCK_SIZE)) {
dresp->mnt[0].fileinfo.bdevinfo.block_size = 
0x200;
fsa_dev_ptr->block_size = 0x200;
} else {
@@ -688,7 +691,7 @@ static void _aac_probe_container1(void * context, struct 
fib * fibptr)
int status;
 
dresp = (struct aac_mount *) fib_data(fibptr);
-   if (!(fibptr->dev->supplement_adapter_info.SupportedOptions2 &
+   if (!(fibptr->dev->supplement_adapter_info.supported_options2 &
AAC_OPTION_VARIABLE_BLOCK_SIZE))
dresp->mnt[0].capacityhigh = 0;
if ((le32_to_cpu(dresp->status) != ST_OK) ||
@@ -705,7 +708,7 @@ static void _aac_probe_container1(void * context, struct 
fib * fibptr)
 
dinfo = (struct aac_query_mount *)fib_data(fibptr);
 
-   if (fibptr->dev->supplement_adapter_info.SupportedOptions2 &
+   if (fibptr->dev->supplement_adapter_info.supported_options2 &
AAC_OPTION_VARIABLE_BLOCK_SIZE)
dinfo->command = cpu_to_le32(VM_NameServeAllBlk);
else
@@ -745,7 +748,7 @@ static int _aac_probe_container(struct scsi_cmnd * scsicmd, 
int (*callback)(stru
 
dinfo = (struct aac_query_mount *)fib_data(fibptr);
 
-   if (fibptr->dev->supplement_adapter_info.SupportedOptions2 &
+   if (fibptr->dev->supplement_adapter_info.supported_options2 &
AAC_OPTION_VARIABLE_BLOCK_SIZE)
dinfo->command = cpu_to_le32(VM_NameServeAllBlk);
else
@@ -896,12 +899,14 @@ char * get_container_type(unsigned tindex)
 static void setinqstr(struct aac_dev *dev, void *data, int tindex)
 {
struct scsi_inq *str;
+   struct aac_supplement_adapter_info *sup_adap_info;
 
+   sup_adap_info = >supplement_adapter_info;
str = (struct scsi_inq *)(data); /* cast data to scsi inq block */
memset(str, ' ', sizeof(*str));
 
-   if (dev->supplement_adapter_info.AdapterTypeText[0]) {
-   char * cp = dev->supplement_adapter_info.AdapterTypeText;
+   if (sup_adap_info->adapter_type_text[0]) {
+   char *cp = sup_adap_info->adapter_type_text;
int c;
if ((cp[0] == 'A') && (cp[1] == 'O') && (cp[2] == 'C'))
inqstrcpy("SMC", str->vid);
@@ -911,8 +916,7 @@ static void setinqstr(struct aac_dev *dev, void *data, int 
tindex)
++cp;
c = *cp;
*cp = '\0';
-   inqstrcpy (dev->supplement_adapter_info.AdapterTypeText,
-  str->vid);
+   inqstrcpy(sup_adap_info->adapter_type_text, 

[PATCH 03/16] aacraid: Fix for excessive prints on EEH

[Raghava Aditya Renukunta]

This issue showed up on a kdump debug(single CPU on powerkvm), when EEH
errors rendered the adapter unusable. The driver correctly detected the
issue and attempted to restart the controller, in doing so the driver
attempted to read the status registers of the controller. This triggered
additional eeh errors which continued for a good 6 minutes.

Fixed by returning without waiting when EEH error is reported.

Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/commsup.c | 38 +-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 56090f5..6220b47 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -461,6 +461,30 @@ int aac_queue_get(struct aac_dev * dev, u32 * index, u32 
qid, struct hw_fib * hw
return 0;
 }
 
+#ifdef CONFIG_EEH
+static inline int aac_check_eeh_failure(struct aac_dev *dev)
+{
+   /* Check for an EEH failure for the given
+* device node. Function eeh_dev_check_failure()
+* returns 0 if there has not been an EEH error
+* otherwise returns a non-zero value.
+*
+* Need to be called before any PCI operation,
+* i.e.,before aac_adapter_check_health()
+*/
+   struct eeh_dev *edev = pci_dev_to_eeh_dev(dev->pdev);
+
+   if (eeh_dev_check_failure(edev)) {
+   /* The EEH mechanisms will handle this
+* error and reset the device if
+* necessary.
+*/
+   return 1;
+   }
+   return 0;
+}
+#endif
+
 /*
  * Define the highest level of host to adapter communication routines.
  * These routines will support host to adapter FS commuication. These
@@ -496,7 +520,6 @@ int aac_fib_send(u16 command, struct fib *fibptr, unsigned 
long size,
unsigned long mflags = 0;
unsigned long sflags = 0;
 
-
if (!(hw_fib->header.XferState & cpu_to_le32(HostOwned)))
return -EBUSY;
/*
@@ -662,6 +685,12 @@ int aac_fib_send(u16 command, struct fib *fibptr, unsigned 
long size,
}
return -ETIMEDOUT;
}
+
+#if defined(CONFIG_EEH)
+   if (aac_check_eeh_failure(dev))
+   return -EFAULT;
+#endif
+
if ((blink = aac_adapter_check_health(dev)) > 
0) {
if (wait == -1) {
printk(KERN_ERR "aacraid: 
aac_fib_send: adapter blinkLED 0x%x.\n"
@@ -755,7 +784,14 @@ int aac_hba_send(u8 command, struct fib *fibptr, 
fib_callback callback,
FIB_COUNTER_INCREMENT(aac_config.NativeSent);
 
if (wait) {
+
spin_unlock_irqrestore(>event_lock, flags);
+
+#if defined(CONFIG_EEH)
+   if (aac_check_eeh_failure(dev))
+   return -EFAULT;
+#endif
+
/* Only set for first known interruptable command */
if (down_interruptible(>event_wait)) {
fibptr->done = 2;
-- 
2.7.4

[PATCH 07/16] aacraid: Fix sync fibs time out on controller reset

[Raghava Aditya Renukunta]

After controller shutdown, all sync fibs time out due to not knowing
about the switch to INT-x mode

Fixed by replacing aac_src_access_devreg() to aac_set_intx_mode() call.

Cc: sta...@vger.kernel.org
Fixes: 495c021767bd78c998 (aacraid: MSI-x support)
Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/aacraid.h  | 1 +
 drivers/scsi/aacraid/comminit.c | 2 +-
 drivers/scsi/aacraid/src.c  | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index b5a2c87..9281e72 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -2639,6 +2639,7 @@ void aac_hba_callback(void *context, struct fib *fibptr);
 #define fib_data(fibctx) ((void *)(fibctx)->hw_fib_va->data)
 struct aac_dev *aac_init_adapter(struct aac_dev *dev);
 void aac_src_access_devreg(struct aac_dev *dev, int mode);
+void aac_set_intx_mode(struct aac_dev *dev);
 int aac_get_config_status(struct aac_dev *dev, int commit_flag);
 int aac_get_containers(struct aac_dev *dev);
 int aac_scsi_cmd(struct scsi_cmnd *cmd);
diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
index d0c7724..cd3456e 100644
--- a/drivers/scsi/aacraid/comminit.c
+++ b/drivers/scsi/aacraid/comminit.c
@@ -326,7 +326,7 @@ int aac_send_shutdown(struct aac_dev * dev)
 dev->pdev->device == PMC_DEVICE_S8 ||
 dev->pdev->device == PMC_DEVICE_S9) &&
 dev->msi_enabled)
-   aac_src_access_devreg(dev, AAC_ENABLE_INTX);
+   aac_set_intx_mode(dev);
return status;
 }
 
diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index c17b060..b23c818 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -657,7 +657,7 @@ static int aac_srcv_ioremap(struct aac_dev *dev, u32 size)
return 0;
 }
 
-static void aac_set_intx_mode(struct aac_dev *dev)
+void aac_set_intx_mode(struct aac_dev *dev)
 {
if (dev->msi_enabled) {
aac_src_access_devreg(dev, AAC_ENABLE_INTX);
-- 
2.7.4

[PATCH 08/16] aacraid: Skip wellness sync on controller failure

[Raghava Aditya Renukunta]

aac_command_thread checks on the health of controller periodically,
using aac_check_health. If the status is an error state KERNEL_PANIC or
anything else. The driver will attempt to restart the adapter, but the
response is not checked in aac_command_thread. This allows the periodic
sync to go thru and lead the driver to a hung state.

Fixed by terminating the periodic loop(intended per original design),
if the controller is not restored to a healthy state.

Cc: sta...@vger.kernel.org
Fixes: 3d77d8404478353358 (scsi: aacraid: Added support for periodic wellness 
sync)
Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/commsup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 863c98d..de4285d 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -2472,7 +2472,7 @@ int aac_command_thread(void *data)
 
/* Don't even try to talk to adapter if its sick */
ret = aac_check_health(dev);
-   if (!dev->queues)
+   if (ret || !dev->queues)
break;
next_check_jiffies = jiffies
   + ((long)(unsigned)check_interval)
-- 
2.7.4

[PATCH 00/16] aacraid: Fixes and enhancements for arc family

[Raghava Aditya Renukunta]

This patch set contains issue fixes, enhancements and other misc changes.

The majority of the fixes are a direct outcome of testing and work done on the
adapter reset mechanism. Initially it just had IOP reset and then was augmented
with IWBR soft hardware resets in the previous patch set. The reset mechanism
is triggered in 2 paths, one is from the eh handler from the kernel and the
other is from the driver's internal periodic health checkup.

Raghava Aditya Renukunta (16):
 [SCSI] aacraid: Fix camel case
 [SCSI] aacraid: Use correct channel number for raw srb
 [SCSI] aacraid: Fix for excessive prints on EEH
 [SCSI] aacraid: Prevent E3 lockup when deleting units
 [SCSI] aacraid: Fix memory leak in fib init path
 [SCSI] aacraid: Added sysfs for driver version
 [SCSI] aacraid: Fix sync fibs time out on controller reset
 [SCSI] aacraid: Skip wellness sync on controller failure
 [SCSI] aacraid: Reload offlined drives after controller reset
 [SCSI] aacraid: Terminate kthread on controller fw assert
 [SCSI] aacraid: Decrease adapter health check  interval
 [SCSI] aacraid: Skip IOP reset on controller panic(SMART Family)
 [SCSI] aacraid: Reorder Adapter status check
 [SCSI] aacraid: Save adapter fib log before an IOP reset
 [SCSI] aacraid: Fix a potential spinlock double unlock bug
 [SCSI] aacraid: Update driver version

 drivers/scsi/aacraid/aachba.c   |  59 --
 drivers/scsi/aacraid/aacraid.h  | 107 +---
 drivers/scsi/aacraid/commctrl.c |   2 +-
 drivers/scsi/aacraid/comminit.c |   2 +-
 drivers/scsi/aacraid/commsup.c  |  97 +---
 drivers/scsi/aacraid/linit.c|  47 --
 drivers/scsi/aacraid/rx.c   |   2 +-
 drivers/scsi/aacraid/src.c  |  48 +++---
 8 files changed, 248 insertions(+), 116 deletions(-)

-- 
2.7.4

Re: [PATCH] scsi: lpfc: Add shutdown method for kexec

[Eric W. Biederman]

Brian King  writes:

> On 02/13/2017 08:04 PM, Benjamin Herrenschmidt wrote:
>> On Mon, 2017-02-13 at 15:57 -0600, Brian King wrote:
>>> If we do transition to use remove rather than shutdown, I think we
>>> want
>>> some way for a device driver to know whether we are doing kexec or
>>> not.
>>> A RAID adapter with a write cache is going to want to flush its write
>>> cache on a PCI hotplug remove, but for a kexec, its going to want to
>>> skip
>>> that so the kexec is faster. Today, since kexec looks like a reboot,
>>> rather than a shutdown, we can skip the flush on a reboot, since its
>>> technically not needed there either.
>> 
>> What happens if a non-flushed adapter gets a PERST ?
>
> It depends on the adapter, so it really needs to be a policy in the device
> driver. For any adapter that has a non volatile write cache, data must 
> preserved,
> otherwise the write cache is not truly non volatile. For adapters with a 
> volatile
> write cache, then its subject to the adapter hardware. For ipr adapters
> that have a volatile cache, they do guarantee the data is preserved across
> a PERST.
>
>> I wouldn't trust that 'don't have to flush' magic ...
>
> I really don't think we want to force RAID adapters that have gigabytes of
> *non volatile* write cache to flush their cache when we are merely performing
> a kexec. This can take several minutes in the worst case scenarios.


It is very very simple.  kexec does not guarantee what comes after it.

An optimization that skips flushing because the hardware write cache can
be trusted is fine.  An optimization that skips flushing because some
later kernel will take care of it is not.

I really don't see any room in there for distinguishing between which
kind of shutdown and/or kexec we are doing.  Either the hardware handles
it or it doesn't.

If the hardware doesn't handle everything than we need to handle it in
software.

Eric

[PATCH v4] sd: Check for unaligned partial completion

[Damien Le Moal]

Commit "mpt3sas: Force request partial completion alignment" was not
considering the case of REQ_TYPE_FS commands not operating on sector
size units (e.g. REQ_OP_ZONE_REPORT and its 64B aligned partial
replies). This could result is incorrectly retrying (forever) those
commands.

Move the partial completion alignement check of mpt3sas to a generic
implementation in sd_done so that the check comes after good_bytes and
resid corrections done in that function depending on the request
command. The check is added to the initial command switch so that
commands with special payload size handling are ignored.

Signed-off-by: Damien Le Moal 

---

Changes from v3:
- Moved check to initial switch-case so that commands with special payload
  handling are ignored.

Changes from v2:
- Fixed good_bytes calculation after correction of unaligned resid
  It should be good_bytes=scsi_buflen() - resid, and not good_bytes-=resid

 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 ---
 drivers/scsi/sd.c| 20 
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 0b5b423..1961535 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -4658,7 +4658,6 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 
msix_index, u32 reply)
struct MPT3SAS_DEVICE *sas_device_priv_data;
u32 response_code = 0;
unsigned long flags;
-   unsigned int sector_sz;
 
mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
@@ -4717,20 +4716,6 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 
msix_index, u32 reply)
}
 
xfer_cnt = le32_to_cpu(mpi_reply->TransferCount);
-
-   /* In case of bogus fw or device, we could end up having
-* unaligned partial completion. We can force alignment here,
-* then scsi-ml does not need to handle this misbehavior.
-*/
-   sector_sz = scmd->device->sector_size;
-   if (unlikely(scmd->request->cmd_type == REQ_TYPE_FS && sector_sz &&
-xfer_cnt % sector_sz)) {
-   sdev_printk(KERN_INFO, scmd->device,
-   "unaligned partial completion avoided (xfer_cnt=%u, 
sector_sz=%u)\n",
-   xfer_cnt, sector_sz);
-   xfer_cnt = round_down(xfer_cnt, sector_sz);
-   }
-
scsi_set_resid(scmd, scsi_bufflen(scmd) - xfer_cnt);
if (ioc_status & MPI2_IOCSTATUS_FLAG_LOG_INFO_AVAILABLE)
log_info =  le32_to_cpu(mpi_reply->IOCLogInfo);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 1f5d92a..d05a328 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1790,6 +1790,8 @@ static int sd_done(struct scsi_cmnd *SCpnt)
 {
int result = SCpnt->result;
unsigned int good_bytes = result ? 0 : scsi_bufflen(SCpnt);
+   unsigned int sector_size = SCpnt->device->sector_size;
+   unsigned int resid;
struct scsi_sense_hdr sshdr;
struct scsi_disk *sdkp = scsi_disk(SCpnt->request->rq_disk);
struct request *req = SCpnt->request;
@@ -1820,6 +1822,24 @@ static int sd_done(struct scsi_cmnd *SCpnt)
scsi_set_resid(SCpnt, blk_rq_bytes(req));
}
break;
+   default:
+   /*
+* In case of bogus fw or device, we could end up having
+* an unaligned partial completion. Check this here and force
+* alignment.
+*/
+   resid = scsi_get_resid(SCpnt);
+   if (resid & (sector_size - 1)) {
+   SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt,
+   "Unaligned partial completion (resid=%u, 
sector_sz=%u)\n",
+   resid, sector_size));
+   resid = round_up(resid, sector_size);
+   if (resid < good_bytes)
+   good_bytes -= resid;
+   else
+   good_bytes = 0;
+   scsi_set_resid(SCpnt, resid);
+   }
}
 
if (result) {
-- 
2.9.3

Re: [PATCH RFC] target/user: Add double ring buffers support.

[Andy Grover]

On 02/13/2017 09:50 PM, Xiubo Li wrote:
> The struct tcmu_cmd_entry {} size is fixed 44 bytes without iovec[], and
> the size of struct iovec[N] is about 16 bytes * N.
> 
> The cmd entry size will be [44B, N *16 + 44B], and the data size will be
> [0, N * 4096], so the ratio of sizeof(cmd entry): sizeof(entry datas) ==
> (N * 16 + 44)Bytes : (N * 4096)Bytes == (N * 16)/(N * 4096) +
> 44/(N*4096) == 1/256 + 11/(N * 1024).
> When N is bigger, the ratio will be smaller. If N >= 1, the ratio will
> be [15/1024, 4/1024).
> 
> So, that's right, 512M is a little bigger than actually needed, for the
> safe ratio i think 16/1024 is enough, if the data area is fixed 1G, so
> the cmd area could be 16M+.
> Or cmd area(64M) + data area(960M) == 1G ?

Seems like a good ratio. You could look at growing the cmd ring when
deciding to allocate more pages for the data area. But, growing the cmd
ring is a little trickier (see below) so maybe have a different policy
for deciding when & how much to grow.

Changing the cmd ring size: Unlike the data area where you can just
allocate & map more pages, I think the cmd ring you probably want to
complete all I/O, get cmd_head and cmd_tail back to the top with a PAD
op, and *then* reallocate/remap the cmd ring and update
tcmu_mailbox.cmdr_size.

Regards -- Andy

[PATCH] scsi: megaraid_sas: handle dma_addr_t right on 32-bit

[Arnd Bergmann]

When building with a dma_addr_t that is different from pointer size, we get
this warning:

drivers/scsi/megaraid/megaraid_sas_fusion.c: In function 
'megasas_make_prp_nvme':
drivers/scsi/megaraid/megaraid_sas_fusion.c:1654:17: error: cast to pointer 
from integer of different size [-Werror=int-to-pointer-cast]

It's better to not pretend that the dma address is a pointer and instead
use a dma_addr_t consistently.

Fixes: 33203bc4d61b ("scsi: megaraid_sas: NVME fast path io support")
Signed-off-by: Arnd Bergmann 
---
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c 
b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 750090119f81..29650ba669da 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -1619,7 +1619,8 @@ megasas_make_prp_nvme(struct megasas_instance *instance, 
struct scsi_cmnd *scmd,
 {
int sge_len, offset, num_prp_in_chain = 0;
struct MPI25_IEEE_SGE_CHAIN64 *main_chain_element, *ptr_first_sgl;
-   u64 *ptr_sgl, *ptr_sgl_phys;
+   u64 *ptr_sgl;
+   dma_addr_t ptr_sgl_phys;
u64 sge_addr;
u32 page_mask, page_mask_result;
struct scatterlist *sg_scmd;
@@ -1651,14 +1652,14 @@ megasas_make_prp_nvme(struct megasas_instance 
*instance, struct scsi_cmnd *scmd,
 */
page_mask = mr_nvme_pg_size - 1;
ptr_sgl = (u64 *)cmd->sg_frame;
-   ptr_sgl_phys = (u64 *)cmd->sg_frame_phys_addr;
+   ptr_sgl_phys = cmd->sg_frame_phys_addr;
memset(ptr_sgl, 0, instance->max_chain_frame_sz);
 
/* Build chain frame element which holds all prps except first*/
main_chain_element = (struct MPI25_IEEE_SGE_CHAIN64 *)
((u8 *)sgl_ptr + sizeof(struct MPI25_IEEE_SGE_CHAIN64));
 
-   main_chain_element->Address = cpu_to_le64((uintptr_t)ptr_sgl_phys);
+   main_chain_element->Address = cpu_to_le64(ptr_sgl_phys);
main_chain_element->NextChainOffset = 0;
main_chain_element->Flags = IEEE_SGE_FLAGS_CHAIN_ELEMENT |
IEEE_SGE_FLAGS_SYSTEM_ADDR |
@@ -1696,16 +1697,15 @@ megasas_make_prp_nvme(struct megasas_instance 
*instance, struct scsi_cmnd *scmd,
scmd_printk(KERN_NOTICE,
scmd, "page boundary ptr_sgl: 0x%p\n",
ptr_sgl);
-   ptr_sgl_phys++;
-   *ptr_sgl =
-   cpu_to_le64((uintptr_t)ptr_sgl_phys);
+   ptr_sgl_phys += 8;
+   *ptr_sgl = cpu_to_le64(ptr_sgl_phys);
ptr_sgl++;
num_prp_in_chain++;
}
 
*ptr_sgl = cpu_to_le64(sge_addr);
ptr_sgl++;
-   ptr_sgl_phys++;
+   ptr_sgl_phys += 8;
num_prp_in_chain++;
 
sge_addr += mr_nvme_pg_size;
-- 
2.9.0

Re: [PATCH] sg: protect access to to 'reserved' page array

[Dmitry Vyukov]

On Wed, Feb 1, 2017 at 2:21 PM, Hannes Reinecke  wrote:
> On 02/01/2017 02:12 PM, Christoph Hellwig wrote:
>> On Wed, Feb 01, 2017 at 12:22:15PM +0100, Hannes Reinecke wrote:
>>> The 'reserved' page array is used as a short-cut for mapping
>>> data, saving us to allocate pages per request.
>>> However, the 'reserved' array is only capable of holding one
>>> request, so we need to protect it against concurrent accesses.
>>>
>>> Cc: sta...@vger.kernel.org
>>> Reported-by: Dmitry Vyukov 
>>> Link: http://www.spinics.net/lists/linux-scsi/msg104326.html
>>> Signed-off-by: Hannes Reinecke 
>>> Tested-by: Johannes Thumshirn 
>>> ---
>>>  drivers/scsi/sg.c | 30 --
>>>  1 file changed, 12 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
>>> index 652b934..6a8601c 100644
>>> --- a/drivers/scsi/sg.c
>>> +++ b/drivers/scsi/sg.c
>>> @@ -155,6 +155,8 @@
>>>  unsigned char next_cmd_len; /* 0: automatic, >0: use on next write() */
>>>  char keep_orphan;   /* 0 -> drop orphan (def), 1 -> keep for 
>>> read() */
>>>  char mmap_called;   /* 0 -> mmap() never called on this fd */
>>> +unsigned long flags;
>>> +#define SG_RESERVED_IN_USE 1
>>>  struct kref f_ref;
>>>  struct execute_work ew;
>>>  } Sg_fd;
>>> @@ -198,7 +200,6 @@ static int sg_common_write(Sg_fd * sfp, Sg_request * 
>>> srp,
>>>  static Sg_request *sg_get_rq_mark(Sg_fd * sfp, int pack_id);
>>>  static Sg_request *sg_add_request(Sg_fd * sfp);
>>>  static int sg_remove_request(Sg_fd * sfp, Sg_request * srp);
>>> -static int sg_res_in_use(Sg_fd * sfp);
>>>  static Sg_device *sg_get_dev(int dev);
>>>  static void sg_device_destroy(struct kref *kref);
>>>
>>> @@ -721,7 +722,7 @@ static int sg_allow_access(struct file *filp, unsigned 
>>> char *cmd)
>>>  sg_remove_request(sfp, srp);
>>>  return -EINVAL; /* either MMAP_IO or DIRECT_IO (not 
>>> both) */
>>>  }
>>> -if (sg_res_in_use(sfp)) {
>>> +if (test_bit(SG_RESERVED_IN_USE, >flags)) {
>>>  sg_remove_request(sfp, srp);
>>>  return -EBUSY;  /* reserve buffer already being used */
>>>  }
>>> @@ -963,10 +964,14 @@ static int max_sectors_bytes(struct request_queue *q)
>>>  val = min_t(int, val,
>>>  max_sectors_bytes(sdp->device->request_queue));
>>>  if (val != sfp->reserve.bufflen) {
>>> -if (sg_res_in_use(sfp) || sfp->mmap_called)
>>> +if (sfp->mmap_called)
>>> +return -EBUSY;
>>> +if (test_and_set_bit(SG_RESERVED_IN_USE, >flags))
>>>  return -EBUSY;
>>> +
>>>  sg_remove_scat(sfp, >reserve);
>>>  sg_build_reserve(sfp, val);
>>> +clear_bit(SG_RESERVED_IN_USE, >flags);
>>
>>
>> This seems to be abusing an atomic bitflag as a lock.
>
> Hmm. I wouldn't call it 'abusing'; the driver can proceed quite happily
> without the 'reserved' buffer, so taking a lock would be overkill.
> I could modify it to use a mutex if you insist ...
>
>>  And I think
>> in general we have two different things here that this patch conflates:
>>
>>  a) a lock to protect building and using the reserve lists
>>  b) a flag is a reservations is in use
>>
> No. This is not about reservations, this is about the internal
> 'reserved' page buffer array.
> (Just in case to avoid any misunderstandings).
> We need to have an atomic / protected check in the 'sfp' structure if
> the 'reserved' page buffer array is in use; there's an additional check
> in the 'sg_request' structure (res_in_use) telling us which of the
> requests is using it.


So, how should we proceed here?
We could use a mutex with only trylock, but it would be effectively the same.

There is one missed user of sg_res_in_use in "case SG_SET_FORCE_LOW_DMA".

Re: [PATCH] snic: switch to pci_irq_alloc_vectors

[Narsimhulu Musini (nmusini)]

Hi Martin,
Thanks for the changes. Looks good. Verified.
Acked-by: Narsimhulu Musini 



Thanks
simha
On 07/02/17 5:47 am, "Martin K. Petersen" 
wrote:

>> "Christoph" == Christoph Hellwig  writes:
>
>Narsimhulu,
>
>Please test and review!
>
>   https://patchwork.kernel.org/patch/9549801/
>
>-- 
>Martin K. Petersen Oracle Linux Engineering

[PATCH 4/6] scsi: simplify scsi_execute_req_flags

[Christoph Hellwig]

Add a sshdr argument to __scsi_execute so that we can decode the sense
data directly into the sense header instead of needing a copy of it.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/scsi_lib.c | 27 +--
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 227a77869e13..35b43a8f1bfa 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -215,8 +215,9 @@ void scsi_queue_insert(struct scsi_cmnd *cmd, int reason)
 
 static int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 int data_direction, void *buffer, unsigned bufflen,
-unsigned char *sense, int timeout, int retries, u64 flags,
-req_flags_t rq_flags, int *resid)
+unsigned char *sense, struct scsi_sense_hdr *sshdr,
+int timeout, int retries, u64 flags, req_flags_t rq_flags,
+int *resid)
 {
struct request *req;
struct scsi_request *rq;
@@ -259,6 +260,8 @@ static int __scsi_execute(struct scsi_device *sdev, const 
unsigned char *cmd,
*resid = rq->resid_len;
if (sense && rq->sense_len)
memcpy(sense, rq->sense, SCSI_SENSE_BUFFERSIZE);
+   if (sshdr)
+   scsi_normalize_sense(rq->sense, rq->sense_len, sshdr);
ret = req->errors;
  out:
blk_put_request(req);
@@ -288,7 +291,7 @@ int scsi_execute(struct scsi_device *sdev, const unsigned 
char *cmd,
 int *resid)
 {
return __scsi_execute(sdev, cmd, data_direction, buffer, bufflen, sense,
-   timeout, retries, flags, 0, resid);
+   NULL, timeout, retries, flags, 0, resid);
 }
 EXPORT_SYMBOL(scsi_execute);
 
@@ -297,21 +300,9 @@ int scsi_execute_req_flags(struct scsi_device *sdev, const 
unsigned char *cmd,
 struct scsi_sense_hdr *sshdr, int timeout, int retries,
 int *resid, u64 flags, req_flags_t rq_flags)
 {
-   char *sense = NULL;
-   int result;
-   
-   if (sshdr) {
-   sense = kzalloc(SCSI_SENSE_BUFFERSIZE, GFP_NOIO);
-   if (!sense)
-   return DRIVER_ERROR << 24;
-   }
-   result = __scsi_execute(sdev, cmd, data_direction, buffer, bufflen,
- sense, timeout, retries, flags, rq_flags, resid);
-   if (sshdr)
-   scsi_normalize_sense(sense, SCSI_SENSE_BUFFERSIZE, sshdr);
-
-   kfree(sense);
-   return result;
+   return __scsi_execute(sdev, cmd, data_direction, buffer, bufflen,
+ NULL, sshdr, timeout, retries, flags, rq_flags,
+ resid);
 }
 EXPORT_SYMBOL(scsi_execute_req_flags);
 
-- 
2.11.0

[PATCH 5/6] scsi: merge __scsi_execute into scsi_execute

[Christoph Hellwig]

All but one caller want the decoded sense header, so offer the existing
__scsi_execute helper as the public scsi_execute API to simply the
callers.

Signed-off-by: Christoph Hellwig 
---
 drivers/ata/libata-scsi.c | 12 --
 drivers/scsi/cxlflash/superpipe.c |  8 +++
 drivers/scsi/cxlflash/vlun.c  |  4 ++--
 drivers/scsi/scsi_lib.c   | 48 +--
 drivers/scsi/scsi_transport_spi.c | 24 
 drivers/scsi/sr_ioctl.c   | 19 +++-
 include/scsi/scsi_device.h|  5 ++--
 7 files changed, 46 insertions(+), 74 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index c771d4c341ea..0b1e6762c6f9 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -607,6 +607,7 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user 
*arg)
u8 args[4], *argbuf = NULL, *sensebuf = NULL;
int argsize = 0;
enum dma_data_direction data_dir;
+   struct scsi_sense_hdr sshdr;
int cmd_result;
 
if (arg == NULL)
@@ -655,7 +656,7 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user 
*arg)
/* Good values for timeout and retries?  Values below
   from scsi_ioctl_send_command() for default case... */
cmd_result = scsi_execute(scsidev, scsi_cmd, data_dir, argbuf, argsize,
- sensebuf, (10*HZ), 5, 0, NULL);
+ sensebuf, , (10*HZ), 5, 0, 0, NULL);
 
if (driver_byte(cmd_result) == DRIVER_SENSE) {/* sense data available */
u8 *desc = sensebuf + 8;
@@ -664,9 +665,6 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user 
*arg)
/* If we set cc then ATA pass-through will cause a
 * check condition even if no error. Filter that. */
if (cmd_result & SAM_STAT_CHECK_CONDITION) {
-   struct scsi_sense_hdr sshdr;
-   scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
-);
if (sshdr.sense_key == RECOVERED_ERROR &&
sshdr.asc == 0 && sshdr.ascq == 0x1d)
cmd_result &= ~SAM_STAT_CHECK_CONDITION;
@@ -714,6 +712,7 @@ int ata_task_ioctl(struct scsi_device *scsidev, void __user 
*arg)
int rc = 0;
u8 scsi_cmd[MAX_COMMAND_SIZE];
u8 args[7], *sensebuf = NULL;
+   struct scsi_sense_hdr sshdr;
int cmd_result;
 
if (arg == NULL)
@@ -741,7 +740,7 @@ int ata_task_ioctl(struct scsi_device *scsidev, void __user 
*arg)
/* Good values for timeout and retries?  Values below
   from scsi_ioctl_send_command() for default case... */
cmd_result = scsi_execute(scsidev, scsi_cmd, DMA_NONE, NULL, 0,
-   sensebuf, (10*HZ), 5, 0, NULL);
+   sensebuf, , (10*HZ), 5, 0, 0, NULL);
 
if (driver_byte(cmd_result) == DRIVER_SENSE) {/* sense data available */
u8 *desc = sensebuf + 8;
@@ -750,9 +749,6 @@ int ata_task_ioctl(struct scsi_device *scsidev, void __user 
*arg)
/* If we set cc then ATA pass-through will cause a
 * check condition even if no error. Filter that. */
if (cmd_result & SAM_STAT_CHECK_CONDITION) {
-   struct scsi_sense_hdr sshdr;
-   scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
-   );
if (sshdr.sense_key == RECOVERED_ERROR &&
sshdr.asc == 0 && sshdr.ascq == 0x1d)
cmd_result &= ~SAM_STAT_CHECK_CONDITION;
diff --git a/drivers/scsi/cxlflash/superpipe.c 
b/drivers/scsi/cxlflash/superpipe.c
index 9636970d9611..005a377a427f 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -305,6 +305,7 @@ static int read_cap16(struct scsi_device *sdev, struct 
llun_info *lli)
struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)sdev->host->hostdata;
struct device *dev = >dev->dev;
struct glun_info *gli = lli->parent;
+   struct scsi_sense_hdr sshdr;
u8 *cmd_buf = NULL;
u8 *scsi_cmd = NULL;
u8 *sense_buf = NULL;
@@ -332,7 +333,8 @@ static int read_cap16(struct scsi_device *sdev, struct 
llun_info *lli)
/* Drop the ioctl read semahpore across lengthy call */
up_read(>ioctl_rwsem);
result = scsi_execute(sdev, scsi_cmd, DMA_FROM_DEVICE, cmd_buf,
- CMD_BUFSIZE, sense_buf, to, CMD_RETRIES, 0, NULL);
+ CMD_BUFSIZE, sense_buf, , to, CMD_RETRIES,
+ 0, 0, NULL);
down_read(>ioctl_rwsem);
rc = check_state(cfg);
if (rc) {
@@ -345,10 +347,6 @@ static int read_cap16(struct scsi_device 

[PATCH 2/6] sd: improve TUR handling in sd_check_events

[Christoph Hellwig]

Remove bogus evaluations of retval and sshdr when the device is offline,
and fix a possible NULL pointer dereference by allocating the 8 byte
sized sense header on stack.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/sd.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 40b4038c019e..11e290d1dbff 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1425,7 +1425,6 @@ static unsigned int sd_check_events(struct gendisk *disk, 
unsigned int clearing)
 {
struct scsi_disk *sdkp = scsi_disk_get(disk);
struct scsi_device *sdp;
-   struct scsi_sense_hdr *sshdr = NULL;
int retval;
 
if (!sdkp)
@@ -1454,22 +1453,21 @@ static unsigned int sd_check_events(struct gendisk 
*disk, unsigned int clearing)
 * by sd_spinup_disk() from sd_revalidate_disk(), which happens whenever
 * sd_revalidate() is called.
 */
-   retval = -ENODEV;
-
if (scsi_block_when_processing_errors(sdp)) {
-   sshdr  = kzalloc(sizeof(*sshdr), GFP_KERNEL);
+   struct scsi_sense_hdr sshdr = { 0, };
+
retval = scsi_test_unit_ready(sdp, SD_TIMEOUT, SD_MAX_RETRIES,
- sshdr);
-   }
+ );
 
-   /* failed to execute TUR, assume media not present */
-   if (host_byte(retval)) {
-   set_media_not_present(sdkp);
-   goto out;
-   }
+   /* failed to execute TUR, assume media not present */
+   if (host_byte(retval)) {
+   set_media_not_present(sdkp);
+   goto out;
+   }
 
-   if (media_not_present(sdkp, sshdr))
-   goto out;
+   if (media_not_present(sdkp, ))
+   goto out;
+   }
 
/*
 * For removable scsi disk we have to recognise the presence
@@ -1485,7 +1483,6 @@ static unsigned int sd_check_events(struct gendisk *disk, 
unsigned int clearing)
 *  Medium present state has changed in either direction.
 *  Device has indicated UNIT_ATTENTION.
 */
-   kfree(sshdr);
retval = sdp->changed ? DISK_EVENT_MEDIA_CHANGE : 0;
sdp->changed = 0;
scsi_disk_put(sdkp);
-- 
2.11.0

sense handling improvements

[Christoph Hellwig]

Hi all,

this series is on top of the scsi_request changes in Jens' tree and
further improves the handling of the sense buffer.

The first patch prevents any possibily of reusing stale sense codes
in sense headers, and is a bug fix that we should probably get into
the block tree ASAP.

The rest cleans up handling of the parsed sense data and could go in
either through the block tree, or a SCSI branch on top of the block
tree.

[PATCH 3/6] scsi: make the sense header argument to scsi_test_unit_ready mandatory

[Christoph Hellwig]

It's a tiny structure that can be allocated on the stack, don't
complicate the code by making it optional.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/osd/osd_uld.c |  3 ++-
 drivers/scsi/scsi_ioctl.c  |  3 ++-
 drivers/scsi/scsi_lib.c| 14 ++
 3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 243eab3d10d0..e0ce5d2fd14d 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -372,6 +372,7 @@ EXPORT_SYMBOL(osduld_device_same);
 static int __detect_osd(struct osd_uld_device *oud)
 {
struct scsi_device *scsi_device = oud->od.scsi_device;
+   struct scsi_sense_hdr sense_hdr;
char caps[OSD_CAP_LEN];
int error;
 
@@ -380,7 +381,7 @@ static int __detect_osd(struct osd_uld_device *oud)
 */
OSD_DEBUG("start scsi_test_unit_ready %p %p %p\n",
oud, scsi_device, scsi_device->request_queue);
-   error = scsi_test_unit_ready(scsi_device, 10*HZ, 5, NULL);
+   error = scsi_test_unit_ready(scsi_device, 10*HZ, 5, _hdr);
if (error)
OSD_ERR("warning: scsi_test_unit_ready failed\n");
 
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index 8b8c814df5c7..b6bf3f29a12a 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -199,6 +199,7 @@ static int scsi_ioctl_get_pci(struct scsi_device *sdev, 
void __user *arg)
 int scsi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
 {
char scsi_cmd[MAX_COMMAND_SIZE];
+   struct scsi_sense_hdr sense_hdr;
 
/* Check for deprecated ioctls ... all the ioctls which don't
 * follow the new unique numbering scheme are deprecated */
@@ -243,7 +244,7 @@ int scsi_ioctl(struct scsi_device *sdev, int cmd, void 
__user *arg)
return scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW);
case SCSI_IOCTL_TEST_UNIT_READY:
return scsi_test_unit_ready(sdev, IOCTL_NORMAL_TIMEOUT,
-   NORMAL_RETRIES, NULL);
+   NORMAL_RETRIES, _hdr);
case SCSI_IOCTL_START_UNIT:
scsi_cmd[0] = START_STOP;
scsi_cmd[1] = 0;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 90f65c8f487a..227a77869e13 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2496,28 +2496,20 @@ EXPORT_SYMBOL(scsi_mode_sense);
  * @sdev:  scsi device to change the state of.
  * @timeout: command timeout
  * @retries: number of retries before failing
- * @sshdr_external: Optional pointer to struct scsi_sense_hdr for
- * returning sense. Make sure that this is cleared before passing
- * in.
+ * @sshdr: outpout pointer for decoded sense information.
  *
  * Returns zero if unsuccessful or an error if TUR failed.  For
  * removable media, UNIT_ATTENTION sets ->changed flag.
  **/
 int
 scsi_test_unit_ready(struct scsi_device *sdev, int timeout, int retries,
-struct scsi_sense_hdr *sshdr_external)
+struct scsi_sense_hdr *sshdr)
 {
char cmd[] = {
TEST_UNIT_READY, 0, 0, 0, 0, 0,
};
-   struct scsi_sense_hdr *sshdr;
int result;
 
-   if (!sshdr_external)
-   sshdr = kzalloc(sizeof(*sshdr), GFP_KERNEL);
-   else
-   sshdr = sshdr_external;
-
/* try to eat the UNIT_ATTENTION if there are enough retries */
do {
result = scsi_execute_req(sdev, cmd, DMA_NONE, NULL, 0, sshdr,
@@ -2528,8 +2520,6 @@ scsi_test_unit_ready(struct scsi_device *sdev, int 
timeout, int retries,
} while (scsi_sense_valid(sshdr) &&
 sshdr->sense_key == UNIT_ATTENTION && --retries);
 
-   if (!sshdr_external)
-   kfree(sshdr);
return result;
 }
 EXPORT_SYMBOL(scsi_test_unit_ready);
-- 
2.11.0

[PATCH 1/6] scsi: always zero sshdr in scsi_normalize_sense

[Christoph Hellwig]

This gives us a clear state even if a command didn't return sense data.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/scsi_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_common.c b/drivers/scsi/scsi_common.c
index b1383a71400e..a75673bb82b3 100644
--- a/drivers/scsi/scsi_common.c
+++ b/drivers/scsi/scsi_common.c
@@ -137,11 +137,11 @@ EXPORT_SYMBOL(int_to_scsilun);
 bool scsi_normalize_sense(const u8 *sense_buffer, int sb_len,
  struct scsi_sense_hdr *sshdr)
 {
+   memset(sshdr, 0, sizeof(struct scsi_sense_hdr));
+
if (!sense_buffer || !sb_len)
return false;
 
-   memset(sshdr, 0, sizeof(struct scsi_sense_hdr));
-
sshdr->response_code = (sense_buffer[0] & 0x7f);
 
if (!scsi_sense_valid(sshdr))
-- 
2.11.0

[PATCH 6/6] scsi: remove scsi_execute_req_flags

[Christoph Hellwig]

And switch all callers to use scsi_execute instead.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/device_handler/scsi_dh_alua.c  | 16 ++--
 drivers/scsi/device_handler/scsi_dh_emc.c   |  7 +++
 drivers/scsi/device_handler/scsi_dh_hp_sw.c | 10 --
 drivers/scsi/device_handler/scsi_dh_rdac.c  |  7 +++
 drivers/scsi/scsi_lib.c | 11 ---
 drivers/scsi/sd.c   |  9 -
 drivers/scsi/ufs/ufshcd.c   | 10 +-
 include/scsi/scsi_device.h  |  8 ++--
 8 files changed, 27 insertions(+), 51 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
b/drivers/scsi/device_handler/scsi_dh_alua.c
index d704752b6332..48e200102221 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -151,11 +151,9 @@ static int submit_rtpg(struct scsi_device *sdev, unsigned 
char *buff,
cdb[1] = MI_REPORT_TARGET_PGS;
put_unaligned_be32(bufflen, [6]);
 
-   return scsi_execute_req_flags(sdev, cdb, DMA_FROM_DEVICE,
- buff, bufflen, sshdr,
- ALUA_FAILOVER_TIMEOUT * HZ,
- ALUA_FAILOVER_RETRIES, NULL,
- req_flags, 0);
+   return scsi_execute(sdev, cdb, DMA_FROM_DEVICE, buff, bufflen, NULL,
+   sshdr, ALUA_FAILOVER_TIMEOUT * HZ,
+   ALUA_FAILOVER_RETRIES, req_flags, 0, NULL);
 }
 
 /*
@@ -185,11 +183,9 @@ static int submit_stpg(struct scsi_device *sdev, int 
group_id,
cdb[1] = MO_SET_TARGET_PGS;
put_unaligned_be32(stpg_len, [6]);
 
-   return scsi_execute_req_flags(sdev, cdb, DMA_TO_DEVICE,
- stpg_data, stpg_len,
- sshdr, ALUA_FAILOVER_TIMEOUT * HZ,
- ALUA_FAILOVER_RETRIES, NULL,
- req_flags, 0);
+   return scsi_execute(sdev, cdb, DMA_TO_DEVICE, stpg_data, stpg_len, NULL,
+   sshdr, ALUA_FAILOVER_TIMEOUT * HZ,
+   ALUA_FAILOVER_RETRIES, req_flags, 0, NULL);
 }
 
 static struct alua_port_group *alua_find_get_pg(char *id_str, size_t id_size,
diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c 
b/drivers/scsi/device_handler/scsi_dh_emc.c
index 4a7679f6c73d..3997f21794df 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -276,10 +276,9 @@ static int send_trespass_cmd(struct scsi_device *sdev,
BUG_ON((len > CLARIION_BUFFER_SIZE));
memcpy(csdev->buffer, page22, len);
 
-   err = scsi_execute_req_flags(sdev, cdb, DMA_TO_DEVICE,
-csdev->buffer, len, ,
-CLARIION_TIMEOUT * HZ, CLARIION_RETRIES,
-NULL, req_flags, 0);
+   err = scsi_execute(sdev, cdb, DMA_TO_DEVICE, csdev->buffer, len, NULL,
+   , CLARIION_TIMEOUT * HZ, CLARIION_RETRIES,
+   req_flags, 0, NULL);
if (err) {
if (scsi_sense_valid())
res = trespass_endio(sdev, );
diff --git a/drivers/scsi/device_handler/scsi_dh_hp_sw.c 
b/drivers/scsi/device_handler/scsi_dh_hp_sw.c
index be43c940636d..62d314e07d11 100644
--- a/drivers/scsi/device_handler/scsi_dh_hp_sw.c
+++ b/drivers/scsi/device_handler/scsi_dh_hp_sw.c
@@ -100,9 +100,8 @@ static int hp_sw_tur(struct scsi_device *sdev, struct 
hp_sw_dh_data *h)
REQ_FAILFAST_DRIVER;
 
 retry:
-   res = scsi_execute_req_flags(sdev, cmd, DMA_NONE, NULL, 0, ,
-HP_SW_TIMEOUT, HP_SW_RETRIES,
-NULL, req_flags, 0);
+   res = scsi_execute(sdev, cmd, DMA_NONE, NULL, 0, NULL, ,
+   HP_SW_TIMEOUT, HP_SW_RETRIES, req_flags, 0, NULL);
if (res) {
if (scsi_sense_valid())
ret = tur_done(sdev, h, );
@@ -139,9 +138,8 @@ static int hp_sw_start_stop(struct hp_sw_dh_data *h)
REQ_FAILFAST_DRIVER;
 
 retry:
-   res = scsi_execute_req_flags(sdev, cmd, DMA_NONE, NULL, 0, ,
-HP_SW_TIMEOUT, HP_SW_RETRIES,
-NULL, req_flags, 0);
+   res = scsi_execute(sdev, cmd, DMA_NONE, NULL, 0, NULL, ,
+   HP_SW_TIMEOUT, HP_SW_RETRIES, req_flags, 0, NULL);
if (res) {
if (!scsi_sense_valid()) {
sdev_printk(KERN_WARNING, sdev,
diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c 
b/drivers/scsi/device_handler/scsi_dh_rdac.c
index b64eaae8533d..3cbab8710e58 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -555,10 +555,9 @@ static 

RE: [bug report] scsi: aacraid: Added support for hotplug

[Raghava Aditya Renukunta]

Hi Dan,

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Monday, February 13, 2017 11:44 PM
> To: Raghava Aditya Renukunta
> 
> Cc: linux-scsi@vger.kernel.org
> Subject: Re: [bug report] scsi: aacraid: Added support for hotplug
> 
> EXTERNAL EMAIL
> 
> 
> On Mon, Feb 13, 2017 at 07:39:15PM +, Raghava Aditya Renukunta
> wrote:
> > Hi Don,
> >
> > > -Original Message-
> > > From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> > > Sent: Monday, February 13, 2017 10:47 AM
> > > To: Raghava Aditya Renukunta
> > > 
> > > Cc: linux-scsi@vger.kernel.org
> > > Subject: [bug report] scsi: aacraid: Added support for hotplug
> > >
> > > EXTERNAL EMAIL
> > >
> > >
> > > Hello Raghava Aditya Renukunta,
> > >
> > > The patch 6223a39fe6fb: "scsi: aacraid: Added support for hotplug"
> > > from Feb 2, 2017, leads to the following static checker warning:
> > >
> > > drivers/scsi/aacraid/commsup.c:2243 aac_process_events()
> > > error: double unlock 'spin_lock:t_lock'
> > >
> > > drivers/scsi/aacraid/commsup.c
> > >   2130  spin_lock_irqsave(t_lock, flags);
> > > 
> > >   2131
> > >   2132  while (!list_empty(&(dev->queues-
> > > >queue[HostNormCmdQueue].cmdq))) {
> > >   2133  struct list_head *entry;
> > >   2134  struct aac_aifcmd *aifcmd;
> > >   2135  unsigned int  num;
> > >   2136  struct hw_fib **hw_fib_pool, **hw_fib_p;
> > >   2137  struct fib **fib_pool, **fib_p;
> > >   2138
> > >   2139  set_current_state(TASK_RUNNING);
> > >   2140
> > >   2141  entry = dev->queues-
> > > >queue[HostNormCmdQueue].cmdq.next;
> > >   2142  list_del(entry);
> > >   2143
> > >   2144  t_lock = dev->queues-
> >queue[HostNormCmdQueue].lock;
> > >   2145  spin_unlock_irqrestore(t_lock, flags);
> > > ^
> > >   2146
> > >   2147  fib = list_entry(entry, struct fib, fiblink);
> > >   2148  hw_fib = fib->hw_fib_va;
> > >   2149  if (dev->sa_firmware) {
> > >   2150  /* Thor AIF */
> > >   2151  aac_handle_sa_aif(dev, fib);
> > >   2152  aac_fib_adapter_complete(fib, 
> > > (u16)sizeof(u32));
> > >   2153  continue;
> > >
> > > The locking isn't right here.  We should re-take the spinlock before
> > > continuing.
> >
> > The intention here is to protect the retrieval of entry from the queues.
> > Or do you mean that we should just protect the whole while loop with one
> spin_lock (t_lock)?
> >
> 
> This is a static checker warning that says we call
> spin_unlock_irqrestore(t_lock, flags); at the end of the loop but
> sometimes we're not holding the lock.
> 
> This is a Smatch warning and it doesn't handle loops correctly.  It
> should also warn that on line 2145 we might not be holding the lock
> either but it misses that bug.
> 
> There is no way this continue is correct with regards to locking.
> 
> regards,
> dan carpenter

I agree I will fix it shortly.

Thank you.

> > Thank you,
> > Raghava Aditya
> >
> > >   2154  }
> > >   2155  /*
> > >   2156   *  We will process the FIB here or pass it 
> > > to a
> > >   2157   *  worker thread that is TBD. We Really can't
> > >   2158   *  do anything at this point since we don't 
> > > have
> > >   2159   *  anything defined for this thread to do.
> > >   2160   */
> > >
> > > [ snip ]
> > >
> > >   2221  free_mem:
> > >     /* Free up the remaining resources */
> > >   2223  hw_fib_p = hw_fib_pool;
> > >   2224  fib_p = fib_pool;
> > >   2225  while (hw_fib_p < _fib_pool[num]) {
> > >   2226  kfree(*hw_fib_p);
> > >   2227  kfree(*fib_p);
> > >   2228  ++fib_p;
> > >   2229  ++hw_fib_p;
> > >   2230  }
> > >   2231  kfree(fib_pool);
> > >   2232  free_hw_fib_pool:
> > >   2233  kfree(hw_fib_pool);
> > >   2234  free_fib:
> > >   2235  kfree(fib);
> > >   2236  t_lock = dev->queues-
> >queue[HostNormCmdQueue].lock;
> > >   2237  spin_lock_irqsave(t_lock, flags);
> > >   2238  }
> > >   2239  /*
> > >   2240   *  There are no more AIF's
> > >   2241   */
> > >   2242  t_lock = dev->queues->queue[HostNormCmdQueue].lock;
> > >   2243  spin_unlock_irqrestore(t_lock, flags);
> > >
> > > 

Re: [PATCH V4 net-next 1/2] qed: Add support for hardware offloaded FCoE.

[David Miller]

From: "Dupuis, Chad" 
Date: Mon, 13 Feb 2017 11:17:00 -0800

> @@ -255,6 +259,10 @@ struct qed_hw_info {
>   u32 part_num[4];
>  
>   unsigned char   hw_mac_addr[ETH_ALEN];
> + u64 node_wwn;
> + u64 port_wwn;
> +
> + u16 num_fcoe_conns;

This new num_fcoe_conns member should be indented just like the rest of them.

> +static int
> +qed_sp_fcoe_func_start(struct qed_hwfn *p_hwfn,
> +enum spq_mode comp_mode,
> +struct qed_spq_comp_cb *p_comp_addr)
> +{
> + struct qed_fcoe_pf_params *fcoe_pf_params = NULL;
> + struct fcoe_init_ramrod_params *p_ramrod = NULL;
> + struct fcoe_conn_context *p_cxt = NULL;
> + struct qed_spq_entry *p_ent = NULL;
> + struct fcoe_init_func_ramrod_data *p_data;
> + int rc = 0;
> + struct qed_sp_init_data init_data;
> + struct qed_cxt_info cxt_info;
> + u32 dummy_cid;
> + u16 tmp;
> + u8 i;

Please order local variable declarations from larget the smallest line.

> +#else /* CONFIG_QED_FCOE */
> +static inline struct qed_fcoe_info *
> +qed_fcoe_alloc(struct qed_hwfn *p_hwfn) { return NULL; }
> +static inline void
> +qed_fcoe_setup(struct qed_hwfn *p_hwfn, struct qed_fcoe_info *p_fcoe_info) {}
> +static inline void
> +qed_fcoe_free(struct qed_hwfn *p_hwfn, struct qed_fcoe_info *p_fcoe_info) {}
> +static inline void
> +qed_get_protocol_stats_fcoe(struct qed_dev *cdev,
> + struct qed_mcp_fcoe_stats *stats) {}

Please do not format these functions so compactly, they are very difficult to
read.  Format them just like normal functions.

[patch] scsi: megaraid_sas: array overflow in megasas_dump_frame()

[Dan Carpenter]

The "sz" variable is in terms of bytes, but we're treating the buffer as
an array of __le32 so we have to divide by 4.

Fixes: def0eab3af86 ("scsi: megaraid_sas: enhance debug logs in OCR context")
Signed-off-by: Dan Carpenter 

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
b/drivers/scsi/megaraid/megaraid_sas_base.c
index dc9f42e135bb..7ac9a9ee9bd4 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -2754,7 +2754,7 @@ megasas_dump_frame(void *mpi_request, int sz)
__le32 *mfp = (__le32 *)mpi_request;
 
printk(KERN_INFO "IO request frame:\n\t");
-   for (i = 0; i < sz; i++) {
+   for (i = 0; i < sz / sizeof(__le32); i++) {
if (i && ((i % 8) == 0))
printk("\n\t");
printk("%08x ", le32_to_cpu(mfp[i]));

[bug report] megaraid_sas: Make PI enabled VD 8 byte DMA aligned

[Dan Carpenter]

Hello sumit.sax...@avagotech.com,

The patch 0b48d12d0365: "megaraid_sas: Make PI enabled VD 8 byte DMA
aligned" from Oct 15, 2015, leads to the following static checker
warning:

drivers/scsi/megaraid/megaraid_sas_base.c:1784 
megasas_set_dynamic_target_properties()
warn: if statement not indented

drivers/scsi/megaraid/megaraid_sas_base.c
  1757  void megasas_set_dynamic_target_properties(struct scsi_device *sdev)
  1758  {
  1759  u16 pd_index = 0, ld;
  1760  u32 device_id;
  1761  struct megasas_instance *instance;
  1762  struct fusion_context *fusion;
  1763  struct MR_PRIV_DEVICE *mr_device_priv_data;
  1764  struct MR_PD_CFG_SEQ_NUM_SYNC *pd_sync;
  1765  struct MR_LD_RAID *raid;
  1766  struct MR_DRV_RAID_MAP_ALL *local_map_ptr;
  1767  
  1768  instance = megasas_lookup_instance(sdev->host->host_no);
  1769  fusion = instance->ctrl_context;
  1770  mr_device_priv_data = sdev->hostdata;
  1771  
  1772  if (!fusion || !mr_device_priv_data)
  1773  return;
  1774  
  1775  if (MEGASAS_IS_LOGICAL(sdev)) {
  1776  device_id = ((sdev->channel % 2) * 
MEGASAS_MAX_DEV_PER_CHANNEL)
  1777  + sdev->id;
  1778  local_map_ptr = fusion->ld_drv_map[(instance->map_id & 
1)];
  1779  ld = MR_TargetIdToLdGet(device_id, local_map_ptr);
  1780  if (ld >= instance->fw_supported_vd_count)
  1781  return;
  1782  raid = MR_LdRaidGet(ld, local_map_ptr);
  1783  
  1784  if (raid->capability.ldPiMode == 
MR_PROT_INFO_TYPE_CONTROLLER)
  1785  blk_queue_update_dma_alignment(sdev->request_queue, 
0x7);


  1786  
  1787  mr_device_priv_data->is_tm_capable =
  1788  raid->capability.tmCapable;
  1789  } else if (instance->use_seqnum_jbod_fp) {
  1790  pd_index = (sdev->channel * 
MEGASAS_MAX_DEV_PER_CHANNEL) +
  1791  sdev->id;
  1792  pd_sync = (void *)fusion->pd_seq_sync
  1793  [(instance->pd_seq_map_id - 1) & 1];
  1794  mr_device_priv_data->is_tm_capable =
  1795  pd_sync->seq[pd_index].capability.tmCapable;
  1796  }
  1797  }


regards,
dan carpenter

Re: [PATCH] cdrom: Make device operations read-only

[Jens Axboe]

On 02/13/2017 05:25 PM, Kees Cook wrote:
> Since function tables are a common target for attackers, it's best to keep
> them in read-only memory. As such, this makes the CDROM device ops tables
> const. This drops additionally n_minors, since it isn't used meaningfully,
> and sets the only user of cdrom_dummy_generic_packet explicitly so the
> variables can all be const.

Agree, it's a good change. Applied for 4.11.

-- 
Jens Axboe

Re: [PATCH] scsi: lpfc: Add shutdown method for kexec

[Brian King]

On 02/13/2017 08:04 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2017-02-13 at 15:57 -0600, Brian King wrote:
>> If we do transition to use remove rather than shutdown, I think we
>> want
>> some way for a device driver to know whether we are doing kexec or
>> not.
>> A RAID adapter with a write cache is going to want to flush its write
>> cache on a PCI hotplug remove, but for a kexec, its going to want to
>> skip
>> that so the kexec is faster. Today, since kexec looks like a reboot,
>> rather than a shutdown, we can skip the flush on a reboot, since its
>> technically not needed there either.
> 
> What happens if a non-flushed adapter gets a PERST ?

It depends on the adapter, so it really needs to be a policy in the device
driver. For any adapter that has a non volatile write cache, data must 
preserved,
otherwise the write cache is not truly non volatile. For adapters with a 
volatile
write cache, then its subject to the adapter hardware. For ipr adapters
that have a volatile cache, they do guarantee the data is preserved across
a PERST.

> I wouldn't trust that 'don't have to flush' magic ...

I really don't think we want to force RAID adapters that have gigabytes of
*non volatile* write cache to flush their cache when we are merely performing
a kexec. This can take several minutes in the worst case scenarios.

-Brian

-- 
Brian King
Power Linux I/O
IBM Linux Technology Center

[PATCH v3] sd: Check for unaligned partial completion

[Damien Le Moal]

Commit "mpt3sas: Force request partial completion alignment" was not
considering the case of REQ_TYPE_FS commands not operating on sector
size units (e.g. REQ_OP_ZONE_REPORT and its 64B aligned partial
replies). This could result is incorrectly retrying (forever) those
commands.

Move the partial completion alignement check of mpt3sas to sd_done so
that the check comes after good_bytes & resid corrections of done in
that function depending on the request command to avoid false positive.

Signed-off-by: Damien Le Moal 

---

Changes from v2:
- Fixed good_bytes calculation after correction of unaligned resid
It should be good_bytes=scsi_buflen() - resid, and not good_bytes-=resid
- Fixed email addresses used with git send-email.
My apologies about the noise...

 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 ---
 drivers/scsi/sd.c| 20 
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 0b5b423..1961535 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -4658,7 +4658,6 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 
msix_index, u32 reply)
struct MPT3SAS_DEVICE *sas_device_priv_data;
u32 response_code = 0;
unsigned long flags;
-   unsigned int sector_sz;
 
mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
@@ -4717,20 +4716,6 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 
msix_index, u32 reply)
}
 
xfer_cnt = le32_to_cpu(mpi_reply->TransferCount);
-
-   /* In case of bogus fw or device, we could end up having
-* unaligned partial completion. We can force alignment here,
-* then scsi-ml does not need to handle this misbehavior.
-*/
-   sector_sz = scmd->device->sector_size;
-   if (unlikely(scmd->request->cmd_type == REQ_TYPE_FS && sector_sz &&
-xfer_cnt % sector_sz)) {
-   sdev_printk(KERN_INFO, scmd->device,
-   "unaligned partial completion avoided (xfer_cnt=%u, 
sector_sz=%u)\n",
-   xfer_cnt, sector_sz);
-   xfer_cnt = round_down(xfer_cnt, sector_sz);
-   }
-
scsi_set_resid(scmd, scsi_bufflen(scmd) - xfer_cnt);
if (ioc_status & MPI2_IOCSTATUS_FLAG_LOG_INFO_AVAILABLE)
log_info =  le32_to_cpu(mpi_reply->IOCLogInfo);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 1f5d92a..2f70b36 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1790,6 +1790,8 @@ static int sd_done(struct scsi_cmnd *SCpnt)
 {
int result = SCpnt->result;
unsigned int good_bytes = result ? 0 : scsi_bufflen(SCpnt);
+   unsigned int sector_size = SCpnt->device->sector_size;
+   unsigned int resid;
struct scsi_sense_hdr sshdr;
struct scsi_disk *sdkp = scsi_disk(SCpnt->request->rq_disk);
struct request *req = SCpnt->request;
@@ -1829,6 +1831,24 @@ static int sd_done(struct scsi_cmnd *SCpnt)
}
sdkp->medium_access_timed_out = 0;
 
+   /*
+* In case of bogus fw or device, we could end up having
+* unaligned partial completion. Check this here and force
+* alignment.
+*/
+   resid = scsi_get_resid(SCpnt);
+   if (resid & (sector_size - 1)) {
+   SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt,
+   "Unaligned partial completion (resid=%u, 
sector_sz=%u)\n",
+   resid, sector_size));
+   resid = round_up(resid, sector_size);
+   if (resid < scsi_bufflen(SCpnt))
+   good_bytes = scsi_bufflen(SCpnt) - resid;
+   else
+   good_bytes = 0;
+   scsi_set_resid(SCpnt, resid);
+   }
+
if (driver_byte(result) != DRIVER_SENSE &&
(!sense_valid || sense_deferred))
goto out;
-- 
2.9.3

[RESEND][PATCH v2] ata: xgene: Enable NCQ support for APM X-Gene SATA controller hardware v1.1

[Rameshwar Prasad Sahu]

This patch enables NCQ support for APM X-Gene SATA controller hardware v1.1
that was broken with hardware v1.0. Second thing, here we should not assume
XGENE_AHCI_V2 always in case of having valid _CID in ACPI table. I need to
remove this assumption because V1_1 also has a valid _CID for backward
compatibly with v1.

v2 changes:
1. Changed patch description

Signed-off-by: Rameshwar Prasad Sahu 
---
 drivers/ata/ahci_xgene.c |   14 --
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 73b19b2..8b88be9 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -87,6 +87,7 @@

 enum xgene_ahci_version {
XGENE_AHCI_V1 = 1,
+   XGENE_AHCI_V1_1,
XGENE_AHCI_V2,
 };

@@ -734,6 +735,7 @@ static struct scsi_host_template ahci_platform_sht = {
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id xgene_ahci_acpi_match[] = {
{ "APMC0D0D", XGENE_AHCI_V1},
+   { "APMC0D67", XGENE_AHCI_V1_1},
{ "APMC0D32", XGENE_AHCI_V2},
{},
 };
@@ -742,6 +744,7 @@ MODULE_DEVICE_TABLE(acpi, xgene_ahci_acpi_match);

 static const struct of_device_id xgene_ahci_of_match[] = {
{.compatible = "apm,xgene-ahci", .data = (void *) XGENE_AHCI_V1},
+   {.compatible = "apm,xgene-ahci-v1-1", .data = (void *) XGENE_AHCI_V1_1},
{.compatible = "apm,xgene-ahci-v2", .data = (void *) XGENE_AHCI_V2},
{},
 };
@@ -755,8 +758,7 @@ static int xgene_ahci_probe(struct platform_device *pdev)
struct resource *res;
const struct of_device_id *of_devid;
enum xgene_ahci_version version = XGENE_AHCI_V1;
-   const struct ata_port_info *ppi[] = { _ahci_v1_port_info,
- _ahci_v2_port_info };
+   const struct ata_port_info *ppi;
int rc;

hpriv = ahci_platform_get_resources(pdev);
@@ -821,8 +823,6 @@ static int xgene_ahci_probe(struct platform_device *pdev)
dev_warn(>dev, "%s: Error reading device 
info. Assume version1\n",
__func__);
version = XGENE_AHCI_V1;
-   } else if (info->valid & ACPI_VALID_CID) {
-   version = XGENE_AHCI_V2;
}
}
}
@@ -858,18 +858,20 @@ skip_clk_phy:

switch (version) {
case XGENE_AHCI_V1:
+   ppi = _ahci_v1_port_info;
hpriv->flags = AHCI_HFLAG_NO_NCQ;
break;
case XGENE_AHCI_V2:
+   ppi = _ahci_v2_port_info;
hpriv->flags |= AHCI_HFLAG_YES_FBS;
hpriv->irq_handler = xgene_ahci_irq_intr;
break;
default:
+   ppi = _ahci_v1_port_info;
break;
}

-   rc = ahci_platform_init_host(pdev, hpriv, ppi[version - 1],
-_platform_sht);
+   rc = ahci_platform_init_host(pdev, hpriv, ppi, _platform_sht);
if (rc)
goto disable_resources;

--
1.7.1