Price Inquiry

[Daniel Murray]

Hi,friend,
 
This is Daniel Murray and i am from Sinara Group Co.Ltd Group Co.,LTD in Russia.
We are glad to know about your company from the web and we are interested in 
your products.
Could you kindly send us your Latest catalog and price list for our trial order.
 
Best Regards,
 
Daniel Murray
Purchasing Manager

Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg

[Daniel Vetter]

On Wed, Sep 12, 2018 at 5:08 PM, Arnd Bergmann  wrote:
> The .ioctl and .compat_ioctl file operations have the same prototype so
> they can both point to the same function, which works great almost all
> the time when all the commands are compatible.
>
> One exception is the s390 architecture, where a compat pointer is only
> 31 bit wide, and converting it into a 64-bit pointer requires calling
> compat_ptr(). Most drivers here will ever run in s390, but since we now
> have a generic helper for it, it's easy enough to use it consistently.
>
> I double-checked all these drivers to ensure that all ioctl arguments
> are used as pointers or are ignored, but are not interpreted as integer
> values.
>
> Signed-off-by: Arnd Bergmann 

Acked-by: Daniel Vetter 

At least for the drm and dma-buf bits.
-Daniel

> ---
>  drivers/android/binder.c| 2 +-
>  drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +-
>  drivers/dma-buf/dma-buf.c   | 4 +---
>  drivers/dma-buf/sw_sync.c   | 2 +-
>  drivers/dma-buf/sync_file.c | 2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c| 2 +-
>  drivers/hid/hidraw.c| 4 +---
>  drivers/iio/industrialio-core.c | 2 +-
>  drivers/infiniband/core/uverbs_main.c   | 4 ++--
>  drivers/media/rc/lirc_dev.c | 4 +---
>  drivers/mfd/cros_ec_dev.c   | 4 +---
>  drivers/misc/vmw_vmci/vmci_host.c   | 2 +-
>  drivers/nvdimm/bus.c| 4 ++--
>  drivers/nvme/host/core.c| 2 +-
>  drivers/pci/switch/switchtec.c  | 2 +-
>  drivers/platform/x86/wmi.c  | 2 +-
>  drivers/rpmsg/rpmsg_char.c  | 4 ++--
>  drivers/sbus/char/display7seg.c | 2 +-
>  drivers/sbus/char/envctrl.c | 4 +---
>  drivers/scsi/3w-.c  | 4 +---
>  drivers/scsi/cxlflash/main.c| 2 +-
>  drivers/scsi/esas2r/esas2r_main.c   | 2 +-
>  drivers/scsi/pmcraid.c  | 4 +---
>  drivers/staging/android/ion/ion.c   | 4 +---
>  drivers/staging/vme/devices/vme_user.c  | 2 +-
>  drivers/tee/tee_core.c  | 2 +-
>  drivers/usb/class/cdc-wdm.c | 2 +-
>  drivers/usb/class/usbtmc.c  | 4 +---
>  drivers/video/fbdev/ps3fb.c | 2 +-
>  drivers/virt/fsl_hypervisor.c   | 2 +-
>  fs/btrfs/super.c| 2 +-
>  fs/ceph/dir.c   | 2 +-
>  fs/ceph/file.c  | 2 +-
>  fs/fuse/dev.c   | 2 +-
>  fs/notify/fanotify/fanotify_user.c  | 2 +-
>  fs/userfaultfd.c| 2 +-
>  net/rfkill/core.c   | 2 +-
>  37 files changed, 40 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index d58763b6b009..d2464f5759f8 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5576,7 +5576,7 @@ static const struct file_operations binder_fops = {
> .owner = THIS_MODULE,
> .poll = binder_poll,
> .unlocked_ioctl = binder_ioctl,
> -   .compat_ioctl = binder_ioctl,
> +   .compat_ioctl = generic_compat_ioctl_ptrarg,
> .mmap = binder_mmap,
> .open = binder_open,
> .flush = binder_flush,
> diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c 
> b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> index abc7a7f64d64..8ff77a70addc 100644
> --- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> +++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> @@ -68,7 +68,7 @@ static long adf_ctl_ioctl(struct file *fp, unsigned int 
> cmd, unsigned long arg);
>  static const struct file_operations adf_ctl_ops = {
> .owner = THIS_MODULE,
> .unlocked_ioctl = adf_ctl_ioctl,
> -   .compat_ioctl = adf_ctl_ioctl,
> +   .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>
>  struct adf_ctl_drv_info {
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 13884474d158..a6d7dc4cf7e9 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,9 +325,7 @@ static const struct file_operations dma_buf_fops = {
> .llseek = dma_buf_llseek,
> .poll   = dma_buf_poll,
> .unlocked_ioctl = dma_buf_ioctl,
> -#ifdef CONFIG_COMPAT
> -   .compat_ioctl   = dma_buf_ioctl,
> -#endif
> +   .compat_ioctl   = generic_compat_ioctl_ptrarg,
>  };
>
>  /*
> diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> index 53c1d6d36a64..bc8105

Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution

[Daniel Borkmann]

On 01/11/2018 04:58 PM, Dan Williams wrote:
> On Thu, Jan 11, 2018 at 1:54 AM, Jiri Kosina  wrote:
>> On Tue, 9 Jan 2018, Josh Poimboeuf wrote:
>>> On Tue, Jan 09, 2018 at 11:44:05AM -0800, Dan Williams wrote:
>>>> On Tue, Jan 9, 2018 at 11:34 AM, Jiri Kosina  wrote:
>>>>> On Fri, 5 Jan 2018, Dan Williams wrote:
>>>>>
>>>>> [ ... snip ... ]
>>>>>> Andi Kleen (1):
>>>>>>   x86, barrier: stop speculation for failed access_ok
>>>>>>
>>>>>> Dan Williams (13):
>>>>>>   x86: implement nospec_barrier()
>>>>>>   [media] uvcvideo: prevent bounds-check bypass via speculative 
>>>>>> execution
>>>>>>   carl9170: prevent bounds-check bypass via speculative execution
>>>>>>   p54: prevent bounds-check bypass via speculative execution
>>>>>>   qla2xxx: prevent bounds-check bypass via speculative execution
>>>>>>   cw1200: prevent bounds-check bypass via speculative execution
>>>>>>   Thermal/int340x: prevent bounds-check bypass via speculative 
>>>>>> execution
>>>>>>   ipv6: prevent bounds-check bypass via speculative execution
>>>>>>   ipv4: prevent bounds-check bypass via speculative execution
>>>>>>   vfs, fdtable: prevent bounds-check bypass via speculative execution
>>>>>>   net: mpls: prevent bounds-check bypass via speculative execution
>>>>>>   udf: prevent bounds-check bypass via speculative execution
>>>>>>   userns: prevent bounds-check bypass via speculative execution
>>>>>>
>>>>>> Mark Rutland (4):
>>>>>>   asm-generic/barrier: add generic nospec helpers
>>>>>>   Documentation: document nospec helpers
>>>>>>   arm64: implement nospec_ptr()
>>>>>>   arm: implement nospec_ptr()
>>>>>
>>>>> So considering the recent publication of [1], how come we all of a sudden
>>>>> don't need the barriers in ___bpf_prog_run(), namely for LD_IMM_DW and
>>>>> LDX_MEM_##SIZEOP, and something comparable for eBPF JIT?
>>>>>
>>>>> Is this going to be handled in eBPF in some other way?
>>>>>
>>>>> Without that in place, and considering Jann Horn's paper, it would seem
>>>>> like PTI doesn't really lock it down fully, right?
>>>>
>>>> Here is the latest (v3) bpf fix:
>>>>
>>>> https://patchwork.ozlabs.org/patch/856645/
>>>>
>>>> I currently have v2 on my 'nospec' branch and will move that to v3 for
>>>> the next update, unless it goes upstream before then.
>>
>> Daniel, I guess you're planning to send this still for 4.15?
> 
> It's pending in the bpf.git tree:
> 
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=b2157399cc9

Sorry for the delay, just noticed the question now since not on Cc either:
It made it into in DaveM's tree already and part of his latest pull-req
to Linus.

>>> That patch seems specific to CONFIG_BPF_SYSCALL.  Is the bpf() syscall
>>> the only attack vector?  Or are there other ways to run bpf programs
>>> that we should be worried about?
>>
>> Seems like Alexei is probably the only person in the whole universe who
>> isn't CCed here ... let's fix that.
> 
> He will be cc'd on v2 of this series which will be available later today.

Re: [Nouveau] [PATCH 03/10] driver:gpu: return -ENOMEM on allocation failure.

[Daniel Vetter]

On Wed, Sep 13, 2017 at 01:02:12PM +0530, Allen Pais wrote:
> Signed-off-by: Allen Pais 

Applied to drm-misc-next, thanks.
-Daniel

> ---
>  drivers/gpu/drm/gma500/mid_bios.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/gma500/mid_bios.c 
> b/drivers/gpu/drm/gma500/mid_bios.c
> index d75ecb3..1fa1633 100644
> --- a/drivers/gpu/drm/gma500/mid_bios.c
> +++ b/drivers/gpu/drm/gma500/mid_bios.c
> @@ -237,7 +237,7 @@ static int mid_get_vbt_data_r10(struct drm_psb_private 
> *dev_priv, u32 addr)
>  
>   gct = kmalloc(sizeof(*gct) * vbt.panel_count, GFP_KERNEL);
>   if (!gct)
> - return -1;
> + return -ENOMEM;
>  
>   gct_virtual = ioremap(addr + sizeof(vbt),
>   sizeof(*gct) * vbt.panel_count);
> -- 
> 2.7.4
> 
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 02/15] drm: make device_type const

[Daniel Vetter]

On Sat, Aug 19, 2017 at 01:52:13PM +0530, Bhumika Goyal wrote:
> Make these const as they are only stored in the type field of a device
> structure, which is const.
> Done using Coccinelle.

I can't apply this, it's missing your s-o-b line. You can just replay with
that.

Thanks, Daniel

> ---
>  drivers/gpu/drm/drm_sysfs.c  | 2 +-
>  drivers/gpu/drm/ttm/ttm_module.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 1c5b5ce..84e4ebe 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -39,7 +39,7 @@
>   * drm_connector_unregister().
>   */
>  
> -static struct device_type drm_sysfs_device_minor = {
> +static const struct device_type drm_sysfs_device_minor = {
>   .name = "drm_minor"
>  };
>  
> diff --git a/drivers/gpu/drm/ttm/ttm_module.c 
> b/drivers/gpu/drm/ttm/ttm_module.c
> index 66fc639..e6604e0 100644
> --- a/drivers/gpu/drm/ttm/ttm_module.c
> +++ b/drivers/gpu/drm/ttm/ttm_module.c
> @@ -37,7 +37,7 @@
>  static DECLARE_WAIT_QUEUE_HEAD(exit_q);
>  static atomic_t device_released;
>  
> -static struct device_type ttm_drm_class_type = {
> +static const struct device_type ttm_drm_class_type = {
>   .name = "ttm",
>   /**
>* Add pm ops here.
> -- 
> 1.9.1
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 05/22] drm/i915: Make use of the new sg_map helper function

[Daniel Vetter]

On Thu, Apr 13, 2017 at 04:05:18PM -0600, Logan Gunthorpe wrote:
> This is a single straightforward conversion from kmap to sg_map.
> 
> Signed-off-by: Logan Gunthorpe 

Acked-by: Daniel Vetter 

Probably makes sense to merge through some other tree, but please be aware
of the considerable churn rate in i915 (i.e. make sure your tree is in
linux-next before you send a pull request for this). Plane B would be to
get the prep patch in first and then merge the i915 conversion one kernel
release later.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 27 ---
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 67b1fc5..1b1b91a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2188,6 +2188,15 @@ static void __i915_gem_object_reset_page_iter(struct 
> drm_i915_gem_object *obj)
>   radix_tree_delete(&obj->mm.get_page.radix, iter.index);
>  }
>  
> +static void i915_gem_object_unmap(const struct drm_i915_gem_object *obj,
> +   void *ptr)
> +{
> + if (is_vmalloc_addr(ptr))
> + vunmap(ptr);
> + else
> + sg_unmap(obj->mm.pages->sgl, ptr, SG_KMAP);
> +}
> +
>  void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
>enum i915_mm_subclass subclass)
>  {
> @@ -2215,10 +2224,7 @@ void __i915_gem_object_put_pages(struct 
> drm_i915_gem_object *obj,
>   void *ptr;
>  
>   ptr = ptr_mask_bits(obj->mm.mapping);
> - if (is_vmalloc_addr(ptr))
> - vunmap(ptr);
> - else
> - kunmap(kmap_to_page(ptr));
> + i915_gem_object_unmap(obj, ptr);
>  
>   obj->mm.mapping = NULL;
>   }
> @@ -2475,8 +2481,11 @@ static void *i915_gem_object_map(const struct 
> drm_i915_gem_object *obj,
>   void *addr;
>  
>   /* A single page can always be kmapped */
> - if (n_pages == 1 && type == I915_MAP_WB)
> - return kmap(sg_page(sgt->sgl));
> + if (n_pages == 1 && type == I915_MAP_WB) {
> + addr = sg_map(sgt->sgl, SG_KMAP);
> + if (IS_ERR(addr))
> + return NULL;
> + }
>  
>   if (n_pages > ARRAY_SIZE(stack_pages)) {
>   /* Too big for stack -- allocate temporary array instead */
> @@ -2543,11 +2552,7 @@ void *i915_gem_object_pin_map(struct 
> drm_i915_gem_object *obj,
>   goto err_unpin;
>   }
>  
> - if (is_vmalloc_addr(ptr))
> - vunmap(ptr);
> - else
> - kunmap(kmap_to_page(ptr));
> -
> + i915_gem_object_unmap(obj, ptr);
>   ptr = obj->mm.mapping = NULL;
>   }
>  
> -- 
> 2.1.4
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Claims Requirements

[Daniel Ellmerer]

The British National Lottery
P O Box 1010
3b Olympic Way, Sefton Business Park,
Aintree, Liverpool , L30 1RD
(Customer Services)

Ref: UK/9420X2/68
Batch: 074/05/ZY369
Ticket number:56475600545 188
Lucky Numbers: 05,06,17,20,28,42(Bonus33)

WINNING NOTIFICATION:

We wish to congratulate and inform you on the selection of cash prize
1,000,000.00 (British Pounds) held on the 20th January 2017
in London Uk.The selection process was carried out through random
selection in Our computerized email selection system (ess) from a
database of over 250,000 email addresses drawn from which you were
selected. And Your e-mail address attached to ticket number:
56475600545 188 with Serial number 5368/02 drew the lucky numbers:
05, 06, 17, 20, 28, 42 (Bonus 33) ,which subsequently won you the
lottery in the 1st category i.e match 5 plus bonus.

You have therefore been approved to claim a total sum of 1 Million
Pounds,(One Million Pounds) in cash credited to fileKTU/ 9023118308/03.
This is from a total cash prize of ?1,000,000 Million Pounds,shared
amongst the (4)lucky winners in this category i.e Match 6 plus bonus.
For due processing of your winning claim,please contact the
FIDUCIARY AGENT Information Officer Mr. Francisco pedro who has been
assigned to assist you. You are to contact him with the following
details for the release of your winnings.

Agent Name: Mr. Francisco pedro
Tel:+447024077948
Email: mr.franciscopedro...@yahoo.com

Contact him, please provide him with the following Requirements below:

Claims Requirements:
1.Name in full--
2.Address---
3.Nationality---
4.Age---
5.Occupation
6.Sex --
7.Phone/Fax-
8.Present Country---

If you do not contact your claims agent within 5 working days of this
Notification, your winnings would be revoked. Winners are advised to
keep their winning details/information from the public to avoid
Fraudulent claim (IMPORTANT) pending the prize claim by Winner.

*Winner under the age of 18 are automatically disqualified. *Staff of
the British Lottery are not to partake in this Lottery.

Accept my hearty congratulations once again!

Regards
Mrs. Stella Ellis
(Group Coordinator
Note that you are not to reply to this E-mail,please contact your
claims officer directly to start the processing of your claims
application form.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sd: assign appropriate log level

[Daniel Walker]

On 10/17/2016 10:19 AM, James Bottomley wrote:

On Mon, 2016-10-17 at 09:51 -0700, David Singleton wrote:

From: Shikhar Dogra 

Reduce chatter on console for usb hotplug.
KERN_ERR is too high severity for these messages, moving them
to KERN_WARNING

It's an error because we have several USB to IDE bridges that have
write back cache drives but report nothing to the caching mode page.
  For them this is a serious error because their data integrity is at
risk.  I'm open to other ways to fix your problem, but downgrading the
message severity because *you* don't have an issue would mask the
problem for others, so it's not really viable.


Is there a way to detect when you have a device of the type where this 
is a serious issue ? This typically happen for USB drives, but seems to 
have no effect on them.



Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] virtio_scsi: use complete() instead complete_all()

[Daniel Wagner]

From: Daniel Wagner 

There is only one waiter for the completion, therefore there is no need
to use complete_all(). Let's make that clear by using complete() instead
of complete_all().

The usage pattern of the completion is:

waiter context  waker context

virtscsi_tmf()
  DECLARE_COMPLETION_ONSTACK()
  virtscsi_kick_cmd()
  wait_for_completion()

virtscsi_complete_free()
  complete()

Signed-off-by: Daniel Wagner 
---
 drivers/scsi/virtio_scsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 7dbbb29..86924ff 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -258,7 +258,7 @@ static void virtscsi_complete_free(struct virtio_scsi 
*vscsi, void *buf)
struct virtio_scsi_cmd *cmd = buf;
 
if (cmd->comp)
-   complete_all(cmd->comp);
+   complete(cmd->comp);
 }
 
 static void virtscsi_ctrl_done(struct virtqueue *vq)
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] sym53c8xx_2: use complete() instead complete_all()

[Daniel Wagner]

From: Daniel Wagner 

There is only one waiter for the completion, therefore there is no need
to use complete_all(). Let's make that clear by using complete() instead
of complete_all().

The usage pattern of the completion is:

waiter context  waker context

sym_eh_handler()
  struct completion eh_done
  init_completion(eh_done)
  pci_channel_offline()
  wait_for_completion_timeout(eh_done)

sym2_io_resume()
  complete(eh_done)

Signed-off-by: Daniel Wagner 
---
 drivers/scsi/sym53c8xx_2/sym_glue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c 
b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 5d00e51..d32e3ba 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -1874,7 +1874,7 @@ static void sym2_io_resume(struct pci_dev *pdev)
 
spin_lock_irq(shost->host_lock);
if (sym_data->io_reset)
-   complete_all(sym_data->io_reset);
+   complete(sym_data->io_reset);
spin_unlock_irq(shost->host_lock);
 }
 
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] wireless: Use complete() instead complete_all()

[Daniel Wagner]

From: Daniel Wagner 

Hi,

Using complete_all() is not wrong per se but it suggest that there
might be more than one waiter. For -rt I am reviewing all
complete_all() users and would like to leave only the real ones in the
tree. The main problem for -rt about complete_all() is that it can be
uses inside IRQ context and that can lead to unbounded amount work
inside the interrupt handler. That is a no no for -rt.

The patches grouped per subsystem and in small batches to allow
reviewing.

Patch #1 fixes a minor bug in the completion API usage. The current code
works but it could work better.

cheers,
daniel

Daniel Wagner (3):
  csiostor: fix completion usage
  sym53c8xx_2: use complete() instead complete_all()
  virtio_scsi: use complete() instead complete_all()

 drivers/scsi/csiostor/csio_scsi.c   | 5 ++---
 drivers/scsi/sym53c8xx_2/sym_glue.c | 2 +-
 drivers/scsi/virtio_scsi.c  | 2 +-
 3 files changed, 4 insertions(+), 5 deletions(-)

-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] csiostor: fix completion usage

[Daniel Wagner]

From: Daniel Wagner 

The (re)initialzing of the completion object should be done before we
trigger the transfer. Doing this after triggering the hardware opens up
a race window. Without the timeout we would problaly even deadlock. Use
also reinit_completion because we initalize the whole data structure in
csio_scscim_init().

There is only one waiter for the completion, therefore there is no need
to use complete_all(). Let's make that clear by using complete() instead
of complete_all().

The usage pattern of the completion is:

waiter context  waker context

csio_eh_abort_handler()
  reinit_completion()
  wait_for_completion_timeout()

csio_scsi_err_handler()
  complete()

Signed-off-by: Daniel Wagner 
---
 drivers/scsi/csiostor/csio_scsi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/csiostor/csio_scsi.c 
b/drivers/scsi/csiostor/csio_scsi.c
index c2a6f9f..89a52b9 100644
--- a/drivers/scsi/csiostor/csio_scsi.c
+++ b/drivers/scsi/csiostor/csio_scsi.c
@@ -1721,7 +1721,7 @@ csio_scsi_err_handler(struct csio_hw *hw, struct 
csio_ioreq *req)
 
/* Wake up waiting threads */
csio_scsi_cmnd(req) = NULL;
-   complete_all(&req->cmplobj);
+   complete(&req->cmplobj);
 }
 
 /*
@@ -1945,6 +1945,7 @@ csio_eh_abort_handler(struct scsi_cmnd *cmnd)
ready = csio_is_lnode_ready(ln);
tmo = CSIO_SCSI_ABRT_TMO_MS;
 
+   reinit_completion(&ioreq->cmplobj);
spin_lock_irq(&hw->lock);
rv = csio_do_abrt_cls(hw, ioreq, (ready ? SCSI_ABORT : SCSI_CLOSE));
spin_unlock_irq(&hw->lock);
@@ -1964,8 +1965,6 @@ csio_eh_abort_handler(struct scsi_cmnd *cmnd)
goto inval_scmnd;
}
 
-   /* Wait for completion */
-   init_completion(&ioreq->cmplobj);
wait_for_completion_timeout(&ioreq->cmplobj, msecs_to_jiffies(tmo));
 
/* FW didnt respond to abort within our timeout */
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] cxlflash: Fix to avoid virtual LUN failover failure

[Daniel Axtens]

> Virtual LUNs may be accessed through one or both ports of the adapter.

Is it possible that there might ever be adapters with a number of ports
other than 2? In particular, is it possible for 3 or 4 port adapters to
exist?

If so, do you need something with a bit more fidelity?

If not, this is a good approach.

Regards,
Daniel

> This access is encoded in the translation entries that comprise the
> virtual LUN and used by the AFU for load-balancing I/O and handling
> failover scenarios. In a link loss scenario, even though the AFU is
> able to maintain connectivity to the LUN, it is up to the application
> to retry the failed I/O. When applications are unaware of the virtual
> LUN's underlying topology, they are unable to make a sound decision of
> when to retry an I/O and therefore are forced to make their reaction to
> a failed I/O absolute. The result is either a failure to retry I/O or
> increased latency for scenarios where a retry is pointless.
>
> To remedy this scenario, provide feedback back to the application on
> virtual LUN creation as to which ports the LUN may be accessed. LUN's
> spanning both ports are candidates for a retry in a presence of an I/O
> failure.
>
> Signed-off-by: Matthew R. Ochs 
> ---
>  drivers/scsi/cxlflash/vlun.c   |  2 ++
>  include/uapi/scsi/cxlflash_ioctl.h | 10 ++
>  2 files changed, 12 insertions(+)
>
> diff --git a/drivers/scsi/cxlflash/vlun.c b/drivers/scsi/cxlflash/vlun.c
> index a53f583..50f8e93 100644
> --- a/drivers/scsi/cxlflash/vlun.c
> +++ b/drivers/scsi/cxlflash/vlun.c
> @@ -1008,6 +1008,8 @@ int cxlflash_disk_virtual_open(struct scsi_device 
> *sdev, void *arg)
>   virt->last_lba = last_lba;
>   virt->rsrc_handle = rsrc_handle;
>  
> + if (lli->port_sel == BOTH_PORTS)
> + virt->hdr.return_flags |= DK_CXLFLASH_ALL_PORTS_ACTIVE;
>  out:
>   if (likely(ctxi))
>   put_context(ctxi);
> diff --git a/include/uapi/scsi/cxlflash_ioctl.h 
> b/include/uapi/scsi/cxlflash_ioctl.h
> index 831351b..2302f3c 100644
> --- a/include/uapi/scsi/cxlflash_ioctl.h
> +++ b/include/uapi/scsi/cxlflash_ioctl.h
> @@ -31,6 +31,16 @@ struct dk_cxlflash_hdr {
>  };
>  
>  /*
> + * Return flag definitions available to all ioctls
> + *
> + * Similar to the input flags, these are grown from the bottom-up with the
> + * intention that ioctl-specific return flag definitions would grow from the
> + * top-down, allowing the two sets to co-exist. While not required/enforced
> + * at this time, this provides future flexibility.
> + */
> +#define DK_CXLFLASH_ALL_PORTS_ACTIVE 0x0001ULL
> +
> +/*
>   * Notes:
>   * -
>   * The 'context_id' field of all ioctl structures contains the context
> -- 
> 2.1.0
>
> ___
> Linuxppc-dev mailing list
> linuxppc-...@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev


signature.asc
Description: PGP signature

Re: [PATCH 3/6] cxlflash: Updated date of the driver

[Daniel Axtens]

Hi Uma,

It looks like CXLFLASH_DRIVER_DATE is only used once, on init, and it's
just printed. Is it necessary? It looks like having it will require
sending a patch to update it quite often.

Regards,
Daniel



signature.asc
Description: PGP signature

Re: [PATCH v5 18/34] cxlflash: Fix AFU version access/storage and add check

[Daniel Axtens]

"Matthew R. Ochs"  writes:

> Correct, downlevel (unsupported) AFUs don't implement the interface_version
> register and thus will return -1 at that offset.
Cool. It would be good to document that somewhere, either the comment or
the commit message, but it's not worth holding up the series for it.

I also tend to use the form (var == ~0ULL) for tests like this but
that's an aesthetic thing.

Reviewed-by: Daniel Axtens 

Regards,
Daniel


signature.asc
Description: PGP signature

Re: [PATCH v5 17/34] cxlflash: Remove dual port online dependency

[Daniel Axtens]

"Matthew R. Ochs"  writes:

> At present, both ports must be online for the device to
> configure properly. Remove this dependency and the unnecessary
> internal LUN override logic as well. Additionally, as a refactoring
> measure, change the return code variable name to match that used
> throughout the driver.
>
> With this change, the card will be able to configure even when the
> link is down. At some later point when the link is transitioned to
> 'up', a link state change interrupt will trigger the port configuration.
> Note that despite its void-like behavior, the function was left with a
> return code for right now in case its behavior needs to be altered again
> in the near future based on testing.
>

Thanks for updating that.

Reviewed-by: Daniel Axtens 

Regards,
Daniel

> Signed-off-by: Matthew R. Ochs 
> Signed-off-by: Manoj N. Kumar 
> Reviewed-by: Brian King 
> ---
>  drivers/scsi/cxlflash/main.c | 27 ++-
>  1 file changed, 10 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index ed9fd8c..c25efc3 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -1030,7 +1030,7 @@ static int wait_port_offline(u64 *fc_regs, u32 
> delay_us, u32 nretry)
>   */
>  static int afu_set_wwpn(struct afu *afu, int port, u64 *fc_regs, u64 wwpn)
>  {
> - int ret = 0;
> + int rc = 0;
>  
>   set_port_offline(fc_regs);
>  
> @@ -1038,33 +1038,26 @@ static int afu_set_wwpn(struct afu *afu, int port, 
> u64 *fc_regs, u64 wwpn)
>  FC_PORT_STATUS_RETRY_CNT)) {
>   pr_debug("%s: wait on port %d to go offline timed out\n",
>__func__, port);
> - ret = -1; /* but continue on to leave the port back online */
> + rc = -1; /* but continue on to leave the port back online */
>   }
>  
> - if (ret == 0)
> + if (rc == 0)
>   writeq_be(wwpn, &fc_regs[FC_PNAME / 8]);
>  
> + /* Always return success after programming WWPN */
> + rc = 0;
> +
>   set_port_online(fc_regs);
>  
>   if (!wait_port_online(fc_regs, FC_PORT_STATUS_RETRY_INTERVAL_US,
> FC_PORT_STATUS_RETRY_CNT)) {
> - pr_debug("%s: wait on port %d to go online timed out\n",
> -  __func__, port);
> - ret = -1;
> -
> - /*
> -  * Override for internal lun!!!
> -  */
> - if (afu->internal_lun) {
> - pr_debug("%s: Overriding port %d online timeout!!!\n",
> -  __func__, port);
> - ret = 0;
> - }
> + pr_err("%s: wait on port %d to go online timed out\n",
> +__func__, port);
>   }
>  
> - pr_debug("%s: returning rc=%d\n", __func__, ret);
> + pr_debug("%s: returning rc=%d\n", __func__, rc);
>  
> - return ret;
> + return rc;
>  }
>  
>  /**
> -- 
> 2.1.0
>
> ___
> Linuxppc-dev mailing list
> linuxppc-...@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev


signature.asc
Description: PGP signature

Re: [PATCH v5 18/34] cxlflash: Fix AFU version access/storage and add check

[Daniel Axtens]

"Matthew R. Ochs"  writes:

> The AFU version is stored as a non-terminated string of bytes within
> a 64-bit little-endian register. Presently the value is read directly
> (no MMIO accessor) and is stored in a buffer that is not big enough
> to contain a NULL terminator. Additionally the version obtained is not
> evaluated against a known value to prevent usage with unsupported AFUs.
> All of these deficiencies can lead to a variety of problems.
>
> + if ((afu->interface_version + 1) == 0) {
> + pr_err("Back level AFU, please upgrade. AFU version %s "
> +"interface version 0x%llx\n", afu->version,
> +afu->interface_version);
> + rc = -EINVAL;
> + goto err1;

I'm confused by this if statement. If afu->interface_version + 1 == 0,
and interface_version is a 64bit unsigned int, that would mean that
afu->interface_version was 0x   .

Are you trying to check against all Fs? Is that value significant in the
hardware?

Regards,
Daniel



> + } else
> + pr_debug("%s: afu version %s, interface version 0x%llX\n",
> +  __func__, afu->version, afu->interface_version);
>  
>   rc = start_afu(cfg);
>   if (rc) {
> diff --git a/drivers/scsi/cxlflash/sislite.h b/drivers/scsi/cxlflash/sislite.h
> index 63bf394..8425d1a 100644
> --- a/drivers/scsi/cxlflash/sislite.h
> +++ b/drivers/scsi/cxlflash/sislite.h
> @@ -340,7 +340,7 @@ struct sisl_global_regs {
>  #define SISL_AFUCONF_MBOX_CLR_READ 0x0010ULL
>   __be64 afu_config;
>   __be64 rsvd[0xf8];
> - __be64 afu_version;
> + __le64 afu_version;
>   __be64 interface_version;
>  };
>  
> -- 
> 2.1.0
>
> ___
> Linuxppc-dev mailing list
> linuxppc-...@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 25/32] cxlflash: Fix to prevent EEH recovery failure

[Daniel Axtens]

"Matthew R. Ochs"  writes:

>>> The process_sense() routine can perform a read capacity which
>>> can take some time to complete. If an EEH occurs while waiting
>>> on the read capacity, the EEH handler is unable to obtain the
>>> context's mutex in order to put the context in an error state.
>>> The EEH handler will sit and wait until the context is free,
>>> but this wait can last longer than the EEH handler tolerates,
>>> leading to a failed recovery.
>> 
>> I'm not quite clear on what you mean by the EEH handler timing
>> out. AFAIK there's nothing in eehd and the EEH core that times out if a
>> driver doesn't respond - indeed, it's pretty easy to hang eehd with a
>> misbehaving driver.
>> 
>> Are you referring to your own internal timeouts?
>> cxlflash_wait_for_pci_err_recovery and anything else that uses
>> CXLFLASH_PCI_ERROR_RECOVERY_TIMEOUT?
>
> Reading through this again I can see how this is misleading. This is
> actually similar and related to the deadlock scenario described in
> "Fix to avoid potential deadlock on EEH". Without this fix, you'd end
> up in a similar situation but deadlocked on the context mutex instead
> of the ioctl semaphore.

That makes _much_ more sense. If you could please revise the commit
message to explain that, you can include this in the next version:
Reviewed-by: Daniel Axtens 

Regards,
Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 27/32] cxlflash: Fix to prevent stale AFU RRQ

[Daniel Axtens]

>>> Following an adapter reset, the AFU RRQ that resides in host memory
>>> holds stale data. This can lead to a condition where the RRQ interrupt
>>> handler tries to process stale entries and/or endlessly loops due to an
>>> out of sync generation bit.
>>> 
>>> To fix, the AFU RRQ in host memory needs to be cleared after each reset.
>> 
>> This looks good. Do you need anything to bail out of cxlflash_rrq_irq if
>> the data goes stale or to all Fs while that function is running?
>
> We're not performing an MMIO here, so I'm not sure how the all Fs check
> would apply. We're also protected fairly well by the generation bit. I suppose
> we could look at adding some type of 'max iterations' count to protect against
> a runaway handler but that would be in a future patch.

Ah, right you are. I had confused all Fs with UEs.

Reviewed-by: Daniel Axtens 

Regards,
Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 17/32] cxlflash: Remove dual port online dependency

[Daniel Axtens]

(resending to the list this time, apologies!)

>> I'm not sure I fully understand the flow of this function, but it looks
>> like you set rc=0 regardless of how things actually go: is this ever
>> going to print a return value other than zero?
>
> Correct, this function behaves more like a void for the time being. The
> overall goal of this is to allow a card to configure even when the link is
> down. At some later point when the link is transitioned to 'up', a link state
> change interrupt will trigger the port configuration. I left this with a 
> return
> code for right now in case we need to alter the behavior again (based
> upon testing) and actually return a value other than 0.

OK. That makes more sense - it wasn't clear to me how it could be
correct to proceed if the links were down but now I understand how that
works. I think that explanation should go in the commit message.

>>> if (!wait_port_online(fc_regs, FC_PORT_STATUS_RETRY_INTERVAL_US,
>>>   FC_PORT_STATUS_RETRY_CNT)) {
>>> pr_debug("%s: wait on port %d to go online timed out\n",
>>>  __func__, port);

As an aside, should this be a bit noisier? It seems like something
a user would probably want to know - especially in the case where
something has actually gone wrong so there's no link state change
interrupt forthcoming regardless of how long you wait.

Regards,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 32/32] cxlflash: Fix to avoid potential deadlock on EEH

[Daniel Axtens]

"Matthew R. Ochs"  writes:

> Ioctl threads that use scsi_execute() can run for an excessive amount
> of time due to the fact that they have lengthy timeouts and retry logic
> built in. Under normal operation this is not an issue. However, once EEH
> enters the picture, a long execution time coupled with the possibility
> that a timeout can trigger entry to the driver via registered reset
> callbacks becomes a liability.
>
> In particular, a deadlock can occur when an EEH event is encountered
> while in running in scsi_execute(). As part of the recovery, the EEH
> handler drains all currently running ioctls, waiting until they have
> completed before proceeding with a reset. As the scsi_execute()'s are
> situated on the ioctl path, the EEH handler will wait until they (and
> the remainder of the ioctl handler they're associated with) have
> completed. Normally this would not be much of an issue aside from the
> longer recovery period. Unfortunately, the scsi_execute() triggers a
> reset when it times out. The reset handler will see that the device is
> already being reset and wait until that reset completed. This creates
> a condition where the EEH handler becomes stuck, infinitely waiting for
> the ioctl thread to complete.
>
> To avoid this behavior, temporarily unmark the scsi_execute() threads
> as an ioctl thread by releasing the ioctl read semaphore. This allows
> the EEH handler to proceed with a recovery while the thread is still
> running. Once the scsi_execute() returns, the ioctl read semaphore is
> reacquired and the adapter state is rechecked in case it changed while
> inside of scsi_execute(). The state check will wait if the adapter is
> still being recovered or returns a failure if the recovery failed. In
> the event that the adapter reset failed, the failure is simply returned
> as the ioctl would be unable to continue.

Yep, looks good.

Reviewed-by: Daniel Axtens 

>
> Reported-by: Brian King 
> Signed-off-by: Matthew R. Ochs 
> Signed-off-by: Manoj N. Kumar 
> ---
>  drivers/scsi/cxlflash/superpipe.c | 30 +-
>  drivers/scsi/cxlflash/superpipe.h |  2 ++
>  drivers/scsi/cxlflash/vlun.c  | 29 +
>  3 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/cxlflash/superpipe.c 
> b/drivers/scsi/cxlflash/superpipe.c
> index f625e07..8af7cdc 100644
> --- a/drivers/scsi/cxlflash/superpipe.c
> +++ b/drivers/scsi/cxlflash/superpipe.c
> @@ -283,6 +283,24 @@ out:
>   * @sdev:SCSI device associated with LUN.
>   * @lli: LUN destined for capacity request.
>   *
> + * The READ_CAP16 can take quite a while to complete. Should an EEH occur 
> while
> + * in scsi_execute(), the EEH handler will attempt to recover. As part of the
> + * recovery, the handler drains all currently running ioctls, waiting until 
> they
> + * have completed before proceeding with a reset. As this routine is used on 
> the
> + * ioctl path, this can create a condition where the EEH handler becomes 
> stuck,
> + * infinitely waiting for this ioctl thread. To avoid this behavior, 
> temporarily
> + * unmark this thread as an ioctl thread by releasing the ioctl read 
> semaphore.
> + * This will allow the EEH handler to proceed with a recovery while this 
> thread
> + * is still running. Once the scsi_execute() returns, reacquire the ioctl 
> read
> + * semaphore and check the adapter state in case it changed while inside of
> + * scsi_execute(). The state check will wait if the adapter is still being
> + * recovered or return a failure if the recovery failed. In the event that 
> the
> + * adapter reset failed, simply return the failure as the ioctl would be 
> unable
> + * to continue.
> + *
> + * Note that the above puts a requirement on this routine to only be called 
> on
> + * an ioctl thread.
> + *
>   * Return: 0 on success, -errno on failure
>   */
>  static int read_cap16(struct scsi_device *sdev, struct llun_info *lli)
> @@ -314,8 +332,18 @@ retry:
>   dev_dbg(dev, "%s: %ssending cmd(0x%x)\n", __func__,
>   retry_cnt ? "re" : "", scsi_cmd[0]);
>  
> + /* Drop the ioctl read semahpore across lengthy call */
> + up_read(&cfg->ioctl_rwsem);
>   result = scsi_execute(sdev, scsi_cmd, DMA_FROM_DEVICE, cmd_buf,
> CMD_BUFSIZE, sense_buf, to, CMD_RETRIES, 0, NULL);
> + down_read(&cfg->ioctl_rwsem);
> + rc = check_state(cfg);
> + if (rc) {
> + dev_err(dev, "%s: Failed state! result=0x08%X\n",
> + __func__, result);
> + rc = -ENODEV;
> + goto out;
> +  

Re: [PATCH v4 30/32] cxlflash: Fix to avoid corrupting adapter fops

[Daniel Axtens]

"Matthew R. Ochs"  writes:

> The corruption that this fix remedies is due to the fact that the fops
> is initially defaulted to values found within a static structure. When
> the fops is handed down to the CXL services later in the attach path,
> certain services are patched. The fops structure remains correct until
> the user count drops to 0 and the fops is reset, triggering the process
> to repeat again. The user counts are tightly coupled with the creation
> and deletion of the user context. If multiple users perform a disk
> attach at the same time, when the user count is currently 0, some users
> can be in the middle of obtaining a file descriptor and have not yet
> reached the context creation code that [in addition to creating the
> context] increments the user count. Subsequent users coming in to
> perform the attach see that the user count is still 0, and reinitialize
> the fops, temporarily removing the patched fops. The users that are in
> the middle obtaining their file descriptor may then receive an invalid
> descriptor.
>
> The fix simply removes the user count altogether and moves the fops
> initialization to probe time such that it is only performed one time
> for the life of the adapter. In the future, if the CXL services adopt
> a private member for their context, that could be used to store the
> adapter structure reference and cxlflash could revert to a model that
> does not require an embedded fops.

Yep, this looks good.

We have discussed adding a private data field to a cxl context, and will
no doubt revisit the question at some point in the future :)

Reviewed-by: Daniel Axtens 

>
> Signed-off-by: Matthew R. Ochs 
> Signed-off-by: Manoj N. Kumar 
> ---
>  drivers/scsi/cxlflash/common.h|  3 +--
>  drivers/scsi/cxlflash/main.c  |  1 +
>  drivers/scsi/cxlflash/superpipe.c | 11 +--
>  3 files changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
> index bbfe711..c11cd19 100644
> --- a/drivers/scsi/cxlflash/common.h
> +++ b/drivers/scsi/cxlflash/common.h
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  
> +extern const struct file_operations cxlflash_cxl_fops;
>  
>  #define MAX_CONTEXT  CXLFLASH_MAX_CONTEXT   /* num contexts per afu */
>  
> @@ -115,8 +116,6 @@ struct cxlflash_cfg {
>   struct list_head ctx_err_recovery; /* contexts w/ recovery pending */
>   struct file_operations cxl_fops;
>  
> - atomic_t num_user_contexts;
> -
>   /* Parameters that are LUN table related */
>   int last_lun_index[CXLFLASH_NUM_FC_PORTS];
>   int promote_lun_index;
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index be78906..38e7edc 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -2386,6 +2386,7 @@ static int cxlflash_probe(struct pci_dev *pdev,
>  
>   cfg->init_state = INIT_STATE_NONE;
>   cfg->dev = pdev;
> + cfg->cxl_fops = cxlflash_cxl_fops;
>  
>   /*
>* The promoted LUNs move to the top of the LUN table. The rest stay
> diff --git a/drivers/scsi/cxlflash/superpipe.c 
> b/drivers/scsi/cxlflash/superpipe.c
> index 3cc8609..f625e07 100644
> --- a/drivers/scsi/cxlflash/superpipe.c
> +++ b/drivers/scsi/cxlflash/superpipe.c
> @@ -712,7 +712,6 @@ static void destroy_context(struct cxlflash_cfg *cfg,
>   kfree(ctxi->rht_needs_ws);
>   kfree(ctxi->rht_lun);
>   kfree(ctxi);
> - atomic_dec_if_positive(&cfg->num_user_contexts);
>  }
>  
>  /**
> @@ -769,7 +768,6 @@ static struct ctx_info *create_context(struct 
> cxlflash_cfg *cfg,
>   INIT_LIST_HEAD(&ctxi->luns);
>   INIT_LIST_HEAD(&ctxi->list); /* initialize for list_empty() */
>  
> - atomic_inc(&cfg->num_user_contexts);
>   mutex_lock(&ctxi->mutex);
>  out:
>   return ctxi;
> @@ -1164,10 +1162,7 @@ out:
>   return rc;
>  }
>  
> -/*
> - * Local fops for adapter file descriptor
> - */
> -static const struct file_operations cxlflash_cxl_fops = {
> +const struct file_operations cxlflash_cxl_fops = {
>   .owner = THIS_MODULE,
>   .mmap = cxlflash_cxl_mmap,
>   .release = cxlflash_cxl_release,
> @@ -1286,10 +1281,6 @@ static int cxlflash_disk_attach(struct scsi_device 
> *sdev,
>  
>   int fd = -1;
>  
> - /* On first attach set fileops */
> - if (atomic_read(&cfg->num_user_contexts) == 0)
> - cfg->cxl_fops = cxlflash_cxl_fops;
> -
>   if (attach->num_interrupts > 4) {
>   dev_dbg(dev, "%s: Cannot support this many interrupts %llu\n",
>   __func__, attach->num_interrupts);
> -- 
> 2.1.0


signature.asc
Description: PGP signature

Re: [PATCH v4 29/32] cxlflash: Fix to double the delay each time

[Daniel Axtens]

"Matthew R. Ochs"  writes:

>> On Sep 28, 2015, at 8:40 PM, Daniel Axtens  wrote:
>> 
>> -BEGIN PGP SIGNED MESSAGE-
>> Hash: SHA512
>> 
>> "Matthew R. Ochs"  writes:
>> 
>>> From: Manoj Kumar 
>>> 
>>> The operator used to double the delay is incorrect and
>>> does not result in delay doubling.
>>> 
>>> To fix, use a left shift instead of the XOR operator.
>>> 
>> I can see that the patch is correct, but this commit message is a bit
>> confusing. What delay? In what circumstances are you doubling it? Why?
>
> This is the response delay while resetting the master context. The reset
> is performed by writing a bit and then waiting for it to clear. While waiting
> for it to clear, the code relaxes the delta between MMIO reads.

OK. If you do a v5, please include this in the commit message.

Regards,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 29/32] cxlflash: Fix to double the delay each time

[Daniel Axtens]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

"Matthew R. Ochs"  writes:

> From: Manoj Kumar 
>
> The operator used to double the delay is incorrect and
> does not result in delay doubling.
>
> To fix, use a left shift instead of the XOR operator.
>
I can see that the patch is correct, but this commit message is a bit
confusing. What delay? In what circumstances are you doubling it? Why?

Regards,
Daniel

> Reported-by: Tomas Henzl 
> Signed-off-by: Matthew R. Ochs 
> Signed-off-by: Manoj N. Kumar 
> Reviewed-by: Brian King 
> ---
>  drivers/scsi/cxlflash/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index ab11ee6..be78906 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -303,7 +303,7 @@ write_rrin:
>   if (rrin != 0x1)
>   break;
>   /* Double delay each time */
> - udelay(2 ^ nretry);
> + udelay(2 << nretry);
>   } while (nretry++ < MC_ROOM_RETRY_CNT);
>  }
>  
> -- 
> 2.1.0
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: GPGTools - https://gpgtools.org

iQIcBAEBCgAGBQJWCewRAAoJEPC3R3P2I92F7xQQAKxvcnUpB/xLeq6CyB3LwsB/
cqFMP7OwDRfmQZG5ld5uqfXsjCCW4c0BaVknMgpD5gAdHbrwbc5gFgYuC0kIDUrH
aEwTyXwaH7+o82agjxf7TRrsh2NtLzdSB6NN1UNL73aPVXJFKWITajIKUAnYF5d1
AvB3yb4KtX0qklo205igVceuLbBBaXY6Lz3V58XDSG2JAtVlVkamJK7M8zySrNTY
Y627zTVkbopOGaBMLmu0q92dWDk2saU2wqKolo+pRy1NpOmY2XOU9hSRqCqAhnS4
xOWj7Bdrh1HdnQX26EC+f485nOijf0zddpJ6LsgPWMElJ04N4siQwmrwG55FbOs4
QDFKPdsVdZ8qmVueGCLRdTsMNAxcobi44P1/QXxGTmsn9xqyQBgZUzbd5oRUVswI
md21qir3xjRN7g4R78084I88dM1yrHpJ6NuHDxFeXbtrw8087lKgU05Ys6m+ebuC
7nPnYRJ54frAi6a7mmBq+Xc4IbVnaGl9IZi1gujM2mkTaAk7/wl7oRMYyUTKlblp
BeoQN0cdG1fChP1yLHzZV6G1JXwSw7nP6otf7YfUIrmjbzeZ5zWovSqJjnvNYBCT
zgW681gIF3GwaN9z9I+XY9BO1P6EkYp+dgAQM7PqdR00GjFjTJO0yn8WHh113lmE
L2sC0Nezuq8V4/qtspmh
=Km/z
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 27/32] cxlflash: Fix to prevent stale AFU RRQ

[Daniel Axtens]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

"Matthew R. Ochs"  writes:

> Following an adapter reset, the AFU RRQ that resides in host memory
> holds stale data. This can lead to a condition where the RRQ interrupt
> handler tries to process stale entries and/or endlessly loops due to an
> out of sync generation bit.
>
> To fix, the AFU RRQ in host memory needs to be cleared after each reset.

This looks good. Do you need anything to bail out of cxlflash_rrq_irq if
the data goes stale or to all Fs while that function is running?

Daniel

>
> Signed-off-by: Matthew R. Ochs 
> Signed-off-by: Manoj N. Kumar 
> Reviewed-by: Brian King 
> ---
>  drivers/scsi/cxlflash/main.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index 24aedfb..ab11ee6 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -1598,6 +1598,9 @@ static int start_afu(struct cxlflash_cfg *cfg)
>  
>   init_pcr(cfg);
>  
> + /* After an AFU reset, RRQ entries are stale, clear them */
> + memset(&afu->rrq_entry, 0, sizeof(afu->rrq_entry));
> +
>   /* Initialize RRQ pointers */
>   afu->hrrq_start = &afu->rrq_entry[0];
>   afu->hrrq_end = &afu->rrq_entry[NUM_RRQ_ENTRY - 1];
> -- 
> 2.1.0
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: GPGTools - https://gpgtools.org

iQIcBAEBCgAGBQJWCesgAAoJEPC3R3P2I92FE4kP+wYTEdCDP50RfKZPNCpoY//T
5tDNV58W1RasTrJLVGrpeGBboI4WTzRlKYAYX1iLC6ZSkX1rQS5rBqgImQQAg3i5
N9qoAdNCch06UVO7psceptT9653xt0OzitHW/tmB8pOMII65UIm8AtydSs2+J0e8
htw9Jnw098jls8NJtzz32T48QoM+kV1w7vfJJN36Z6FjL36KJLQCAL2/gLvv+3Zt
2ueeP6MM0q0iFxnQJwPFdf3DebggbMqeVbNorNAyCSkNNoUh2QR8NMHeFS8OoXCl
nrzjFoeSFDXh4c4bRzVoGRalIFzP+VY9guk9d/VtAt8ZkkrIyfI3K9lJTgDq2gqj
ShnT/WB4lskUVc/FjdkyIc+Nyg6pPkm4OftCnN7sf7CSd2+myO/d6S1f1qJlCn7s
3KBLXEFrKXKHl9TVDqkZSZVvyJOKmxoMqT98I00CWhXFkRgO126tnqsMsmrqgzxu
QxZQSo+/Oi7av6YuVtTP2c9SeYbGBfhOMOH5Dx7z9VY57rWvE5CVsduYJoG2D59P
Td5ToL1H3UOt8G1vzeqmkb7AWK882py2OLS7WG0PUDRDa2dfE6+8NOCJmuX9tW2x
/NnLlInDTiVTpz3iiJpe9Efu3SkL2KeMEBxi4jdt31hFO0BPeFWnZHKjwOa+uQ/w
OSaHCUZEZ6LRdbeJ1GLk
=O6Zk
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 25/32] cxlflash: Fix to prevent EEH recovery failure

[Daniel Axtens]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

"Matthew R. Ochs"  writes:


> The process_sense() routine can perform a read capacity which
> can take some time to complete. If an EEH occurs while waiting
> on the read capacity, the EEH handler is unable to obtain the
> context's mutex in order to put the context in an error state.
> The EEH handler will sit and wait until the context is free,
> but this wait can last longer than the EEH handler tolerates,
> leading to a failed recovery.

I'm not quite clear on what you mean by the EEH handler timing
out. AFAIK there's nothing in eehd and the EEH core that times out if a
driver doesn't respond - indeed, it's pretty easy to hang eehd with a
misbehaving driver.

Are you referring to your own internal timeouts?
cxlflash_wait_for_pci_err_recovery and anything else that uses
CXLFLASH_PCI_ERROR_RECOVERY_TIMEOUT?

Regards,
Daniel

>
> To address this issue, make the context unavailable to new,
> non-system owned threads and release the context while calling
> into process_sense(). After returning from process_sense() the
> context mutex is reacquired and the context is made available
> again. The context can be safely moved to the error state if
> needed during the unavailable window as no other threads will
> hold its reference.
>
> Signed-off-by: Matthew R. Ochs 
> Signed-off-by: Manoj N. Kumar 
> Reviewed-by: Brian King 
> ---
>  drivers/scsi/cxlflash/superpipe.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/scsi/cxlflash/superpipe.c 
> b/drivers/scsi/cxlflash/superpipe.c
> index a6316f5..7283e83 100644
> --- a/drivers/scsi/cxlflash/superpipe.c
> +++ b/drivers/scsi/cxlflash/superpipe.c
> @@ -1787,12 +1787,21 @@ static int cxlflash_disk_verify(struct scsi_device 
> *sdev,
>* inquiry (i.e. the Unit attention is due to the WWN changing).
>*/
>   if (verify->hint & DK_CXLFLASH_VERIFY_HINT_SENSE) {
> + /* Can't hold mutex across process_sense/read_cap16,
> +  * since we could have an intervening EEH event.
> +  */
> + ctxi->unavail = true;
> + mutex_unlock(&ctxi->mutex);
>   rc = process_sense(sdev, verify);
>   if (unlikely(rc)) {
>   dev_err(dev, "%s: Failed to validate sense data (%d)\n",
>   __func__, rc);
> + mutex_lock(&ctxi->mutex);
> + ctxi->unavail = false;
>   goto out;
>   }
> + mutex_lock(&ctxi->mutex);
> + ctxi->unavail = false;
>   }
>  
>   switch (gli->mode) {
> -- 
> 2.1.0
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: GPGTools - https://gpgtools.org

iQIcBAEBCgAGBQJWCeieAAoJEPC3R3P2I92F+hMP/1OdLQCin+kKbOb9qxf952bH
DUAkmEhc0oD7xZFQI8HgDmHRxpes5HHxXtwXFsLgsr8QYG+aOIV568GXIZtTbrl0
aCFMqtKXZ6jVqv5L60r1tgzcWxmWdshMLd1op6t3BwA67nUc5Edcr94ePUyDDLj1
at335wCnxuGxn0kdB0Ud/lbPzTsgDPcuV6tCLy0o4J15KFOyFt9hCjO4nmL/wcIt
kmjyn5SHbdgje+73uaRQnXkli4wDA9x7x6/8wFgLspnOxgMEJgnHmm+HYbOXnHyX
nFFHw9+X2ETUcucVWuKNaFzW1vH+WJDteEZbjS7t7liJIkmIiZSFHyUTtVGdBkl1
FsWswA0pkzuGq94Wsb0nGtNHbsMw+WeWTcTlNN46DMG/wqz75iO3yMGK9MZuddSX
9jUokiM0kQvvfwAoujmvpMCVB4b2oseRRG4/yJ0lKSCcC8kETQTXgVHbT8oLmCdk
rUA0hxbbKzVQsDzw8s5HqYZjqHdLp3sDPeyukPeJl2CNhysrmnyHXpq8XgcLi3op
kbuuiR3z8UH3MW4BDpplnjhZ+5Wyw9cSI57vRF2Kr80NnU+5hBvftNh4rBneeny2
0gCDlPHDvB7Ks9HkcxkK9MW78FTgj50ePofS/dUUod4M9ohDd4MSwRKjpwQ+H3By
jmxnzfvWO/oTlL1D9+2W
=3BcU
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 17/32] cxlflash: Remove dual port online dependency

[Daniel Axtens]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

Hi,

>  static int afu_set_wwpn(struct afu *afu, int port, u64 *fc_regs, u64 wwpn)
>  {
> - int ret = 0;
> + int rc = 0;
I realise it's nice to have things consistent, but making this change
now makes the rest of the patch quite difficult to follow.

>  
>   set_port_offline(fc_regs);
>  
> @@ -1038,33 +1038,26 @@ static int afu_set_wwpn(struct afu *afu, int port, 
> u64 *fc_regs, u64 wwpn)
>  FC_PORT_STATUS_RETRY_CNT)) {
>   pr_debug("%s: wait on port %d to go offline timed out\n",
>__func__, port);
> - ret = -1; /* but continue on to leave the port back online */
> + rc = -1; /* but continue on to leave the port back online */
>   }
>  
> - if (ret == 0)
> + if (rc == 0)
>   writeq_be(wwpn, &fc_regs[FC_PNAME / 8]);
>  
> + /* Always return success after programming WWPN */
> + rc = 0;
> +
>   set_port_online(fc_regs);
>  
>   if (!wait_port_online(fc_regs, FC_PORT_STATUS_RETRY_INTERVAL_US,
> FC_PORT_STATUS_RETRY_CNT)) {
>   pr_debug("%s: wait on port %d to go online timed out\n",
>__func__, port);
> - ret = -1;
> -
> - /*
> -  * Override for internal lun!!!
> -  */
> - if (afu->internal_lun) {
> - pr_debug("%s: Overriding port %d online timeout!!!\n",
> -  __func__, port);
> - ret = 0;
> - }
>   }
>  
> - pr_debug("%s: returning rc=%d\n", __func__, ret);
> + pr_debug("%s: returning rc=%d\n", __func__, rc);
I'm not sure I fully understand the flow of this function, but it looks
like you set rc=0 regardless of how things actually go: is this ever
going to print a return value other than zero?

Regards,
Daniel
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: GPGTools - https://gpgtools.org

iQIcBAEBCgAGBQJWCc87AAoJEPC3R3P2I92FQNcP/RF+M8MGZ2PJ8heh98D84rb5
Dx+Yq1czRJ+YZbK5tCfkyU66KspEzM7IIOiiXDLlBZ+AjcQtCUopTNMbL/UN+oVT
5lWrvPZlWRJqRN5bA/RA3i/DSBRucucmP8n4pmTKqsMMqKwzk/f3sE+Uo5oAzS+y
JaSywxm+Vd4dkW5T94kc6TXCeWcaD47tG0mgg0jHGwFtOioDEeWgf7Kie52+RV+o
I6z7GlQj9dgcKs2NmVr67AoY1dfRYl1ZvvJN7bYoLbHnEgiSw1d6XZK/2cqHzIpE
S1KEHOyuSZJh8Txwfg6oJ3sbpFZaurSIXDXfOhWuJ90OrOu4hgeODTPX/3o2CKae
K+WhsL6XOhrxyMhfq/VWplF6Hjo7VqLcT9e0sYZ4YNkUJrGAza3iPOqngK9zmdsM
80HLJdbsiZMkl+i55IOuisckCtvjUtVE+bDlzau6vwgBlgZ9DKByPPmqJGjS9I3L
vCEKsRZryaSvaYSnK46kpqXsukN/+QMefXL25IfTf4wQQaV4O+mSJxkkLXPAKqfd
cvCFg08MyAQS+YyNMBdFDJyj7tWVclGZhJkqlyjPjQ2YrFA5tQ7MoqY05NomxY9Q
xo0JuaceNccFetKPg1LMmTp5Ag/2DCcnGq/0Z3ioGVJTFIVil0BnWIFctlGbquya
n4Ylfe3h1T6hWJ7bjxwF
=cZRI
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 09/32] cxlflash: Correct naming of limbo state and waitq

[Daniel Axtens]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

"Matthew R. Ochs"  writes:

Looks good from an EEH point of view: in an error situation, your driver
asks to be reset and then is waiting for CXL and EEH to carry that out,
so 'reset' matches with that as well.

Reviewed-by: Daniel Axtens 

> Limbo is not an accurate representation of this state and is
> also not consistent with the terminology that other drivers
> use to represent this concept. Rename the state and and its
> associated waitq to 'reset'.
>
> Signed-off-by: Matthew R. Ochs 
> Signed-off-by: Manoj N. Kumar 
> Reviewed-by: Brian King 
> ---
>  drivers/scsi/cxlflash/common.h|  4 ++--
>  drivers/scsi/cxlflash/main.c  | 26 +-
>  drivers/scsi/cxlflash/superpipe.c | 14 +++---
>  3 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
> index 1abe4e0..11318de 100644
> --- a/drivers/scsi/cxlflash/common.h
> +++ b/drivers/scsi/cxlflash/common.h
> @@ -79,7 +79,7 @@ enum cxlflash_init_state {
>  
>  enum cxlflash_state {
>   STATE_NORMAL,   /* Normal running state, everything good */
> - STATE_LIMBO,/* Limbo running state, trying to reset/recover */
> + STATE_RESET,/* Reset state, trying to reset/recover */
>   STATE_FAILTERM  /* Failed/terminating state, error out users/threads */
>  };
>  
> @@ -125,7 +125,7 @@ struct cxlflash_cfg {
>  
>   wait_queue_head_t tmf_waitq;
>   bool tmf_active;
> - wait_queue_head_t limbo_waitq;
> + wait_queue_head_t reset_waitq;
>   enum cxlflash_state state;
>  };
>  
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index 6e85c77..8940336 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -382,8 +382,8 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
> struct scsi_cmnd *scp)
>   spin_unlock_irqrestore(&cfg->tmf_waitq.lock, lock_flags);
>  
>   switch (cfg->state) {
> - case STATE_LIMBO:
> - dev_dbg_ratelimited(&cfg->dev->dev, "%s: device in limbo!\n",
> + case STATE_RESET:
> + dev_dbg_ratelimited(&cfg->dev->dev, "%s: device is in reset!\n",
>   __func__);
>   rc = SCSI_MLQUEUE_HOST_BUSY;
>   goto out;
> @@ -479,8 +479,8 @@ static int cxlflash_eh_device_reset_handler(struct 
> scsi_cmnd *scp)
>   if (unlikely(rcr))
>   rc = FAILED;
>   break;
> - case STATE_LIMBO:
> - wait_event(cfg->limbo_waitq, cfg->state != STATE_LIMBO);
> + case STATE_RESET:
> + wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);
>   if (cfg->state == STATE_NORMAL)
>   break;
>   /* fall through */
> @@ -519,7 +519,7 @@ static int cxlflash_eh_host_reset_handler(struct 
> scsi_cmnd *scp)
>  
>   switch (cfg->state) {
>   case STATE_NORMAL:
> - cfg->state = STATE_LIMBO;
> + cfg->state = STATE_RESET;
>   scsi_block_requests(cfg->host);
>   cxlflash_mark_contexts_error(cfg);
>   rcr = cxlflash_afu_reset(cfg);
> @@ -528,11 +528,11 @@ static int cxlflash_eh_host_reset_handler(struct 
> scsi_cmnd *scp)
>   cfg->state = STATE_FAILTERM;
>   } else
>   cfg->state = STATE_NORMAL;
> - wake_up_all(&cfg->limbo_waitq);
> + wake_up_all(&cfg->reset_waitq);
>   scsi_unblock_requests(cfg->host);
>   break;
> - case STATE_LIMBO:
> - wait_event(cfg->limbo_waitq, cfg->state != STATE_LIMBO);
> + case STATE_RESET:
> + wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);
>   if (cfg->state == STATE_NORMAL)
>   break;
>   /* fall through */
> @@ -705,7 +705,7 @@ static void cxlflash_wait_for_pci_err_recovery(struct 
> cxlflash_cfg *cfg)
>   struct pci_dev *pdev = cfg->dev;
>  
>   if (pci_channel_offline(pdev))
> - wait_event_timeout(cfg->limbo_waitq,
> + wait_event_timeout(cfg->reset_waitq,
>  !pci_channel_offline(pdev),
>  CXLFLASH_PCI_ERROR_RECOVERY_TIMEOUT);
>  }
> @@ -2304,7 +2304,7 @@ static int cxlflash_probe(struct pci_dev *pdev,
>   cfg->mcctx = NULL;
>  
>   init_waitqueue_head(&cfg->tmf_waitq);
> -

Re: [PATCH v4 08/32] cxlflash: Fix to avoid CXL services during EEH

[Daniel Axtens]

You have two versions of check_state() below, which is a bit
confusing. It looks like you've moved the function and also added the
up/down of the read semaphore. I assume that's all that changed?

>  
>  /**
> + * check_state() - checks and responds to the current adapter state
> + * @cfg: Internal structure associated with the host.
> + *
> + * This routine can block and should only be used on process context.
> + * It assumes that the caller is an ioctl thread and holding the ioctl
> + * read semaphore. This is temporarily let up across the wait to allow
> + * for draining actively running ioctls. Also note that when waking up
> + * from waiting in reset, the state is unknown and must be checked again
> + * before proceeding.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +static int check_state(struct cxlflash_cfg *cfg)
> +{
> + struct device *dev = &cfg->dev->dev;
> + int rc = 0;
> +
> +retry:
> + switch (cfg->state) {
> + case STATE_LIMBO:
> + dev_dbg(dev, "%s: Limbo state, going to wait...\n", __func__);
> + up_read(&cfg->ioctl_rwsem);
> + rc = wait_event_interruptible(cfg->limbo_waitq,
> +   cfg->state != STATE_LIMBO);
> + down_read(&cfg->ioctl_rwsem);
> + if (unlikely(rc))
> + break;
> + goto retry;
> + case STATE_FAILTERM:
> + dev_dbg(dev, "%s: Failed/Terminating!\n", __func__);
> + rc = -ENODEV;
> + break;
> + default:
> + break;
> + }
> +
> + return rc;
> +}
> +
> +/**
>   * cxlflash_disk_attach() - attach a LUN to a context
>   * @sdev:SCSI device associated with LUN.
>   * @attach:  Attach ioctl data structure.
> @@ -1523,41 +1563,6 @@ err1:
>  }
>  
>  /**
> - * check_state() - checks and responds to the current adapter state
> - * @cfg: Internal structure associated with the host.
> - *
> - * This routine can block and should only be used on process context.
> - * Note that when waking up from waiting in limbo, the state is unknown
> - * and must be checked again before proceeding.
> - *
> - * Return: 0 on success, -errno on failure
> - */
> -static int check_state(struct cxlflash_cfg *cfg)
> -{
> - struct device *dev = &cfg->dev->dev;
> - int rc = 0;
> -
> -retry:
> - switch (cfg->state) {
> - case STATE_LIMBO:
> - dev_dbg(dev, "%s: Limbo, going to wait...\n", __func__);
> - rc = wait_event_interruptible(cfg->limbo_waitq,
> -   cfg->state != STATE_LIMBO);
> - if (unlikely(rc))
> - break;
> - goto retry;
> - case STATE_FAILTERM:
> - dev_dbg(dev, "%s: Failed/Terminating!\n", __func__);
> - rc = -ENODEV;
> - break;
> - default:
> - break;
> - }
> -
> - return rc;
> -}
> -
> -/**
>   * cxlflash_afu_recover() - initiates AFU recovery
>   * @sdev:SCSI device associated with LUN.
>   * @recover: Recover ioctl data structure.
> @@ -1646,9 +1651,14 @@ retry_recover:
>   /* Test if in error state */
>   reg = readq_be(&afu->ctrl_map->mbox_r);
>   if (reg == -1) {
> - dev_dbg(dev, "%s: MMIO read fail! Wait for recovery...\n",
> - __func__);
> - mutex_unlock(&ctxi->mutex);
> + dev_dbg(dev, "%s: MMIO fail, wait for recovery.\n", __func__);
> +
> + /*
> +  * Before checking the state, put back the context obtained with
> +  * get_context() as it is no longer needed and sleep for a short
> +  * period of time (see prolog notes).
> +  */
> + put_context(ctxi);

Is this needed for the drain to work? It looks like it fixes a
refcounting bug in the function, but I'm not sure I understand how it
interacts with the rest of this patch.

Anyway, the patch overall looks good to me, and makes your driver
interact with CXL's EEH support in the way I intended when I wrote it.

Reviewed-by: Daniel Axtens 

Regards,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 07/32] cxlflash: Fix context encode mask width

[Daniel Axtens]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512


Looks good to me.

Reviewed-by: Daniel Axtens 

Regards,
Daniel

"Matthew R. Ochs"  writes:

> The context encode mask covers more than 32-bits, making it
> a long integer. This should be noted by appending the ULL
> width suffix to the mask.
>
> Signed-off-by: Matthew R. Ochs 
> Signed-off-by: Manoj N. Kumar 
> Reviewed-by: Brian King 
> ---
>  drivers/scsi/cxlflash/superpipe.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/cxlflash/superpipe.h 
> b/drivers/scsi/cxlflash/superpipe.h
> index 72d53cf..7947091 100644
> --- a/drivers/scsi/cxlflash/superpipe.h
> +++ b/drivers/scsi/cxlflash/superpipe.h
> @@ -87,7 +87,7 @@ enum ctx_ctrl {
>   CTX_CTRL_FILE   = (1 << 5)
>  };
>  
> -#define ENCODE_CTXID(_ctx, _id)  (u64)_ctx) & 0x0) << 28) | 
> _id)
> +#define ENCODE_CTXID(_ctx, _id)  (u64)_ctx) & 0x0ULL) << 28) 
> | _id)
>  #define DECODE_CTXID(_val)   (_val & 0x)
>  
>  struct ctx_info {
> -- 
> 2.1.0
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: GPGTools - https://gpgtools.org

iQIcBAEBCgAGBQJWCcGLAAoJEPC3R3P2I92FJ1kP/juiS2a5hBDOGjVliW6KlWK6
pb9yRZ1qmvgiL1xtMO/rfSEEb/kwO6QSbDcKqLtKwI24DBkVF2eDAOo9mOKKNhBI
EF/AwZsvG1DE3o0+AAx3mY7+AO1rP+uZSvCJf5GvDDdSfYXfq5f49+aM6HJ0n130
ewlQmOnwwGDpO2gehccrDSmSbUB1CwKY1uQ5KtczGqdtfkgSu7YVRHKiNiid9Nop
uJWDE452NBm2dM5Jf2hXFXwMSzx9bbmM5Ue27gf1HES8CGw3Z2BQtYIx6tjHLYH/
Rj4dPSLBjuBeEI4YZHqxkYs3ipBV38wRwuG0O5o+YKUD6TczeYtwwCVAmma8pTm2
tYT8xrY19Ng3oQ1P68zxP4D5uYbdFhIOyqLhss9Lnas0VxzZUbdfiuF4V8ABSGTz
L8WFw2ff/73LtVaSr/KGyt1RvfLpoEFxkYd3+sGwsfqM/HyC2K3g4RZucLct1oeX
PPPKz5ZQF9K/R/mKJEIAG+Fh0usryj97Wgf+r3Ie0QQDorOoG+8itGg0OJ0o1ojV
wjduJiwsoD+2HwnTw85TwSp6PPY+8o7Gml1PvfGUFWJjPgSUkcQ6L3lrispE4twt
hgrWSsRE/GO5+JD9KNSYnKr8dRxD6iDFefppZv64nWJKi11PZoz7z09Q+dE1KAng
jpNUi5wyGph2AwD7E9I2
=M1UV
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 06/32] cxlflash: Fix to avoid sizeof(bool)

[Daniel Axtens]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512


Looks good to me.

Reviewed-by: Daniel Axtens 

Regards,
Daniel

"Matthew R. Ochs"  writes:

> Using sizeof(bool) is considered poor form for various reasons and
> sparse warns us of that. Correct by changing type from bool to u8.
>
> Signed-off-by: Matthew R. Ochs 
> Signed-off-by: Manoj N. Kumar 
> Reviewed-by: Brian King 
> ---
>  drivers/scsi/cxlflash/superpipe.c | 2 +-
>  drivers/scsi/cxlflash/superpipe.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/cxlflash/superpipe.c 
> b/drivers/scsi/cxlflash/superpipe.c
> index ffa68cc..28aa9d9 100644
> --- a/drivers/scsi/cxlflash/superpipe.c
> +++ b/drivers/scsi/cxlflash/superpipe.c
> @@ -737,7 +737,7 @@ static struct ctx_info *create_context(struct 
> cxlflash_cfg *cfg,
>   struct afu *afu = cfg->afu;
>   struct ctx_info *ctxi = NULL;
>   struct llun_info **lli = NULL;
> - bool *ws = NULL;
> + u8 *ws = NULL;
>   struct sisl_rht_entry *rhte;
>  
>   ctxi = kzalloc(sizeof(*ctxi), GFP_KERNEL);
> diff --git a/drivers/scsi/cxlflash/superpipe.h 
> b/drivers/scsi/cxlflash/superpipe.h
> index fffb179..72d53cf 100644
> --- a/drivers/scsi/cxlflash/superpipe.h
> +++ b/drivers/scsi/cxlflash/superpipe.h
> @@ -97,7 +97,7 @@ struct ctx_info {
>   u32 rht_out;/* Number of checked out RHT entries */
>   u32 rht_perms;  /* User-defined permissions for RHT entries */
>   struct llun_info **rht_lun;   /* Mapping of RHT entries to LUNs */
> - bool *rht_needs_ws; /* User-desired write-same function per RHTE */
> + u8 *rht_needs_ws;   /* User-desired write-same function per RHTE */
>  
>   struct cxl_ioctl_start_work work;
>   u64 ctxid;
> -- 
> 2.1.0
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: GPGTools - https://gpgtools.org

iQIcBAEBCgAGBQJWCcCzAAoJEPC3R3P2I92FisQP/1Gd443NSLYkye19i4D7qlA/
3tfNTBBSYL68A1L1ssgRxtpOkqb80hjOSXZhR2SDLRGNXDoC5+8fwti/sF8wxewF
rxIsliMN2WnqgyYpw4qt7T6AMZtSyP1yXQkPWZcVQi5a9p1w3YmLdvwr9hnyKx8n
TRjEe3OoDS/dde8l+4C1YKCL/U4oDuGdH7DLT+vhn49TfxQL4aNiV52Wu5l3Sptw
f4/VVXAklNkU6yHJiRonjf/GJggfcgKUmsy4y8ayvkNuMu2V+ffP9AbdwdZXrplK
CempBE0AvQdjLS0b8oIErOHfyWJcagHhMZicblLT5xp4kR0mDsIybEI433cGjESI
4ECg2AjKRoBEu6s8fR3WFR82nZo97o0ck2pAD4RVJTA/aY7h1DlZ45kg2cYEatRW
OVnQqd//bHKyWljWzl7qXuwpXF8G92iP4CFQ4d8aruUFp9vwFlR8GuIpsq69kb5X
FM6EuTzcbD+GkT+P9lw2T/6LHrb+2kTANHJgWKalfugJxAwcDps6X7tKxQ9mvqps
HyVVfOtNkK2yvAV8iEqPdT9ltXIhJ/g9c9ZtQ7F2Qg7pxnB15M5ykOIeVVsRIEof
F1HAG8xzPNY0Q7MBHDixqQ7IbHDnbkM+SI9outZu5C0ECjc3UwdfOaegm+A6eWs9
3bv8aFdbQlHfjSFUIhqY
=RI1B
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/10] scsi: ufs-exynos: add UFS host support for Exynos SoCs

[amit daniel kachhap]

On Fri, Aug 21, 2015 at 2:58 PM, Alim Akhtar  wrote:
> From: Seungwon Jeon 
>
> This patch introduces Exynos UFS host controller driver,
> which mainly handles vendor-specific operations including
> link startup, power mode change and hibernation/unhibernation.
>
> Signed-off-by: Seungwon Jeon 
> Signed-off-by: Alim Akhtar 
> ---
>  .../devicetree/bindings/ufs/ufs-exynos.txt |   92 ++
>  drivers/scsi/ufs/Kconfig   |   12 +
>  drivers/scsi/ufs/Makefile  |1 +
>  drivers/scsi/ufs/ufs-exynos-hw.c   |  147 +++
>  drivers/scsi/ufs/ufs-exynos-hw.h   |   43 +
>  drivers/scsi/ufs/ufs-exynos.c  | 1175 
> 
>  drivers/scsi/ufs/ufs-exynos.h  |  463 
>  drivers/scsi/ufs/ufshci.h  |   26 +-
>  drivers/scsi/ufs/unipro.h  |   47 +
>  9 files changed, 2005 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/ufs/ufs-exynos.txt
>  create mode 100644 drivers/scsi/ufs/ufs-exynos-hw.c
>  create mode 100644 drivers/scsi/ufs/ufs-exynos-hw.h
>  create mode 100644 drivers/scsi/ufs/ufs-exynos.c
>  create mode 100644 drivers/scsi/ufs/ufs-exynos.h
>
> diff --git a/Documentation/devicetree/bindings/ufs/ufs-exynos.txt 
> b/Documentation/devicetree/bindings/ufs/ufs-exynos.txt
> new file mode 100644
> index 000..1a6184d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ufs/ufs-exynos.txt
> @@ -0,0 +1,92 @@
> +* Exynos Universal Flash Storage (UFS) Host Controller
> +
> +UFSHC nodes are defined to describe on-chip UFS host controllers.
> +Each UFS controller instance should have its own node.
> +
> +Required properties:
> +- compatible: compatible list, contains "samsung,exynos7-ufs"
> +- interrupts: 
> +- reg   : 
> +
> +Optional properties:
> +- vdd-hba-supply: phandle to UFS host controller supply regulator 
> node
> +- vcc-supply: phandle to VCC supply regulator node
> +- vccq-supply   : phandle to VCCQ supply regulator node
> +- vccq2-supply  : phandle to VCCQ2 supply regulator node
> +- vcc-supply-1p8: For embedded UFS devices, valid VCC range is 
> 1.7-1.95V
> +  or 2.7-3.6V. This boolean property when set, 
> specifies
> + to use low voltage range of 1.7-1.95V. Note for 
> external
> + UFS cards this property is invalid and valid VCC 
> range is
> + always 2.7-3.6V.
> +- vcc-max-microamp  : specifies max. load that can be drawn from vcc 
> supply
> +- vccq-max-microamp : specifies max. load that can be drawn from vccq 
> supply
> +- vccq2-max-microamp: specifies max. load that can be drawn from vccq2 
> supply
> +- -fixed-regulator : boolean property specifying that -supply is 
> a fixed regulator
> +
> +- clocks: List of phandle and clock specifier pairs
> +- clock-names   : List of clock input name strings sorted in the same
> +  order as the clocks property.
> +- freq-table-hz: Array of  operating frequencies 
> stored in the same
> +  order as the clocks property. If this property is 
> not
> + defined or a value in the array is "0" then it is 
> assumed
> + that the frequency is set by the parent clock or a
> + fixed rate clock source.
> +- pclk-freq-avail-range : specifies available frequency range(min/max) for 
> APB clock
> +- ufs,pwr-attr-mode : specifies mode value for power mode change
> +- ufs,pwr-attr-lane : specifies lane count value for power mode change
> +- ufs,pwr-attr-gear : specifies gear count value for power mode change
> +- ufs,pwr-attr-hs-series : specifies HS rate series for power mode change
> +- ufs,pwr-local-l2-timer : specifies array of local UNIPRO L2 timer values
> +   AFC0ReqTimeOutVal>
> +- ufs,pwr-remote-l2-timer : specifies array of remote UNIPR L2 timer values
s/UNIPR/UNIPRO
> +   AFC0ReqTimeOutVal>
> +- ufs-rx-adv-fine-gran-sup_en : specifies support of fine granularity of MPHY
I suppose this field is bool type. This can be mentioned here.
> +- ufs-rx-adv-fine-gran-step : specifies granularity steps of MPHY
> +- ufs-rx-adv-min-activate-time-cap : specifies rx advanced minimum activate 
> time of MPHY
> +- ufs-pa-granularity : specifies Granularity for PA_TActivate and 
> PA_Hibern8Time
> +- ufs-pa-tacctivate : specifies time wake-up remote M-RX
> +- ufs-pa-hibern8time : specifies minimum time to wait in HIBERN8 state
> +
> +Note: If above properties are not defined it can be assumed that the supply
> +regulators or clocks are always on.
> +
> +Example:
> +   ufshc@0x1557 {
> +   compatible = "samsung,exynos7-ufs";
> +   reg = <0xfc59800

Re: [PATCH 09/10] scsi: ufs: return value of pwr_change_notify

[amit daniel kachhap]

On Fri, Aug 21, 2015 at 2:58 PM, Alim Akhtar  wrote:
> From: Seungwon Jeon 
>
> Behavior of the "powwer mode change" contains vendor specific
s/powwer/power
> operation known as pwr_change_notify. This change adds return
> for pwr_change_notify to find success or failure.
>
> Signed-off-by: Seungwon Jeon 
> Signed-off-by: Alim Akhtar 
> ---
>  drivers/scsi/ufs/ufshcd.c |   22 +++---
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 8982da9..142a927 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -2579,14 +2579,18 @@ static int ufshcd_change_power_mode(struct ufs_hba 
> *hba,
> dev_err(hba->dev,
> "%s: power mode change failed %d\n", __func__, ret);
> } else {
> -   if (hba->vops && hba->vops->pwr_change_notify)
> -   hba->vops->pwr_change_notify(hba,
> -   POST_CHANGE, NULL, pwr_mode);
> +   if (hba->vops && hba->vops->pwr_change_notify) {
> +   ret = hba->vops->pwr_change_notify(hba,
> +   POST_CHANGE, NULL, pwr_mode);
> +   if (ret)
> +   goto out;
> +   }
>
> memcpy(&hba->pwr_info, pwr_mode,
> sizeof(struct ufs_pa_layer_attr));
> }
>
> +out:
> return ret;
>  }
>
> @@ -2601,14 +2605,18 @@ int ufshcd_config_pwr_mode(struct ufs_hba *hba,
> struct ufs_pa_layer_attr final_params = { 0 };
> int ret;
>
> -   if (hba->vops && hba->vops->pwr_change_notify)
> -   hba->vops->pwr_change_notify(hba,
> -PRE_CHANGE, desired_pwr_mode, &final_params);
> -   else
> +   if (hba->vops && hba->vops->pwr_change_notify) {
> +   ret = hba->vops->pwr_change_notify(hba,
> +   PRE_CHANGE, desired_pwr_mode, &final_params);
> +   if (ret)
> +   goto out;
> +   } else {
> memcpy(&final_params, desired_pwr_mode, sizeof(final_params));
> +   }
>
> ret = ufshcd_change_power_mode(hba, &final_params);
>
> +out:
> return ret;
>  }
>  EXPORT_SYMBOL_GPL(ufshcd_config_pwr_mode);
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/10] scsi: ufs: make ufshcd_config_pwr_mode of non-static func

[amit daniel kachhap]

On Fri, Aug 21, 2015 at 2:57 PM, Alim Akhtar  wrote:
> From: Seungwon Jeon 
>
> It can be used in the vendor's driver for the specific purpose.
more description of this log will be useful.
>
> Signed-off-by: Seungwon Jeon 
> Signed-off-by: Alim Akhtar 
> ---
>  drivers/scsi/ufs/ufshcd.c |5 ++---
>  drivers/scsi/ufs/ufshcd.h |2 ++
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index d425ea1..8982da9 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -185,8 +185,6 @@ static int ufshcd_uic_hibern8_ctrl(struct ufs_hba *hba, 
> bool en);
>  static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
>  static int ufshcd_host_reset_and_restore(struct ufs_hba *hba);
>  static irqreturn_t ufshcd_intr(int irq, void *__hba);
> -static int ufshcd_config_pwr_mode(struct ufs_hba *hba,
> -   struct ufs_pa_layer_attr *desired_pwr_mode);
>  static int ufshcd_change_power_mode(struct ufs_hba *hba,
>  struct ufs_pa_layer_attr *pwr_mode);
>
> @@ -2597,7 +2595,7 @@ static int ufshcd_change_power_mode(struct ufs_hba *hba,
>   * @hba: per-adapter instance
>   * @desired_pwr_mode: desired power configuration
>   */
> -static int ufshcd_config_pwr_mode(struct ufs_hba *hba,
> +int ufshcd_config_pwr_mode(struct ufs_hba *hba,
> struct ufs_pa_layer_attr *desired_pwr_mode)
>  {
> struct ufs_pa_layer_attr final_params = { 0 };
> @@ -2613,6 +2611,7 @@ static int ufshcd_config_pwr_mode(struct ufs_hba *hba,
>
> return ret;
>  }
> +EXPORT_SYMBOL_GPL(ufshcd_config_pwr_mode);
>
>  /**
>   * ufshcd_complete_dev_init() - checks device readiness
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 045968e..13368e1 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -636,6 +636,8 @@ extern int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 
> attr_sel,
>u8 attr_set, u32 mib_val, u8 peer);
>  extern int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
>u32 *mib_val, u8 peer);
> +extern int ufshcd_config_pwr_mode(struct ufs_hba *hba,
> +   struct ufs_pa_layer_attr *desired_pwr_mode);
>
>  /* UIC command interfaces for DME primitives */
>  #define DME_LOCAL  0
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/10] scsi: ufs: add add specific callback for hibern8

[amit daniel kachhap]

On Fri, Aug 21, 2015 at 2:57 PM, Alim Akhtar  wrote:
> From: Seungwon Jeon 
>
> Some host controller needs specific handling before/after
> (un)hibernation, This change adds specific callback function
> to support vendor's implementation.
>
> Signed-off-by: Seungwon Jeon 
> Signed-off-by: Alim Akhtar 
> ---
>  drivers/scsi/ufs/ufshcd.c |   36 
>  drivers/scsi/ufs/ufshcd.h |3 +++
>  2 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index bc27f5e..d425ea1 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -181,8 +181,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba);
>  static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
>  bool skip_ref_clk);
>  static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
> -static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba);
> -static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba);
> +static int ufshcd_uic_hibern8_ctrl(struct ufs_hba *hba, bool en);
>  static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
>  static int ufshcd_host_reset_and_restore(struct ufs_hba *hba);
>  static irqreturn_t ufshcd_intr(int irq, void *__hba);
> @@ -215,6 +214,16 @@ static inline void ufshcd_disable_irq(struct ufs_hba 
> *hba)
> }
>  }
>
> +static inline int ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
> +{
> +   return ufshcd_uic_hibern8_ctrl(hba, true);
> +}
> +
> +static inline int ufshcd_uic_hibern8_exit(struct ufs_hba *hba)
> +{
> +   return ufshcd_uic_hibern8_ctrl(hba, false);
> +}
> +
>  /*
>   * ufshcd_wait_for_register - wait for register value to change
>   * @hba - per-adapter interface
> @@ -2395,7 +2404,7 @@ out:
> return ret;
>  }
>
> -static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
> +static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
>  {
> struct uic_command uic_cmd = {0};
>
> @@ -2404,7 +2413,7 @@ static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
> return ufshcd_uic_pwr_ctrl(hba, &uic_cmd);
>  }
>
> -static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba)
> +static int __ufshcd_uic_hibern8_exit(struct ufs_hba *hba)
>  {
> struct uic_command uic_cmd = {0};
> int ret;
> @@ -2419,6 +2428,25 @@ static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba)
> return ret;
>  }
>
> +static int ufshcd_uic_hibern8_ctrl(struct ufs_hba *hba, bool en)
> +{
> +   int ret;
> +
> +   if (hba->vops && hba->vops->hibern8_notify)
> +   hba->vops->hibern8_notify(hba, en, PRE_CHANGE);
Return of hibern8_notify is not checked. Otherwise make the return type void.
> +
> +   ret = en ? __ufshcd_uic_hibern8_enter(hba) :
> +   __ufshcd_uic_hibern8_exit(hba);
> +   if (ret)
> +   goto out;
> +
> +   if (hba->vops && hba->vops->hibern8_notify)
> +   hba->vops->hibern8_notify(hba, en, POST_CHANGE);
> +
> +out:
> +   return ret;
> +}
> +
>   /**
>   * ufshcd_init_pwr_info - setting the POR (power on reset)
>   * values in hba power info
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 0b7dde0..045968e 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -260,6 +260,8 @@ struct ufs_pwr_mode_info {
>   * @specify_nexus_t_xfer_req:
>   * @specify_nexus_t_tm_req: called before command is issued to allow vendor
>   * specific handling to be set for nexus type.
> + * @hibern8_notify: called before and after hibernate/unhibernate is carried 
> out
> + * to allow vendor spesific implementation.
>   * @suspend: called during host controller PM callback
>   * @resume: called during host controller PM callback
>   */
> @@ -276,6 +278,7 @@ struct ufs_hba_variant_ops {
> int (*pwr_change_notify)(struct ufs_hba *,
> bool, struct ufs_pa_layer_attr *,
> struct ufs_pa_layer_attr *);
> +   int (*hibern8_notify)(struct ufs_hba *, bool, bool);
> void(*specify_nexus_t_xfer_req)(struct ufs_hba *,
> int, struct scsi_cmnd *);
> void(*specify_nexus_t_tm_req)(struct ufs_hba *, int, u8);
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/10] scsi: ufs: add quirk not to allow reset of interrupt aggregation

[amit daniel kachhap]

Few comments below,

On Fri, Aug 21, 2015 at 2:57 PM, Alim Akhtar  wrote:
> From: Seungwon Jeon 
>
> Some host controller supports interrupt aggregation, but doesn't
> allow to reset counter and timer by s/w.
>
> Signed-off-by: Seungwon Jeon 
> Signed-off-by: Alim Akhtar 
> ---
>  drivers/scsi/ufs/ufshcd.c |3 ++-
>  drivers/scsi/ufs/ufshcd.h |6 ++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b441a39..35380aa 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -3204,7 +3204,8 @@ static void ufshcd_transfer_req_compl(struct ufs_hba 
> *hba)
>  * false interrupt if device completes another request after resetting
>  * aggregation and before reading the DB.
>  */
> -   if (ufshcd_is_intr_aggr_allowed(hba))
> +   if (ufshcd_is_intr_aggr_allowed(hba) &&
> +   !(hba->quirks & UFSHCI_QUIRK_BROKEN_RESET_INTR_AGGR))
How about to rename this quirk as UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR as
there are some drawbacks about the existing method also as per the
comments above. Or this can be also put as opts instead as quirk.
> ufshcd_reset_intr_aggr(hba);
>
> tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 24245c9..7986a54 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -471,6 +471,12 @@ struct ufs_hba {
>  */
> #define UFSHCI_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(7)
>
> +   /*
> +* This quirk needs to be enabled if host controller doesn't allow
> +* that the interrupt aggregation timer and counter are reset by s/w.
> +*/
> +   #define UFSHCI_QUIRK_BROKEN_RESET_INTR_AGGR UFS_BIT(8)
> +
> unsigned int quirks;/* Deviations from standard UFSHCI spec. */
>
> wait_queue_head_t tm_wq;
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/10] scsi: ufs: add quirk to contain unconformable utrd field

[amit daniel kachhap]

Few minor comments below,

On Fri, Aug 21, 2015 at 2:57 PM, Alim Akhtar  wrote:
> From: Seungwon Jeon 
>
> UTRD(UTP Transfer Request Descriptor)'s field such as offset/length,
> especially response's has DWORD expression. This quirk can be specified
> for host controller not to conform standard.
>
> Signed-off-by: Seungwon Jeon 
> Signed-off-by: Alim Akhtar 
> ---
>  drivers/scsi/ufs/ufshcd.c |   28 +---
>  drivers/scsi/ufs/ufshcd.h |7 +++
>  2 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b0ade73..f882bf0 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1009,7 +1009,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct 
> uic_command *uic_cmd)
>   *
>   * Returns 0 in case of success, non-zero value in case of failure
>   */
> -static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
> +static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
>  {
> struct ufshcd_sg_entry *prd_table;
> struct scatterlist *sg;
> @@ -1023,8 +1023,13 @@ static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
> return sg_segments;
>
> if (sg_segments) {
> -   lrbp->utr_descriptor_ptr->prd_table_length =
> -   cpu_to_le16((u16) (sg_segments));
> +   if (hba->quirks & UFSHCI_QUIRK_BROKEN_UTRD)
> +   lrbp->utr_descriptor_ptr->prd_table_length =
> +   cpu_to_le16((u16)(sg_segments *
> +   sizeof(struct ufshcd_sg_entry)));
> +   else
> +   lrbp->utr_descriptor_ptr->prd_table_length =
> +   cpu_to_le16((u16) (sg_segments));
>
> prd_table = (struct ufshcd_sg_entry *)lrbp->ucd_prdt_ptr;
>
> @@ -1347,7 +1352,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, 
> struct scsi_cmnd *cmd)
>
> /* form UPIU before issuing the command */
> ufshcd_compose_upiu(hba, lrbp);
> -   err = ufshcd_map_sg(lrbp);
> +   err = ufshcd_map_sg(hba, lrbp);
> if (err) {
> lrbp->cmd = NULL;
> clear_bit_unlock(tag, &hba->lrb_in_use);
> @@ -2035,12 +2040,21 @@ static void ufshcd_host_memory_configure(struct 
> ufs_hba *hba)
> 
> cpu_to_le32(upper_32_bits(cmd_desc_element_addr));
>
> /* Response upiu and prdt offset should be in double words */
This comment can be moved below for the else case.
> -   utrdlp[i].response_upiu_offset =
> +   if (hba->quirks & UFSHCI_QUIRK_BROKEN_UTRD) {
> +   utrdlp[i].response_upiu_offset =
> +   cpu_to_le16(response_offset);
> +   utrdlp[i].prd_table_offset =
> +   cpu_to_le16(prdt_offset);
> +   utrdlp[i].response_upiu_length =
> +   cpu_to_le16(ALIGNED_UPIU_SIZE);
> +   } else {
> +   utrdlp[i].response_upiu_offset =
> cpu_to_le16((response_offset >> 2));
> -   utrdlp[i].prd_table_offset =
> +   utrdlp[i].prd_table_offset =
> cpu_to_le16((prdt_offset >> 2));
> -   utrdlp[i].response_upiu_length =
> +   utrdlp[i].response_upiu_length =
> cpu_to_le16(ALIGNED_UPIU_SIZE >> 2);
> +   }
>
> hba->lrb[i].utr_descriptor_ptr = (utrdlp + i);
> hba->lrb[i].ucd_req_ptr =
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index c40a0e7..1fa5ac1 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -459,6 +459,13 @@ struct ufs_hba {
>  */
> #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5)
>
> +   /*
> +* This quirk needs to be enabled if host controller doesn't conform
> +* with UTRD. Some fields such as offset/length might not be in 
> double word,
> +* but in byte.
> +*/
> +   #define UFSHCI_QUIRK_BROKEN_UTRDUFS_BIT(6)
This macro name may be given more meaningful name such as
UFSHCI_QUIRK_BYTE_ALIGN_UTRD or something similar.
> +
> unsigned int quirks;/* Deviations from standard UFSHCI spec. */
>
> wait_queue_head_t tm_wq;
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordo

Re: [PATCH 01/10] phy: exynos-ufs: add UFS PHY driver for EXYNOS SoC

[amit daniel kachhap]

Hi,

Few minor comments,

On Fri, Aug 21, 2015 at 2:57 PM, Alim Akhtar  wrote:
> From: Seungwon Jeon 
>
> This patch introduces Exynos UFS PHY driver. This driver
> supports to deal with phy calibration and power control
> according to UFS host driver's behavior.
>
> Signed-off-by: Seungwon Jeon 
> Signed-off-by: Alim Akhtar 
> Cc: Kishon Vijay Abraham I 
> ---
>  .../devicetree/bindings/phy/samsung-phy.txt|   22 ++
>  drivers/phy/Kconfig|7 +
>  drivers/phy/Makefile   |1 +
>  drivers/phy/phy-exynos-ufs.c   |  277 
> 
>  drivers/phy/phy-exynos-ufs.h   |   73 ++
>  drivers/phy/phy-exynos7-ufs.h  |   89 +++
>  include/linux/phy/phy-exynos-ufs.h |  107 
>  7 files changed, 576 insertions(+)
>  create mode 100644 drivers/phy/phy-exynos-ufs.c
>  create mode 100644 drivers/phy/phy-exynos-ufs.h
>  create mode 100644 drivers/phy/phy-exynos7-ufs.h
>  create mode 100644 include/linux/phy/phy-exynos-ufs.h
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt 
> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index 60c6f2a..1abe2c4 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -174,3 +174,25 @@ Example:
> usbdrdphy0 = &usb3_phy0;
> usbdrdphy1 = &usb3_phy1;
> };
> +
> +Samsung Exynos7 soc serise UFS PHY Controller
> +-
> +
> +UFS PHY nodes are defined to describe on-chip UFS Physical layer controllers.
> +Each UFS PHY controller should have its own node.
> +
> +Required properties:
> +- compatible: compatible list, contains "samsung,exynos7-ufs-phy"
> +- reg : offset and length of the UFS PHY register set;
> +- reg-names : reg name(s) must be 'phy-pma';
> +- #phy-cells : must be zero
> +- samsung,syscon-phandle : a phandle to the PMU system controller, no 
> arguments
> +
> +Example:
> +   ufs_phy: ufs-phy@0x15571800 {
> +   compatible = "samsung,exynos7-ufs-phy";
> +   reg = <0x15571800 0x240>;
> +   reg-names = "phy-pma";
> +   samsung,syscon-phandle = <&pmu_system_controller>;
> +   #phy-cells = <0>;
> +   };
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 6b8dd16..7449376 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -358,4 +358,11 @@ config PHY_BRCMSTB_SATA
>   Enable this to support the SATA3 PHY on 28nm Broadcom STB SoCs.
>   Likely useful only with CONFIG_SATA_BRCMSTB enabled.
>
> +config PHY_EXYNOS_UFS
> +   tristate "EXYNOS SoC series UFS PHY driver"
> +   depends on OF && ARCH_EXYNOS
> +   select GENERIC_PHY
> +   help
> + Support for UFS PHY on Samsung EXYNOS chipsets.
> +
>  endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index f344e1b..7a36818 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -45,3 +45,4 @@ obj-$(CONFIG_PHY_QCOM_UFS)+= phy-qcom-ufs-qmp-14nm.o
>  obj-$(CONFIG_PHY_TUSB1210) += phy-tusb1210.o
>  obj-$(CONFIG_PHY_BRCMSTB_SATA) += phy-brcmstb-sata.o
>  obj-$(CONFIG_PHY_PISTACHIO_USB)+= phy-pistachio-usb.o
> +obj-$(CONFIG_PHY_EXYNOS_UFS)   += phy-exynos-ufs.o
> diff --git a/drivers/phy/phy-exynos-ufs.c b/drivers/phy/phy-exynos-ufs.c
> new file mode 100644
> index 000..840375d
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-ufs.c
> @@ -0,0 +1,277 @@
> +/*
> + * UFS PHY driver for Samsung EXYNOS SoC
> + *
> + * Copyright (C) 2015 Samsung Electronics Co., Ltd.
> + * Author: Seungwon Jeon 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "phy-exynos-ufs.h"
> +
> +#define for_each_phy_lane(phy, i) \
> +   for (i = 0; i < (phy)->lane_cnt; i++)
> +#define for_each_phy_cfg(cfg) \
> +   for (; (cfg)->id; (cfg)++)
> +
> +#define phy_pma_writel(phy, val, reg) \
> +   writel((val), (phy)->reg_pma + (reg))
> +#define phy_pma_readl(phy, reg) \
> +   readl((phy)->reg_pma + (reg))
> +
> +#define PHY_DEF_LANE_CNT   1
> +
> +static inline struct exynos_ufs_phy *get_exynos_ufs_phy(struct phy *phy)
> +{
> +   return (struct exynos_ufs_phy *)phy_get_drvdata(phy);
> +}
> +
> +static void exynos_ufs_phy_config(struct exynos_ufs_phy *phy,
> +   const struct exynos_ufs_phy_cfg *cfg, u8 lane)
> +{
> +   enum {LANE_0, LANE_1}; /* lane index */
> +
> +   switch (l

Re: [PATCH v4 1/3] cxlflash: Base error recovery support

[Daniel Axtens]

On Tue, 2015-08-11 at 16:21 +1000, Daniel Axtens wrote:
> Actually, I forgot one thing:
> 
> >  
> >  config CXLFLASH
> > tristate "Support for IBM CAPI Flash"
> > -   depends on PCI && SCSI && CXL
> > +   depends on PCI && SCSI && CXL && EEH
> 
> Should you depend on CXL_EEH here, seeing as you use CONFIG_CXL_EEH?
> 
Actually, never mind, I'm wrong. What you have is good.

> >  
> > +#ifndef CONFIG_CXL_EEH
> > +#define cxl_perst_reloads_same_image(_a, _b) do { } while (0)
> > +#endif
> > +
> >  #endif /* _CXLFLASH_MAIN_H */
> 

-- 
Regards,
Daniel


signature.asc
Description: This is a digitally signed message part

Re: [PATCH v4 1/3] cxlflash: Base error recovery support

[Daniel Axtens]

Actually, I forgot one thing:

>  
>  config CXLFLASH
>   tristate "Support for IBM CAPI Flash"
> - depends on PCI && SCSI && CXL
> + depends on PCI && SCSI && CXL && EEH

Should you depend on CXL_EEH here, seeing as you use CONFIG_CXL_EEH?

>  
> +#ifndef CONFIG_CXL_EEH
> +#define cxl_perst_reloads_same_image(_a, _b) do { } while (0)
> +#endif
> +
>  #endif /* _CXLFLASH_MAIN_H */

-- 
Regards,
Daniel


signature.asc
Description: This is a digitally signed message part

Re: [PATCH v4 1/3] cxlflash: Base error recovery support

[Daniel Axtens]

> @@ -487,11 +515,27 @@ static int cxlflash_eh_host_reset_handler(struct 
> scsi_cmnd *scp)
>get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
>get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
>  
> - rcr = cxlflash_afu_reset(cfg);
> - if (rcr == 0)
> - rc = SUCCESS;
> - else
> + switch (cfg->state) {
> + case STATE_NORMAL:
> + cfg->state = STATE_LIMBO;
> + scsi_block_requests(cfg->host);
> +
> + rcr = cxlflash_afu_reset(cfg);
> + if (!rcr)
> + rc = FAILED;

I think you want:

if (rcr) {
rc = FAILED;
break;
}

cxlflash_afu_reset returns 0 on success, so I think you want to drop the
negation, and also I think if it fails you want to break out and not set
STATE_NORMAL. Is that right?

Once you fix that, or explain to me why I'm wrong:
Reviewed-by: Daniel Axtens 

-- 
Regards,
Daniel


signature.asc
Description: This is a digitally signed message part

Re: [PATCH v3 2/4] cxlflash: Base error recovery support

[Daniel Axtens]

Hi, 

> @@ -1857,9 +1884,18 @@ static int cxlflash_eh_device_reset_handler(struct 
> scsi_cmnd *scp)
>get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
>get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
>  
> - rcr = send_tmf(afu, scp, TMF_LUN_RESET);
> - if (unlikely(rcr))
> - rc = FAILED;
> + switch (cfg->eeh_active) {
> + case EEH_STATE_NONE:
> + rcr = send_tmf(afu, scp, TMF_LUN_RESET);
> + if (unlikely(rcr))
> + rc = FAILED;
> + break;
> + case EEH_STATE_ACTIVE:
> + wait_event(cfg->eeh_waitq, cfg->eeh_active != EEH_STATE_ACTIVE);
> + break;
> + case EEH_STATE_FAILED:
> + break;
> + }
>  

Looking at the context here, it looks like rc gets initalised to
SUCCESS. In that case, in the EEH failed case, you'll return SUCCESS.
I'm not particularly clear on the semantics here: does that make sense?

Likewise, in the EEH active state, when you finish the wait_event,
should you check if the EEH state went to NONE or FAILED before you
break?

There's a similar case below in host_reset.

>   pr_debug("%s: returning rc=%d\n", __func__, rc);
>   return rc;
> @@ -1889,11 +1925,23 @@ static int cxlflash_eh_host_reset_handler(struct 
> scsi_cmnd *scp)
>get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
>get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
>  
> - rcr = afu_reset(cfg);
> - if (rcr == 0)
> - rc = SUCCESS;
> - else
> - rc = FAILED;
> + switch (cfg->eeh_active) {
> + case EEH_STATE_NONE:
> + cfg->eeh_active = EEH_STATE_FAILED;
> + rcr = afu_reset(cfg);
> + if (rcr == 0)
> + rc = SUCCESS;
> + else
> + rc = FAILED;
> + cfg->eeh_active = EEH_STATE_NONE;
> + wake_up_all(&cfg->eeh_waitq);
> + break;
> + case EEH_STATE_ACTIVE:
> + wait_event(cfg->eeh_waitq, cfg->eeh_active != EEH_STATE_ACTIVE);
> + break;
> + case EEH_STATE_FAILED:
> + break;
> + }
>  
>   pr_debug("%s: returning rc=%d\n", __func__, rc);
>   return rc;
> @@ -2145,6 +2193,11 @@ static void cxlflash_worker_thread(struct work_struct 
> *work)
>   int port;
>   ulong lock_flags;
>  
> + /* Avoid MMIO if the device has failed */
> +
> + if (cfg->eeh_active == EEH_STATE_FAILED)
> + return;
> +
Should this check be != EEH_STATE_NONE? Or is this called within the
error recovery process?

>   spin_lock_irqsave(cfg->host->host_lock, lock_flags);
>  
>   if (cfg->lr_state == LINK_RESET_REQUIRED) {
> @@ -2226,6 +2279,8 @@ static int cxlflash_probe(struct pci_dev *pdev,
>  
>   cfg->init_state = INIT_STATE_NONE;
>   cfg->dev = pdev;
> +
> + cfg->eeh_active = EEH_STATE_NONE;
>   cfg->dev_id = (struct pci_device_id *)dev_id;
>  
> 
> @@ -2286,6 +2341,85 @@ out_remove:
>   goto out;
>  }
>  
> +/**
> + * cxlflash_pci_error_detected() - called when a PCI error is detected
> + * @pdev:PCI device struct.
> + * @state:   PCI channel state.
> + *
> + * Return: PCI_ERS_RESULT_NEED_RESET or PCI_ERS_RESULT_DISCONNECT
> + */
> +static pci_ers_result_t cxlflash_pci_error_detected(struct pci_dev *pdev,
> + pci_channel_state_t state)
> +{
> + struct cxlflash_cfg *cfg = pci_get_drvdata(pdev);
> +
> + pr_debug("%s: pdev=%p state=%u\n", __func__, pdev, state);
> +
> + switch (state) {
> + case pci_channel_io_frozen:
> + cfg->eeh_active = EEH_STATE_ACTIVE;
> + udelay(100);
> +
> + term_mc(cfg, UNDO_START);
> + stop_afu(cfg);
> +
> + return PCI_ERS_RESULT_CAN_RECOVER;

I think that should PCI_ERS_RESULT_NEED_RESET.

Apart from that, it's looking pretty good from an EEH perspective.

-- 
Regards,
Daniel


signature.asc
Description: This is a digitally signed message part

Re: [PATCH v3 2/4] cxlflash: Base error recovery support

[Daniel Axtens]

On Wed, 2015-08-05 at 17:30 -0500, Matthew R. Ochs wrote:
> Hi Brian,
> 
> Thanks for reviewing. Comments inline below.
> 
> 
> -matt
> 
> > On Aug 5, 2015, at 11:04 AM, Brian King  wrote:
> > 
> > On 08/02/2015 11:33 PM, Matthew R. Ochs wrote:
> > 
> >> diff --git a/drivers/scsi/cxlflash/common.h 
> >> b/drivers/scsi/cxlflash/common.h
> >> index ba070a5..3d6217a 100644
> >> --- a/drivers/scsi/cxlflash/common.h
> >> +++ b/drivers/scsi/cxlflash/common.h
> >> @@ -76,6 +76,12 @@ enum cxlflash_init_state {
> >>INIT_STATE_SCSI
> >> };
> >> 
> >> +enum eeh_state {
> >> +  EEH_STATE_NONE,
> >> +  EEH_STATE_ACTIVE,
> >> +  EEH_STATE_FAILED
> >> +};
> > 
> > Can you use pdev->error_state and pci_channel_offline instead of 
> > duplicating this
> > state information in a private driver definition?
> 
> Makes sense, I’ll look into this.
> 
I don't think my vPHB code propagates error_state yet. I'll check, and
if necessary, push a patch and fold it into my v3.

Regards
Daniel

> >> 
> >> +#ifdef CONFIG_CXL_EEH
> >> +  cxl_perst_reloads_same_image(afu, val) 
> >> +#endif
> > 
> > I'd suggest moving this to a .h and defining the function as a noop there 
> > if appropriate, something
> > like:
> > 
> > #ifndef CONFIG_CXL_EEH
> > #define cxl_perst_reloads_same_image(cfg->cxl_afu, true) do { } while(0)
> > #endif
> 
> Done.
> 
> >> 
> >> -  rcr = afu_reset(cfg);
> >> -  if (rcr == 0)
> >> -  rc = SUCCESS;
> >> -  else
> >> -  rc = FAILED;
> >> +  switch (cfg->eeh_active) {
> >> +  case EEH_STATE_NONE:
> >> +  cfg->eeh_active = EEH_STATE_FAILED;
> > 
> > Seems a little strange to be messing with the EEH state machine here when 
> > EEH isn't even at play.
> > If you can't switch to use the existing EEH state machine in the pdev 
> > struct, suggest renaming
> > this internal state machine to something more accurate and using the pdev 
> > EEH state machine where you can.
> > Same goes for the eeh_waitq…
> 
> I do agree that this is a bit strange. What we’re doing here is borrowing the 
> framework we
> put in place to quiesce user contexts and hold off new threads coming in 
> during an EEH
> event. I’ll look into how we can refactor this given that we’re going to move 
> to using the
> existing EEH state machine (pdev->error_state) and will no longer be able to 
> toggle state.
> 
> >> +  pr_debug("%s: pdev=%p state=%u\n", __func__, pdev, state);
> >> +
> >> +  switch (state) {
> >> +  case pci_channel_io_frozen:
> >> +  cfg->eeh_active = EEH_STATE_ACTIVE;
> >> +  udelay(100);
> >> +
> > 
> > I think this udelay needs a comment…
> 
> This may end up going away. I’ll add a comment if we keep it.
> 
> > I'd suggest calling scsi_block_requests here to stop your queuecommand 
> > function from being called.
> > Note that this won't stop EH commands from being sent, so you will still 
> > need to check this
> > in queuecommand, although the right thing to do may be to fix 
> > scsi_send_eh_cmnd to not call
> > queuecommand if the host is blocked.
> > 
> > You’d then need to call scsi_unblock_requests when EEH in the perm failure 
> > and resume cases.
> 
> Good suggestion, we’ll look at adding this in.


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] ipr: AF DASD raw mode implementation in ipr driver

[Daniel Kreling]

This patch implements raw mode support for AF DASD in ipr driver
which allows for tools to send commands directly to physical
devices which are members of RAID arrays when enabled in the firmware.

Signed-off-by: Wen Xiong
Signed-off-by: Brian King
Reviewed-by: Daniel Kreling
---

 drivers/scsi/ipr.c |   85 
+

 drivers/scsi/ipr.h |3 +
 2 files changed, 88 insertions(+)

diff -puN drivers/scsi/ipr.c~ipr_sispipe drivers/scsi/ipr.c
--- linux/drivers/scsi/ipr.c~ipr_sispipe2015-03-26 10:53:55.540578199 
-0500
+++ linux-bjking1/drivers/scsi/ipr.c2015-03-26 10:57:00.570080498 -0500
@@ -498,6 +498,10 @@ struct ipr_error_table_t ipr_error_table
"4061: Multipath redundancy level got better"},
{0x066B9200, 0, IPR_DEFAULT_LOG_LEVEL,
"4060: Multipath redundancy level got worse"},
+   {0x06808100, 0, IPR_DEFAULT_LOG_LEVEL,
+   "9083: Device raw mode enabled"},
+   {0x06808200, 0, IPR_DEFAULT_LOG_LEVEL,
+   "9084: Device raw mode disabled"},
{0x0727, 0, 0,
"Failure due to other device"},
{0x07278000, 0, IPR_DEFAULT_LOG_LEVEL,
@@ -4496,11 +4500,83 @@ static struct device_attribute ipr_resou
.show = ipr_show_resource_type
 };

+/**
+ * ipr_show_raw_mode - Show the adapter's raw mode
+ * @dev:   class device struct
+ * @buf:   buffer
+ *
+ * Return value:
+ * number of bytes printed to buffer
+ **/
+static ssize_t ipr_show_raw_mode(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+   struct scsi_device *sdev = to_scsi_device(dev);
+   struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg 
*)sdev->host->hostdata;
+   struct ipr_resource_entry *res;
+   unsigned long lock_flags = 0;
+   ssize_t len;
+
+   spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
+   res = (struct ipr_resource_entry *)sdev->hostdata;
+   if (res)
+   len = snprintf(buf, PAGE_SIZE, "%d\n", res->raw_mode);
+   else
+   len = -ENXIO;
+   spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
+   return len;
+}
+
+/**
+ * ipr_store_raw_mode - Change the adapter's raw mode
+ * @dev:   class device struct
+ * @buf:   buffer
+ *
+ * Return value:
+ * number of bytes printed to buffer
+ **/
+static ssize_t ipr_store_raw_mode(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+   struct scsi_device *sdev = to_scsi_device(dev);
+   struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg 
*)sdev->host->hostdata;
+   struct ipr_resource_entry *res;
+   unsigned long lock_flags = 0;
+   ssize_t len;
+
+   spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
+   res = (struct ipr_resource_entry *)sdev->hostdata;
+   if (res) {
+   if (ioa_cfg->sis64 && ipr_is_af_dasd_device(res)) {
+   res->raw_mode = simple_strtoul(buf, NULL, 10);
+   len = strlen(buf);
+   if (res->sdev)
+   sdev_printk(KERN_INFO, res->sdev, "raw mode is 
%s\n",
+   res->raw_mode ? "enabled" : "disabled");
+   } else
+   len = -EINVAL;
+   } else
+   len = -ENXIO;
+   spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
+   return len;
+}
+
+static struct device_attribute ipr_raw_mode_attr = {
+   .attr = {
+   .name = "raw_mode",
+   .mode = S_IRUGO | S_IWUSR,
+   },
+   .show = ipr_show_raw_mode,
+   .store = ipr_store_raw_mode
+};
+
 static struct device_attribute *ipr_dev_attrs[] = {
&ipr_adapter_handle_attr,
&ipr_resource_path_attr,
&ipr_device_id_attr,
&ipr_resource_type_attr,
+   &ipr_raw_mode_attr,
NULL,
 };

@@ -6152,6 +6228,13 @@ static void ipr_erp_start(struct ipr_ioa
break;
case IPR_IOASC_NR_INIT_CMD_REQUIRED:
break;
+   case IPR_IOASC_IR_NON_OPTIMIZED:
+   if (res->raw_mode){
+   res->raw_mode = 0;
+   scsi_cmd->result |= (DID_IMM_RETRY << 16);
+   } else
+   scsi_cmd->result |= (DID_ERROR << 16);
+   break;
default:
if (IPR_IOASC_SENSE_KEY(ioasc) > RECOVERED_ERROR)
scsi_cmd->result |= (DID_ERROR << 16);
@@ -6291,6 +6374,8 @@ static int ipr_queuecommand(struct Scsi_
(!ipr_is_gscsi(res) || scsi_cmd->cmnd[0] == IPR_QUERY_RS

Re: [PATCH 6/6] ipr: Driver version 2.6.1

[Daniel Kreling]

Bump driver version.

Signed-off-by: Brian King
Reviewed-by: Daniel Kreling
---

 drivers/scsi/ipr.h |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


===

diff -puN drivers/scsi/ipr.h~ipr_version_2_6_1 drivers/scsi/ipr.h

--- linux/drivers/scsi/ipr.h~ipr_version_2_6_1	2015-03-12 
16:27:14.235168394 -0500


+++ linux-bjking1/drivers/scsi/ipr.h2015-03-12 16:27:38.294920049 -0500

@@ -39,5 +39,5 @@

/*
·*·Literals
·*/
-#define·IPR_DRIVER_VERSION·"2.6.0"
-#define·IPR_DRIVER_DATE·"(November·16,·2012)"
+#define·IPR_DRIVER_VERSION·"2.6.1"
+#define·IPR_DRIVER_DATE·"(March·12,·2015)"

 /*
  * IPR_MAX_CMD_PER_LUN: This defines the maximum number of outstanding
_





--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] ipr: Re-enable write same

[Daniel Kreling]

Re-enable write same support for ipr RAID adapters.

Signed-off-by: Brian King
Reviewed-by: Daniel Kreling
---

 drivers/scsi/ipr.c |1 -
 1 file changed, 1 deletion(-)


===

diff -puN drivers/scsi/ipr.c~ipr_write_same drivers/scsi/ipr.c

--- linux/drivers/scsi/ipr.c~ipr_write_same	2015-03-12 
16:26:04.036892977 -0500


+++ linux-bjking1/drivers/scsi/ipr.c2015-03-12 16:26:04.044892894 -0500

@@ -6404,7 +6404,6 @@

static·struct·scsi_host_template·driver_
» .shost_attrs·=·ipr_ioa_attrs,
» .sdev_attrs·=·ipr_dev_attrs,
» .proc_name·=·IPR_NAME,
-» .no_write_same·=·1,
» .use_blk_tags·=·1,
};
_

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] ipr: Fix possible error path oops during initialization

[Daniel Kreling]

** Adding 'Reviewed-by' tag **

Fixes a possible oops during adapter initialization in some
memory allocation failure error paths scenarios.

Reported-by: Dan Carpenter

Signed-off-by: Brian King
Reviewed-by: Daniel Kreling
---

 drivers/scsi/ipr.c |   14 --
 1 file changed, 8 insertions(+), 6 deletions(-)


===

diff -puN drivers/scsi/ipr.c~ipr_free_cmd_blks_oops drivers/scsi/ipr.c

--- linux/drivers/scsi/ipr.c~ipr_free_cmd_blks_oops2015-03-26 
11:14:41.465782868 -0500


+++ linux-bjking1/drivers/scsi/ipr.c2015-03-26 11:14:41.471782829 -0500

@@ -9061,1 +9061,1 @@

static·void·ipr_free_cmd_blks(struct·ipr

 {
 int i;
-for (i = 0; i < IPR_NUM_CMD_BLKS; i++) {
-if (ioa_cfg->ipr_cmnd_list[i])
-dma_pool_free(ioa_cfg->ipr_cmd_pool,
-  ioa_cfg->ipr_cmnd_list[i],
-  ioa_cfg->ipr_cmnd_list_dma[i]);
+if (ioa_cfg->ipr_cmnd_list) {
+for (i = 0; i < IPR_NUM_CMD_BLKS; i++) {
+if (ioa_cfg->ipr_cmnd_list[i])
+dma_pool_free(ioa_cfg->ipr_cmd_pool,
+  ioa_cfg->ipr_cmnd_list[i],
+  ioa_cfg->ipr_cmnd_list_dma[i]);

-ioa_cfg->ipr_cmnd_list[i] = NULL;
+ioa_cfg->ipr_cmnd_list[i] = NULL;
+}
 }

 if (ioa_cfg->ipr_cmd_pool)
_

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] ipr: Reset in task context

[Daniel Kreling]

** Adding 'Reviewed-by' tag **

The pci_set_pcie_reset_state has changed semantics to not be callable
from interrupt context, so change ipr's usage of the API to comply with
this change by ensuring this occurs from a workqueue.

Signed-off-by: Brian King
Reviewed-by: Daniel Kreling
---

 drivers/scsi/ipr.c |   91 
+

 drivers/scsi/ipr.h |2 +
 2 files changed, 67 insertions(+), 26 deletions(-)


===

diff -puN drivers/scsi/ipr.c~ipr_reset_task drivers/scsi/ipr.c

--- linux/drivers/scsi/ipr.c~ipr_reset_task	2015-03-26 
11:14:39.103798589 -0500


+++ linux-bjking1/drivers/scsi/ipr.c2015-03-26 11:14:39.113798523 -0500

@@ -8320,8 +8320,7 @@

static·int·ipr_reset_start_bist(struct·i
static·int·ipr_reset_slot_reset_done(struct·ipr_cmnd·*ipr_cmd)
{
» ENTER;
-» pci_set_pcie_reset_state(ipr_cmd->ioa_cfg->pdev,·pcie_deassert_reset);
» ipr_cmd->job_step·=·ipr_reset_bist_done;
» ipr_reset_start_timer(ipr_cmd,·IPR_WAIT_FOR_BIST_TIMEOUT);
» LEAVE;

@@ -8328,2 +8327,2 @@

static·int·ipr_reset_slot_reset_done(str
}


===

-   }
-
+   ipr_free_irqs(ioa_cfg);
pci_disable_device(ioa_cfg->pdev);
}
 }
diff -puN drivers/scsi/ipr.h~ipr_reset_task drivers/scsi/ipr.h

--- linux/drivers/scsi/ipr.h~ipr_reset_task	2015-03-26 
11:14:39.107798561 -0500


+++ linux-bjking1/drivers/scsi/ipr.h2015-03-26 11:14:39.123798456 -0500

@@ -1540,4 +1540,5 @@

struct·ipr_ioa_cfg·{
» u8·saved_mode_page_len;
» struct·work_struct·work_q;
+» struct·workqueue_struct·*reset_work_q;

 /**
+ * ipr_reset_reset_work - Pulse a PCIe fundamental reset
+ * @work:  work struct
+ *
+ * Description: This pulses warm reset to a slot.
+ *
+ **/
+static void ipr_reset_reset_work(struct work_struct *work)
+{
+   struct ipr_cmnd *ipr_cmd = container_of(work, struct ipr_cmnd, work);
+   struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg;
+   struct pci_dev *pdev = ioa_cfg->pdev;
+   unsigned long lock_flags = 0;
+
+   ENTER;
+   pci_set_pcie_reset_state(pdev, pcie_warm_reset);
+   msleep(jiffies_to_msecs(IPR_PCI_RESET_TIMEOUT));
+   pci_set_pcie_reset_state(pdev, pcie_deassert_reset);
+
+   spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
+   if (ioa_cfg->reset_cmd == ipr_cmd)
+   ipr_reset_ioa_job(ipr_cmd);
+   spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
+   LEAVE;
+}
+
+/**
  * ipr_reset_slot_reset - Reset the PCI slot of the adapter.
  * @ipr_cmd:   ipr command struct
  *
@@ -8339,12 +8364,11 @@ static int ipr_reset_slot_reset_done(str
 static int ipr_reset_slot_reset(struct ipr_cmnd *ipr_cmd)
 {
struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg;
-   struct pci_dev *pdev = ioa_cfg->pdev;
ENTER;
-   pci_set_pcie_reset_state(pdev, pcie_warm_reset);
+   INIT_WORK(&ipr_cmd->work, ipr_reset_reset_work);
+   queue_work(ioa_cfg->reset_work_q, &ipr_cmd->work);
ipr_cmd->job_step = ipr_reset_slot_reset_done;
-   ipr_reset_start_timer(ipr_cmd, IPR_PCI_RESET_TIMEOUT);
LEAVE;
return IPR_RC_JOB_RETURN;
 }
@@ -9093,26 +9117,25 @@ static void ipr_free_mem(struct ipr_ioa_
 }
 /**
- * ipr_free_all_resources - Free all allocated resources for an adapter.
- * @ipr_cmd:   ipr command struct
+ * ipr_free_irqs - Free all allocated IRQs for the adapter.
+ * @ioa_cfg:   ipr cfg struct
  *
- * This function frees all allocated resources for the
+ * This function frees all allocated IRQs for the
  * specified adapter.
  *
  * Return value:
  * none
  **/
-static void ipr_free_all_resources(struct ipr_ioa_cfg *ioa_cfg)
+static void ipr_free_irqs(struct ipr_ioa_cfg *ioa_cfg)
 {
struct pci_dev *pdev = ioa_cfg->pdev;
-   ENTER;
if (ioa_cfg->intr_flag == IPR_USE_MSI ||
ioa_cfg->intr_flag == IPR_USE_MSIX) {
int i;
for (i = 0; i < ioa_cfg->nvectors; i++)
free_irq(ioa_cfg->vectors_info[i].vec,
-   &ioa_cfg->hrrq[i]);
+&ioa_cfg->hrrq[i]);
} else
free_irq(pdev->irq, &ioa_cfg->hrrq[0]);
@@ -9123,7 +9146,26 @@ static void ipr_free_all_resources(struc
pci_disable_msix(pdev);
ioa_cfg->intr_flag &= ~IPR_USE_MSIX;
}
+}
+
+/**
+ * ipr_free_all_resources - Free all allocated resources for an adapter.
+ * @ipr_cmd:   ipr command struct
+ *
+ * This function frees all allocated resources for the
+ * specified adapter.
+ *
+ * Return value:
+ * none
+ **/
+static void ipr_free_all_resources(struct ipr_ioa_cfg *ioa_cfg)
+{
+   struct pci_dev *pdev = ioa_cfg->pdev;
+   ENTER;
+   ipr_free_irqs(ioa_cfg);
+   if (ioa_cfg->reset_work_q)
+

Re: [PATCH 1/6] ipr: Reboot speed improvements

[Daniel Kreling]

** Adding Reviewed-by tag **

--
Currently when performing a reboot with an ipr adapter,
the adapter gets shutdown completely, flushing all write
cache, as well as performing a full hardware reset of the card
during the shutdown phase of the old kernel. This ensures
the adapter is in a fully quiesced state across the reboot.

There are scenarios, however, such as when performing
kexec, where this full adapter shutdown is not required
and not desired, since it can make the reboot process take
noticeably longer.

This patch adds a module parameter to allow for skipping the
full shutdown during reboot. Rather than performing a full
adapter shutdown and reset, we simply cancel any outstanding
error buffers, place the adapter into a state where it has no
memory of any DMA addresses from the old kernel, then disable
the device. This significantly speeds up kexec boot, particularly
in configurations with multiple ipr adapters.

Signed-off-by: Brian King
Reviewed-by: Daniel Kreling
---

 drivers/scsi/ipr.c |  160 
++---

 drivers/scsi/ipr.h |6 +
 2 files changed, 157 insertions(+), 9 deletions(-)


===

diff -puN drivers/scsi/ipr.c~ipr_cancel_hcams3 drivers/scsi/ipr.c

--- linux/drivers/scsi/ipr.c~ipr_cancel_hcams3	2015-03-26 
11:14:37.041812306 -0500


+++ linux-bjking1/drivers/scsi/ipr.c2015-03-26 11:14:37.051812240 -0500

@@ -99,5 +99,6 @@

static·unsigned·int·ipr_debug·=·0;
static·unsigned·int·ipr_max_devs·=·IPR_DEFAULT_SIS64_DEVS;
static·unsigned·int·ipr_dual_ioa_raid·=·1;
static·unsigned·int·ipr_number_of_msix·=·2;
+static·unsigned·int·ipr_fast_reboot;
static·DEFINE_SPINLOCK(ipr_driver_lock);


===

-   if (!ioa_cfg->hrrq[IPR_INIT_HRRQ].allow_cmds) {
+   if (!ioa_cfg->hrrq[IPR_INIT_HRRQ].allow_cmds ||
+   (ipr_fast_reboot && event == SYS_RESTART && 
ioa_cfg->sis64)) {
spin_unlock_irqrestore(ioa_cfg->host->host_lock, flags);
continue;
}
diff -puN drivers/scsi/ipr.h~ipr_cancel_hcams3 drivers/scsi/ipr.h

--- linux/drivers/scsi/ipr.h~ipr_cancel_hcams3	2015-03-26 
11:14:37.044812286 -0500


+++ linux-bjking1/drivers/scsi/ipr.h2015-03-26 11:14:37.059812188 -0500

@@ -196,6 +196,8 @@

/*
·*·Adapter·Commands
·*/
+#define·IPR_CANCEL_REQUEST» » » » 0xC0
+#define»IPR_CANCEL_64BIT_IOARCB»» » 0x01
#define·IPR_QUERY_RSRC_STATE» » » » 0xC2
#define·IPR_RESET_DEVICE» » » » 0xC3
#define»IPR_RESET_TYPE_SELECT» » » » 0x80

@@ -222,6 +224,7 @@

#define·IPR_ABBREV_SHUTDOWN_TIMEOUT» » (10·*·HZ)
#define·IPR_DUAL_IOA_ABBR_SHUTDOWN_TO» (2·*·60·*·HZ)
#define·IPR_DEVICE_RESET_TIMEOUT» » (ipr_fastfail·?·10·*·HZ·:·30·*·HZ)
+#define·IPR_CANCEL_TIMEOUT» » » (ipr_fastfail·?·10·*·HZ·:·30·*·HZ)
#define·IPR_CANCEL_ALL_TIMEOUT» » (ipr_fastfail·?·10·*·HZ·:·30·*·HZ)
#define·IPR_ABORT_TASK_TIMEOUT» » (ipr_fastfail·?·10·*·HZ·:·30·*·HZ)
#define·IPR_INTERNAL_TIMEOUT» » » (ipr_fastfail·?·10·*·HZ·:·30·*·HZ)

@@ -1402,6 +1405,7 @@

enum·ipr_shutdown_type·{
» IPR_SHUTDOWN_NORMAL·=·0x00,
» IPR_SHUTDOWN_PREPARE_FOR_NORMAL·=·0x40,
» IPR_SHUTDOWN_ABBREV·=·0x80,
-» IPR_SHUTDOWN_NONE·=·0x100
+» IPR_SHUTDOWN_NONE·=·0x100,
+» IPR_SHUTDOWN_QUIESCE·=·0x101,
};

 /* This table describes the differences between DMA controller chips */
@@ -221,6 +222,8 @@ MODULE_PARM_DESC(max_devs, "Specify the
 "[Default=" __stringify(IPR_DEFAULT_SIS64_DEVS) "]");
 module_param_named(number_of_msix, ipr_number_of_msix, int, 0);
 MODULE_PARM_DESC(number_of_msix, "Specify the number of MSIX 
interrupts to use on capable adapters (1 - 16).  (default:2)");

+module_param_named(fast_reboot, ipr_fast_reboot, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(fast_reboot, "Skip adapter shutdown during reboot. Set 
to 1 to enable. (default: 0)");

 MODULE_LICENSE("GPL");
 MODULE_VERSION(IPR_DRIVER_VERSION);
@@ -1462,7 +1465,8 @@ static void ipr_process_ccn(struct ipr_c
list_add_tail(&ipr_cmd->queue, &ipr_cmd->hrrq->hrrq_free_q);
if (ioasc) {
-   if (ioasc != IPR_IOASC_IOA_WAS_RESET)
+   if (ioasc != IPR_IOASC_IOA_WAS_RESET &&
+   ioasc != IPR_IOASC_ABORTED_CMD_TERM_BY_HOST)
dev_err(&ioa_cfg->pdev->dev,
"Host RCB failed with IOASC: 0x%08X\n", ioasc);
@@ -2566,7 +2570,8 @@ static void ipr_process_error(struct ipr
ipr_handle_log_data(ioa_cfg, hostrcb);
if (fd_ioasc == IPR_IOASC_NR_IOA_RESET_REQUIRED)
ipr_initiate_ioa_reset(ioa_cfg, IPR_SHUTDOWN_ABBREV);
-   } else if (ioasc != IPR_IOASC_IOA_WAS_RESET) {
+   } else if (ioasc != IPR_IOASC_IOA_WAS_RESET &&
+  ioasc != IPR_IOASC_ABORTED_CMD_TERM_BY_HOST) {

Re: scsi: add support for a blk-mq based I/O path.

[Daniel Gryniewicz]

On 09/16/2014 12:19 PM, Christoph Hellwig wrote:

Hi Daniel,

this should also be fixed with my patch "scsi: clean up S/G table freeing",
can you test if that fixes the issue for you as well?




Yeah, that fixes it, thanks.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: scsi: add support for a blk-mq based I/O path.

[Daniel Gryniewicz]

Hi, Chris.

I'm working with the OSD Initiator, which does bi-directional requests, 
and this commit (d285203 scsi: add support for a blk-mq based I/O path.) 
causes a use-after free panic for me.  The panic itself follows, but the 
cause is that scsi_end_request() calls blk_finish_request(), which frees 
the request, and then it calls scsi_release_bidi_buffers() which tries 
to indirect through the request to find it's own buffers.  This only 
affects bi-directional requests, so it's unlikely to be hit very often. 
The following patch fixes it for me, but it may not be the correct fix.


Daniel


From af61bb1f014bb1d034f4e7c3a18067ff3c9acedf Mon Sep 17 00:00:00 2001
From: Daniel Gryniewicz 
Date: Tue, 16 Sep 2014 09:44:35 -0400
Subject: [PATCH] Panic accesing freed memory during bidi SCSI

When ending a bi-directionional SCSI request, blk_finish_request()
cleans up and frees the request, but scsi_release_bidi_buffers() tries
to indirect through the request to find it's data buffers.  This causes
a panic due to a null pointer dereference.

Move the call to scsi_release_bidi_buffers() before the call to
blk_finish_request().

Signed-off-by: Daniel Gryniewicz 
---
 drivers/scsi/scsi_lib.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index d837dc1..aaea4b9 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -733,12 +733,13 @@ static bool scsi_end_request(struct request *req, 
int error,

} else {
unsigned long flags;

+   if (bidi_bytes)
+   scsi_release_bidi_buffers(cmd);
+
spin_lock_irqsave(q->queue_lock, flags);
blk_finish_request(req, error);
spin_unlock_irqrestore(q->queue_lock, flags);

-   if (bidi_bytes)
-   scsi_release_bidi_buffers(cmd);
scsi_release_buffers(cmd);
scsi_next_command(cmd);
}
--
2.0.3

[  748.403847] NULL pointer dereference at 00f8
[  748.403847] IP: [] scsi_end_request+0x1b0/0x1f0
[  748.403847] PGD 36bb9067 PUD 36878067 PMD 0
[  748.403847] Oops:  [#1] SMP
[  748.403847] Modules linked in: rpcsec_gss_krb5(E) objlayoutdriver(E) 
nfsv4(E) libore(E) osd(E) libosd(E) nf_conntrack_ipv4(E) 
nf_defrag_ipv4(E) xt_conntrack(E) nf_conntrack(E) ipt_REJECT(E) 
xt_CHECKSUM(E) iptable_mangle(E) xt_tcpudp(E) bridge(E) stp(E) llc(E) 
ip6table_filter(E) ip6_tables(E) iptable_filter(E) ip_tables(E) 
ebtable_nat(E) ebtables(E) x_tables(E) ib_iser(E) rdma_cm(E) iw_cm(E) 
ib_cm(E) ib_sa(E) ib_mad(E) ib_core(E) ib_addr(E) iscsi_tcp(E) 
libiscsi_tcp(E) libiscsi(E) scsi_transport_iscsi(E) ppdev(E) 
kvm_intel(E) kvm(E) snd_intel8x0(E) snd_ac97_codec(E) ac97_bus(E) 
parport_pc(E) snd_pcm(E) snd_timer(E) snd(E) soundcore(E) i2c_piix4(E) 
serio_raw(E) mac_hid(E) lp(E) parport(E) nfsd(E) auth_rpcgss(E) 
nfs_acl(E) nfs(E) lockd(E) sunrpc(E) fscache(E) raid10(E) raid456(E) 
async_raid6_recov(E) async_memcpy(E) async_pq(E) async_xor(E) 
async_tx(E) xor(E) raid6_pq(E) raid1(E) 8139too(E) raid0(E) psmouse(E) 
multipath(E) 8139cp(E) mii(E) linear(E) floppy(E)
[  748.403847] CPU: 0 PID: 3 Comm: ksoftirqd/0 Tainted: GE 
3.17.0-rc5-pnfs #8

[  748.403847] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[  748.403847] task: 880214143200 ti: 880214164000 task.ti: 
880214164000
[  748.403847] RIP: 0010:[]  [] 
scsi_end_request+0x1b0/0x1f0

[  748.403847] RSP: 0018:880214167cf0  EFLAGS: 00010246
[  748.403847] RAX:  RBX: 880036a192b0 RCX: 
69a6
[  748.403847] RDX: dd86 RSI:  RDI: 
0246
[  748.403847] RBP: 880214167d20 R08: 0096 R09: 

[  748.403847] R10: 092f R11: 880214167a0e R12: 
0246
[  748.403847] R13: 880036a1b380 R14:  R15: 
880211fad598
[  748.403847] FS:  () GS:88021fc0() 
knlGS:

[  748.403847] CS:  0010 DS:  ES:  CR0: 8005003b
[  748.403847] CR2: 00f8 CR3: 36902000 CR4: 
06f0

[  748.403847] Stack:
[  748.403847]  0020810a00d4 880036a1b380 880036a192b0 

[  748.403847]  880211fad598  880214167d78 
814f8043
[  748.403847]  880214167d40 3a98 0001 
81099ebd

[  748.403847] Call Trace:
[  748.403847]  [] scsi_io_completion+0x373/0x690
[  748.403847]  [] ? sched_clock_local+0x1d/0x80
[  748.403847]  [] scsi_finish_command+0xcf/0x130
[  748.403847]  [] scsi_softirq_done+0x136/0x160
[  748.403847]  [] blk_done_softirq+0x83/0xa0
[  748.403847]  [] __do_softirq+0xf5/0x2e0
[  748.403847]  [] run_ksoftirqd+0x30/0x50
[  748.403847]  [] smpboot_thread_fn+0xff/0x1b0
[  748.403847]  [] ? SyS_setgroups+0x1a0/0x1a0
[  748.403847]  [] kthre

[PATCH] random32: do not feed jiffies as seed from lpfc driver

[Daniel Borkmann]

In prandom we have already reseeding mechanisms that trigger
periodically from a much better entropy source than just
feeding in jiffies through lpfc_mbx_cmpl_fcf_scan_read_fcf_rec()
[what a function name 8-)]. Therefore, just remove this.

Signed-off-by: Daniel Borkmann 
Cc: James Bottomley 
Cc: James Smart 
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 2a17e31..5072bb2 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -2146,7 +2146,6 @@ lpfc_mbx_cmpl_fcf_scan_read_fcf_rec(struct lpfc_hba 
*phba, LPFC_MBOXQ_t *mboxq)
uint16_t fcf_index, next_fcf_index;
struct lpfc_fcf_rec *fcf_rec = NULL;
uint16_t vlan_id;
-   uint32_t seed;
bool select_new_fcf;
int rc;
 
@@ -2383,9 +2382,6 @@ lpfc_mbx_cmpl_fcf_scan_read_fcf_rec(struct lpfc_hba 
*phba, LPFC_MBOXQ_t *mboxq)
phba->fcf.fcf_flag |= FCF_AVAILABLE;
/* Setup initial running random FCF selection count */
phba->fcf.eligible_fcf_cnt = 1;
-   /* Seeding the random number generator for random selection */
-   seed = (uint32_t)(0x & jiffies);
-   prandom_seed(seed);
}
spin_unlock_irq(&phba->hbalock);
goto read_next_fcf;
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/9] [drivers/scsi] replace strict_strto calls

[Daniel Walter]

Replace obsolete strict_strto with more appropriate kstrto calls

Signed-off-by: Daniel Walter 
---
 drivers/scsi/pmcraid.c| 4 ++--
 drivers/scsi/scsi_sysfs.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index be8ce54..3762488 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -4213,9 +4213,9 @@ static ssize_t pmcraid_store_log_level(
 {
struct Scsi_Host *shost;
struct pmcraid_instance *pinstance;
-   unsigned long val;
+   u8 val;
 
-   if (strict_strtoul(buf, 10, &val))
+   if (kstrtou8(buf, 10, &val))
return -EINVAL;
/* log-level should be from 0 to 2 */
if (val > 2)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 074e8cc..80f715d 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -885,9 +885,9 @@ sdev_store_queue_ramp_up_period(struct device *dev,
const char *buf, size_t count)
 {
struct scsi_device *sdev = to_scsi_device(dev);
-   unsigned long period;
+   unsigned int period;
 
-   if (strict_strtoul(buf, 10, &period))
+   if (kstrtouint(buf, 10, &period))
return -EINVAL;
 
sdev->queue_ramp_up_period = msecs_to_jiffies(period);
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 09/11] [drivers/scsi] replace strict_strto calls

[Daniel Walter]

From: Daniel Walter 
Subject: [PATCH v2 09/11] [drivers/scsi] replace strict_strto calls

Replace obsolete strict_strto with more appropriate kstrto calls

Signed-off-by: Daniel Walter 
---
Resubmit to the mailing list (was submitted to MAINTAINER entry 
anil_ravindran...@pmc-sierra.com
but it seems this email no longer exists).

---
 drivers/scsi/pmcraid.c| 4 ++--
 drivers/scsi/scsi_sysfs.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index be8ce54..3762488 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -4213,9 +4213,9 @@ static ssize_t pmcraid_store_log_level(
 {
struct Scsi_Host *shost;
struct pmcraid_instance *pinstance;
-   unsigned long val;
+   u8 val;
 
-   if (strict_strtoul(buf, 10, &val))
+   if (kstrtou8(buf, 10, &val))
return -EINVAL;
/* log-level should be from 0 to 2 */
if (val > 2)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 074e8cc..80f715d 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -885,9 +885,9 @@ sdev_store_queue_ramp_up_period(struct device *dev,
const char *buf, size_t count)
 {
struct scsi_device *sdev = to_scsi_device(dev);
-   unsigned long period;
+   unsigned int period;
 
-   if (strict_strtoul(buf, 10, &period))
+   if (kstrtouint(buf, 10, &period))
return -EINVAL;
 
sdev->queue_ramp_up_period = msecs_to_jiffies(period);
-- 
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RESEND] SCSI: sd: don't fail if the device doesn't recognize SYNCHRONIZE CACHE

[Daniel Mack]

From: Alan Stern 

Evidently some wacky USB-ATA bridges don't recognize the SYNCHRONIZE
CACHE command, as shown in this email thread:

http://marc.info/?t=13897835622&r=1&w=2

The fact that we can't tell them to drain their caches shouldn't
prevent the system from going into suspend.  Therefore sd_sync_cache()
shouldn't return an error if the device replies with an Invalid
Command ASC.

Signed-off-by: Alan Stern 
Reported-by: Sven Neumann 
Tested-by: Daniel Mack 
CC: Oliver Neukum 
CC: 
---
Hi,

this patch has been around for awhile, but hasn't gained much
attraction, and hasn't been merged anywhere yet. Which is sad,
as it fixes a bug on real hardware when going to suspend :)

Could anyone from the SCSI people have a quick look maybe?

Thanks,
Daniel

 drivers/scsi/sd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 470954a..36d1a23 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1463,8 +1463,8 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
sd_print_sense_hdr(sdkp, &sshdr);
/* we need to evaluate the error return  */
if (scsi_sense_valid(&sshdr) &&
-   /* 0x3a is medium not present */
-   sshdr.asc == 0x3a)
+   (sshdr.asc == 0x3a ||   /* medium not present */
+sshdr.asc == 0x20))/* invalid command */
/* this is no error here */
return 0;
 
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SCSI: sd: don't fail if the device doesn't recognize SYNCHRONIZE CACHE

[Daniel Mack]

On 02/05/2014 12:04 PM, Daniel Mack wrote:
> On 01/15/2014 09:37 PM, Alan Stern wrote:
>> Evidently some wacky USB-ATA bridges don't recognize the SYNCHRONIZE
>> CACHE command, as shown in this email thread:
>>
>>  http://marc.info/?t=13897835622&r=1&w=2
>>
>> The fact that we can't tell them to drain their caches shouldn't
>> prevent the system from going into suspend.  Therefore sd_sync_cache()
>> shouldn't return an error if the device replies with an Invalid
>> Command ASC.
>>
>> Signed-off-by: Alan Stern 
>> Reported-by: Sven Neumann 
>> Tested-by: Daniel Mack 
>> CC: Oliver Neukum 
>> CC: 
> 
> Any objections about this patch? It would be good to get it merged for
> 3.14, if possible.

Ping? Did anyone queue this one up?


Thanks,
Daniel


>>  drivers/scsi/sd.c |4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> Index: usb-3.13/drivers/scsi/sd.c
>> ===
>> --- usb-3.13.orig/drivers/scsi/sd.c
>> +++ usb-3.13/drivers/scsi/sd.c
>> @@ -1463,8 +1463,8 @@ static int sd_sync_cache(struct scsi_dis
>>  sd_print_sense_hdr(sdkp, &sshdr);
>>  /* we need to evaluate the error return  */
>>  if (scsi_sense_valid(&sshdr) &&
>> -/* 0x3a is medium not present */
>> -sshdr.asc == 0x3a)
>> +(sshdr.asc == 0x3a ||   /* medium not present */
>> + sshdr.asc == 0x20))/* invalid command */
>>  /* this is no error here */
>>  return 0;
>>  
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SCSI: sd: don't fail if the device doesn't recognize SYNCHRONIZE CACHE

[Daniel Mack]

On 01/15/2014 09:37 PM, Alan Stern wrote:
> Evidently some wacky USB-ATA bridges don't recognize the SYNCHRONIZE
> CACHE command, as shown in this email thread:
> 
>   http://marc.info/?t=13897835622&r=1&w=2
> 
> The fact that we can't tell them to drain their caches shouldn't
> prevent the system from going into suspend.  Therefore sd_sync_cache()
> shouldn't return an error if the device replies with an Invalid
> Command ASC.
> 
> Signed-off-by: Alan Stern 
> Reported-by: Sven Neumann 
> Tested-by: Daniel Mack 
> CC: Oliver Neukum 
> CC: 

Any objections about this patch? It would be good to get it merged for
3.14, if possible.


Thanks,
Daniel


> 
> ---
> 
> 
> [as1734]
> 
> 
>  drivers/scsi/sd.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Index: usb-3.13/drivers/scsi/sd.c
> ===
> --- usb-3.13.orig/drivers/scsi/sd.c
> +++ usb-3.13/drivers/scsi/sd.c
> @@ -1463,8 +1463,8 @@ static int sd_sync_cache(struct scsi_dis
>   sd_print_sense_hdr(sdkp, &sshdr);
>   /* we need to evaluate the error return  */
>   if (scsi_sense_valid(&sshdr) &&
> - /* 0x3a is medium not present */
> - sshdr.asc == 0x3a)
> + (sshdr.asc == 0x3a ||   /* medium not present */
> +  sshdr.asc == 0x20))/* invalid command */
>   /* this is no error here */
>   return 0;
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Suspend issues with a LaCie USB hard disk connected

[Daniel Mack]

Hi Alan,

On 01/15/2014 06:19 PM, Alan Stern wrote:
> On Wed, 15 Jan 2014, Daniel Mack wrote:
> 
>> Hi,
>>
>> Sorry for the long, primarily holiday-related delay on this.
>>
>> On 12/18/2013 09:46 PM, Alan Stern wrote:
>>> On Wed, 18 Dec 2013, Daniel Mack wrote:
>>>> I'm facing an issue putting an embedded system to sleep while a Lacie
>>>> external USB hard disk is connected. Relevant kernel messages that occur
>>>> at the attempt are:
>>>>
>>>> [   13.834731] PM: Sending message for entering DeepSleep mode
>>>> [   13.846575] sd 0:0:0:0: [sda] Synchronizing SCSI cache
>>>> [   13.858818] sd 0:0:0:0: [sda]
>>>> [   13.862432] Result: hostbyte=0x00 driverbyte=0x08
>>>> [   13.867349] sd 0:0:0:0: [sda]
>>>> [   13.870626] Sense Key : 0x5 [current]
>>>> [   13.874602] sd 0:0:0:0: [sda]
>>>> [   13.877879] ASC=0x20 ASCQ=0x0
>>>> [   13.885053] dpm_run_callback(): scsi_bus_suspend+0x0/0x20 returns -5
>>>> [   13.901130] PM: Device 0:0:0:0 failed to suspend async: error -5
>>>> [   13.907507] PM: Some devices failed to suspend, or early wake event
>>>> detected
>>>>
>>>> What happens is that in sd_sync_cache(), scsi_execute_req_flags()
>>>> returns 0x0802, so driver_byte(res) evaluates to DRIVER_SENSE and
>>>> host_byte(res) is DID_OK, which is an unhandled case that leads to -EIO
>>>> eventually.
>>>>
>>>> I have admittedly not much clue about the SCSI layer, so I wonder what
>>>> would be the best way to fix this. Should DID_OK just be handled as
>>>> non-error condition in the switch? Should the suspend call chain ignore
>>>> such errors from sd_sync_cache()?
>>>>
>>>> I'm open to suggestions and happy to test patches.
>>>
>>> The Sense Key and ASC values indicate that the drive did not understand
>>> the SYNCHRONIZE CACHE command.  A usbmon trace would verify this; see
>>> the instructions in Documentation/usb/usbmon.txt.
>>>
>>> Assuming that really is what happened, we have to decide how to handle 
>>> the situation.
>>
>> Ok, this is the usbmon trace that I captured when the system goes to
>> suspend with the USB storage media connected but unmounted:
>>
>> cebe5e00 3629314504 S Bo:1:003:2 -115 31 = 55534243 1000 
>> 0a35    00
>> cebe5e00 3629315214 C Bo:1:003:2 0 31 >
>> cebe5e00 3629315413 S Bi:1:003:1 -115 13 <
>> cebe5e00 3629315492 C Bi:1:003:1 0 13 = 55534253 1000  01
> 
> That's the SYNCHRONIZE CACHE command, with an error return status.
> 
>> cebe5e00 3629315571 S Bo:1:003:2 -115 31 = 55534243 1100 1200
>> 8603 0012   00
>> cebe5e00 3629315606 C Bo:1:003:2 0 31 >
>> cecd4580 3629315681 S Bi:1:003:1 -115 18 <
>> cecd4580 3629315744 C Bi:1:003:1 0 18 = 7500 000a 
>> 2000 
>> cebe5e00 3629315772 S Bi:1:003:1 -115 13 <
>> cebe5e00 3629315817 C Bi:1:003:1 0 13 = 55534253 1100  00
> 
> And that's the sense data, confirming SK=5 and ASC=20.  This means the 
> drive doesn't understand the command.

Ok.

> There's more stuff later on in the usbmon trace that I don't 
> understand.  But if everything else works okay, it won't matter.

The host controller tried to reset the port and the device, whatever
that results in. You're right, that is unrelated.

> I don't think that is the right thing to do.  Try this patch instead.

[...]

> Index: usb-3.13/drivers/scsi/sd.c
> ===
> --- usb-3.13.orig/drivers/scsi/sd.c
> +++ usb-3.13/drivers/scsi/sd.c
> @@ -1463,8 +1463,8 @@ static int sd_sync_cache(struct scsi_dis
>   sd_print_sense_hdr(sdkp, &sshdr);
>   /* we need to evaluate the error return  */
>   if (scsi_sense_valid(&sshdr) &&
> - /* 0x3a is medium not present */
> - sshdr.asc == 0x3a)
> + (sshdr.asc == 0x3a ||   /* medium not present */
> +  sshdr.asc == 0x20))/* invalid command */
>   /* this is no error here */
>   return 0;
>  

That seems to work equally well for me, thanks!

Feel free to add when submitting:

  Reported-by: Sven Neumann 
  Tested-by: Daniel Mack 


Thanks for your help!
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Suspend issues with a LaCie USB hard disk connected

[Daniel Mack]

Hi,

Sorry for the long, primarily holiday-related delay on this.

On 12/18/2013 09:46 PM, Alan Stern wrote:
> On Wed, 18 Dec 2013, Daniel Mack wrote:
>> I'm facing an issue putting an embedded system to sleep while a Lacie
>> external USB hard disk is connected. Relevant kernel messages that occur
>> at the attempt are:
>>
>> [   13.834731] PM: Sending message for entering DeepSleep mode
>> [   13.846575] sd 0:0:0:0: [sda] Synchronizing SCSI cache
>> [   13.858818] sd 0:0:0:0: [sda]
>> [   13.862432] Result: hostbyte=0x00 driverbyte=0x08
>> [   13.867349] sd 0:0:0:0: [sda]
>> [   13.870626] Sense Key : 0x5 [current]
>> [   13.874602] sd 0:0:0:0: [sda]
>> [   13.877879] ASC=0x20 ASCQ=0x0
>> [   13.885053] dpm_run_callback(): scsi_bus_suspend+0x0/0x20 returns -5
>> [   13.901130] PM: Device 0:0:0:0 failed to suspend async: error -5
>> [   13.907507] PM: Some devices failed to suspend, or early wake event
>> detected
>>
>> What happens is that in sd_sync_cache(), scsi_execute_req_flags()
>> returns 0x0802, so driver_byte(res) evaluates to DRIVER_SENSE and
>> host_byte(res) is DID_OK, which is an unhandled case that leads to -EIO
>> eventually.
>>
>> I have admittedly not much clue about the SCSI layer, so I wonder what
>> would be the best way to fix this. Should DID_OK just be handled as
>> non-error condition in the switch? Should the suspend call chain ignore
>> such errors from sd_sync_cache()?
>>
>> I'm open to suggestions and happy to test patches.
> 
> The Sense Key and ASC values indicate that the drive did not understand
> the SYNCHRONIZE CACHE command.  A usbmon trace would verify this; see
> the instructions in Documentation/usb/usbmon.txt.
> 
> Assuming that really is what happened, we have to decide how to handle 
> the situation.

Ok, this is the usbmon trace that I captured when the system goes to
suspend with the USB storage media connected but unmounted:

cebe5e00 3629314504 S Bo:1:003:2 -115 31 = 55534243 1000 
0a35    00
cebe5e00 3629315214 C Bo:1:003:2 0 31 >
cebe5e00 3629315413 S Bi:1:003:1 -115 13 <
cebe5e00 3629315492 C Bi:1:003:1 0 13 = 55534253 1000  01
cebe5e00 3629315571 S Bo:1:003:2 -115 31 = 55534243 1100 1200
8603 0012   00
cebe5e00 3629315606 C Bo:1:003:2 0 31 >
cecd4580 3629315681 S Bi:1:003:1 -115 18 <
cecd4580 3629315744 C Bi:1:003:1 0 18 = 7500 000a 
2000 
cebe5e00 3629315772 S Bi:1:003:1 -115 13 <
cebe5e00 3629315817 C Bi:1:003:1 0 13 = 55534253 1100  00
cebe5e00 3629319750 S Bo:1:003:2 -115 31 = 55534243 1200 
0a35    00
cebe5e00 3629319826 C Bo:1:003:2 0 31 >
cebe5e00 3629319856 S Bi:1:003:1 -115 13 <
cebe5e00 3629319910 C Bi:1:003:1 0 13 = 55534253 1200  01
cebe5e00 3629319964 S Bo:1:003:2 -115 31 = 55534243 1300 1200
8603 0012   00
cebe5e00 3629319996 C Bo:1:003:2 0 31 >
cecd4300 3629320026 S Bi:1:003:1 -115 18 <
cecd4300 3629320086 C Bi:1:003:1 0 18 = 7500 000a 
2000 
cebe5e00 3629320111 S Bi:1:003:1 -115 13 <
cebe5e00 3629320152 C Bi:1:003:1 0 13 = 55534253 1300  00
cebe5e00 3629320360 S Bo:1:003:2 -115 31 = 55534243 1400 
0a35    00
cebe5e00 3629320610 C Bo:1:003:2 0 31 >
cebe5e00 3629320670 S Bi:1:003:1 -115 13 <
cebe5e00 3629320714 C Bi:1:003:1 0 13 = 55534253 1400  01
cebe5e00 3629320752 S Bo:1:003:2 -115 31 = 55534243 1500 1200
8603 0012   00
cebe5e00 3629320809 C Bo:1:003:2 0 31 >
cecd4300 3629320834 S Bi:1:003:1 -115 18 <
cecd4300 3629320863 C Bi:1:003:1 0 18 = 7500 000a 
2000 
cebe5e00 3629320885 S Bi:1:003:1 -115 13 <
cebe5e00 3629320923 C Bi:1:003:1 0 13 = 55534253 1500  00
ceb36280 3629350243 C Ii:1:001:1 -2:2048 0
cecd4800 3630011191 S Ci:1:001:0 s a3 00  0001 0004 4 <
cecd4800 3630011333 C Ci:1:001:0 0 4 = 07051200
cecd4800 3630011574 S Co:1:001:0 s 23 01 0011 0001  0
cecd4800 3630011699 C Co:1:001:0 0 0
cecd4800 3630011745 S Co:1:001:0 s 23 01 0014 0001  0
cecd4800 3630011789 C Co:1:001:0 0 0
ceb36280 3630112579 S Ii:1:001:1 -115:2048 4 <
cecd4500 3630112825 S Ci:1:001:0 s a3 00  0001 0004 4 <
cecd4500 3630112952 C Ci:1:001:0 0 4 = 0705
cecd4500 3630113001 S Co:1:001:0 s 23 01 0002 0001  0
cecd4500 3630113084 C Co:1:001:0 0 0
ceb36280 3630122552 C Ii:1:001:1 0:2048 1 = 02
ceb36280 3630122566 S Ii:1:001:1 -115:2048 4 <
cecd4500 3630152573 S Ci:1:001:0 s a3 00  0001 0004 4 <
cecd4500 3630152625 C Ci:1:001:0 0 4 = 0305
cecd4500 3630172588 S Ci:1:003:0 s 80 00   0002 2 <
cec

Suspend issues with a LaCie USB hard disk connected

[Daniel Mack]

Hi,

I'm facing an issue putting an embedded system to sleep while a Lacie
external USB hard disk is connected. Relevant kernel messages that occur
at the attempt are:

[   13.834731] PM: Sending message for entering DeepSleep mode
[   13.846575] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[   13.858818] sd 0:0:0:0: [sda]
[   13.862432] Result: hostbyte=0x00 driverbyte=0x08
[   13.867349] sd 0:0:0:0: [sda]
[   13.870626] Sense Key : 0x5 [current]
[   13.874602] sd 0:0:0:0: [sda]
[   13.877879] ASC=0x20 ASCQ=0x0
[   13.885053] dpm_run_callback(): scsi_bus_suspend+0x0/0x20 returns -5
[   13.901130] PM: Device 0:0:0:0 failed to suspend async: error -5
[   13.907507] PM: Some devices failed to suspend, or early wake event
detected

What happens is that in sd_sync_cache(), scsi_execute_req_flags()
returns 0x0802, so driver_byte(res) evaluates to DRIVER_SENSE and
host_byte(res) is DID_OK, which is an unhandled case that leads to -EIO
eventually.

I have admittedly not much clue about the SCSI layer, so I wonder what
would be the best way to fix this. Should DID_OK just be handled as
non-error condition in the switch? Should the suspend call chain ignore
such errors from sd_sync_cache()?

I'm open to suggestions and happy to test patches.


Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hello dear!

[Daniel Fung]

Hello dear!

Please kindly send us your contact infomations to our email: (dan.f...@aim.com) 
to enable us send your voucher.

Kind regards
Daniel F
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

It is Private

[Roussel, Daniel]

It is Private

I am George Daniels, a Banker and credit system programmer (HSBC bank).
I saw your email address while browsing through  the bank D.T.C Screen in my 
office yesterday so I decided to use this very chance to know you. I believe we 
should use every opportunity to know each other better. However, I am 
contacting you for obvious reason which you will understand.

I am sending this mail just to know if this email address is OK,
reply me back so that I will send  more details to you.
I have a very important thing to discuss with you, I look forward to receiving 
your response at
georgedani...@postino.net. Have a pleasant 
day.

George Daniels
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Fwd: Re: New USB storage device, not detected by Ubuntu 12.10 (kernel 3.5.0-17-generic)

[Daniel]

Hi linux-scsi people,

the problem described (and worked around) during the conversation below seems to
be related to your realm.
I apologize if this is redundant and for not taking my own time to suggest a
patch for it. However, as it affects users, I still wanted to get it out of my
backlog and at least mention it here.

Cheers

Daniel

 Original Message 
Subject: Re: New USB storage device, not detected by Ubuntu 12.10 (kernel
3.5.0-17-generic)
Date: Sun, 27 Jan 2013 13:22:42 -0800
From: Matthew Dharm 
To: Daniel 

I don't know.  You would need to ask the linux-scsi mailing lists.

Matt

On Sun, Jan 27, 2013 at 3:48 AM, Daniel  wrote:
> Hi Matt,
>
> the delay_use parameter to usb-storage doesn't solve the problem. It seems
> like the delay has to be added to scsi_mod via scsi_inq_timeout=25 for this
> harddrive to be detected properly (and this obviously needs to be set
> already in the initramfs when scsi_mod is initially loaded).
> Is there any way scsi_inq_timeout can be raised on a per device/vendor
> basis?
>
> Thank you for your support!
>
>
> Cheers
>
>
> Daniel
>
>
>
> On 27.01.2013 02:33, Matthew Dharm wrote:
>>
>> In that case, this is an easy fix.  There is already a parameter for this.
>>
>> When you load the usb-storage module, add the "delay_use=5" parameter
>> -- that will add a 5-second delay before scanning for the device.
>>
>> Through much of 2.6.x, the default for this parameter was 5 seconds.
>> Later is was changed to 1 second (I guess people didn't like to wait).
>>  Try playing around with the value to find something you like, and
>> then you can configure that to be the default via modprobe.conf
>>
>> Matt
>>
>> On Sat, Jan 26, 2013 at 10:22 AM, Daniel  wrote:
>>>
>>> Hi again,
>>>
>>> I just figured out that calling
>>> echo  "scsi add-single-device 8 0 0 0" > /proc/scsi/scsi
>>> (with 8 being the usb-storage controller for that device)
>>> makes it work if called a few seconds after adding the device.
>>> Probably the waiting time before probing is not high enough and a quirk
>>> for that
>>> device needs to be added.
>>>
>>> Cheers
>>>
>>>
>>> Daniel
>>>
>>> On 01/26/2013 07:54 PM, Matthew Dharm wrote:
>>>>
>>>> Is this a kernel you compiled yourself?
>>>>
>>>> Did you compile in the SCSI disk modules?
>>>>
>>>> Are you saying that /proc/scsi/scsi is empty?
>>>>
>>>> You may have to compile with USB Storage Verbose Debug enabled and
>>>> capture a log from that...
>>>>
>>>> Matt
>>>>
>>>> On Sat, Jan 26, 2013 at 9:41 AM, Daniel  wrote:
>>>>>
>>>>> Hi Matt,
>>>>>
>>>>> thank you for getting back to me after just a few minutes! You are
>>>>> doing an
>>>>> amazing job!
>>>>> So this is a Transcend StoreJet 1TB external 2.5" HDD connected via
>>>>> USB.
>>>>> I meanwhile also tried it on another machine which runs kernel
>>>>> 3.8.0rc1, the
>>>>> result is the same.
>>>>> It shows up as a usb-sata-bridge and usb-storage seems to detect it.
>>>>> However, no connected SCSI device shows up in /proc/scsi/scsi .
>>>>> If you need any more information or if I can assist in any way (I'm
>>>>> familiar
>>>>> with compiling the kernel form source)
>>>>>
>>>>> Thank you much!
>>>>>
>>>>>
>>>>> Daniel
>>>>>
>>>>>
>>>>> from lsusb -v
>>>>> Bus 002 Device 004: ID 152d:2329 JMicron Technology Corp. / JMicron USA
>>>>> Technology Corp. JM20329 SATA Bridge
>>>>> Device Descriptor:
>>>>>   bLength18
>>>>>   bDescriptorType 1
>>>>>   bcdUSB   2.00
>>>>>   bDeviceClass0 (Defined at Interface level)
>>>>>   bDeviceSubClass 0
>>>>>   bDeviceProtocol 0
>>>>>   bMaxPacketSize064
>>>>>   idVendor   0x152d JMicron Technology Corp. / JMicron USA
>>>>> Technology Corp.
>>>>>   idProduct  0x2329 JM20329 SATA Bridge
>>>>>   bcdDevice0.00
>>>>>   iManufacturer   1
>>>>>   iProduct  

Re: [PATCH 1/2] megaraid: fix BUG_ON() from incorrect use of delayed work

[Daniel Vacek]

On Wed, Dec 5, 2012 at 12:02 PM, Daniel Vacek  wrote:
> On Tue, Dec 4, 2012 at 4:54 PM, Tejun Heo  wrote:
>> @@ -5190,7 +5188,7 @@ static void
>>  megasas_aen_polling(struct work_struct *work)
>>  {
>> struct megasas_aen_event *ev =
>> -   container_of(work, struct megasas_aen_event, hotplug_work);
>> +   container_of(work, struct megasas_aen_event, 
>> hotplug_work.work);
>> struct megasas_instance *instance = ev->instance;
>> union megasas_evt_class_locale class_locale;
>> struct  Scsi_Host *host;
>
> -megasas_aen_polling(struct work_struct *work)
> +megasas_aen_polling(struct delayed_work *work)
>
> Not really sure. Just asking?

Never mind, it's ok. Now I get it. Sorry for the buzz.

--nX
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] megaraid: fix BUG_ON() from incorrect use of delayed work

[Daniel Vacek]

On Tue, Dec 4, 2012 at 4:54 PM, Tejun Heo  wrote:
> @@ -5190,7 +5188,7 @@ static void
>  megasas_aen_polling(struct work_struct *work)
>  {
> struct megasas_aen_event *ev =
> -   container_of(work, struct megasas_aen_event, hotplug_work);
> +   container_of(work, struct megasas_aen_event, 
> hotplug_work.work);
> struct megasas_instance *instance = ev->instance;
> union megasas_evt_class_locale class_locale;
> struct  Scsi_Host *host;

-megasas_aen_polling(struct work_struct *work)
+megasas_aen_polling(struct delayed_work *work)

Not really sure. Just asking?

--nX
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Bug 48241] New: oops when setting up LVM (3.6.0-next-20121003)

[Daniel Santos]

On 10/03/2012 10:23 AM, James Bottomley wrote:
> On Wed, 2012-10-03 at 15:04 +, bugzilla-dae...@bugzilla.kernel.org
> wrote:
>> https://bugzilla.kernel.org/show_bug.cgi?id=48241
>>
>>Summary: oops when setting up LVM
>>Product: IO/Storage
>>Version: 2.5
>> Kernel Version: 3.6.0-next-20121003
>>   Platform: All
>> OS/Version: Linux
>>   Tree: Mainline
>> Status: NEW
>>   Severity: normal
>>   Priority: P1
>>  Component: SCSI
>> AssignedTo: linux-scsi@vger.kernel.org
>> ReportedBy: daniel.san...@pobox.com
>> Regression: No
>>
>>
>> Created an attachment (id=81921)
>>  --> (https://bugzilla.kernel.org/attachment.cgi?id=81921)
>> image of oops
>
> The image says the RIP is at kthread_data + 0xb
>
> That implies something went wrong within the workqueue or kthread
> systems, I've cc'd linux-kernel, but it's a bit of a vague thing to go
> on and could conceivably be a hardware issue (or some weird thread
> interaction in linux-next).
>
> The first question would be "does it happen in vanilla 3.6"?
>
> James
So just to CC LKML, works in vanilla 3.6.0, happens in both -next & -mm,
tried compiling with both gcc 4.6.3 & 4.7.1.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Patch added to scsi-rc-fixes-2.6: [SCSI] arcmsr: fix message allocation

[Daniel Drake]

nickcheng wrote:

Hi,
I definitely agree it is in atomic context but why is the memory not for
DMA?
Would you please show me why?


It would probably be easier if you could explain where you believe the 
memory IS used for DMA :)


Anyway, looking at ARCMSR_MESSAGE_READ_RQBUFFER
current code does this:
ver_addr = kmalloc(1032, GFP_ATOMIC);

Here are the cases when that buffer is used:

checking for successful malloc: not DMA
if (!ver_addr) {

copying the address: not DMA
ptmpQbuffer = ver_addr;

memcpying 1 byte into the buffer: not DMA
memcpy(ptmpQbuffer, pQbuffer, 1);

incrementing address: not DMA
ptmpQbuffer++;

memcpying from the buffer: not DMA
memcpy(pcmdmessagefld->messagedatabuffer, ver_addr, 
allxfer_len);

freeing the buffer: not DMA
kfree(ver_addr);

Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] arcmsr: fix message allocation

[Daniel Drake]

arcmsr_iop_message_xfer() is called from atomic context under the
queuecommand scsi_host_template handler. James Bottomley pointed out
that the current GFP_KERNEL|GFP_DMA flags are wrong: firstly we are in
atomic context, secondly this memory is not used for DMA.
Also removed some unneeded casts.

Signed-off-by: Daniel Drake <[EMAIL PROTECTED]>
---
 drivers/scsi/arcmsr/arcmsr_hba.c |   26 +++---
 1 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 4f9ff32..f91f79c 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -1387,18 +1387,16 @@ static int arcmsr_iop_message_xfer(struct 
AdapterControlBlock *acb, \
switch(controlcode) {
 
case ARCMSR_MESSAGE_READ_RQBUFFER: {
-   unsigned long *ver_addr;
+   unsigned char *ver_addr;
uint8_t *pQbuffer, *ptmpQbuffer;
int32_t allxfer_len = 0;
-   void *tmp;
 
-   tmp = kmalloc(1032, GFP_KERNEL|GFP_DMA);
-   ver_addr = (unsigned long *)tmp;
-   if (!tmp) {
+   ver_addr = kmalloc(1032, GFP_ATOMIC);
+   if (!ver_addr) {
retvalue = ARCMSR_MESSAGE_FAIL;
goto message_out;
}
-   ptmpQbuffer = (uint8_t *) ver_addr;
+   ptmpQbuffer = ver_addr;
while ((acb->rqbuf_firstindex != acb->rqbuf_lastindex)
&& (allxfer_len < 1031)) {
pQbuffer = &acb->rqbuffer[acb->rqbuf_firstindex];
@@ -1427,26 +1425,24 @@ static int arcmsr_iop_message_xfer(struct 
AdapterControlBlock *acb, \
}
arcmsr_iop_message_read(acb);
}
-   memcpy(pcmdmessagefld->messagedatabuffer, (uint8_t *)ver_addr, 
allxfer_len);
+   memcpy(pcmdmessagefld->messagedatabuffer, ver_addr, 
allxfer_len);
pcmdmessagefld->cmdmessage.Length = allxfer_len;
pcmdmessagefld->cmdmessage.ReturnCode = 
ARCMSR_MESSAGE_RETURNCODE_OK;
-   kfree(tmp);
+   kfree(ver_addr);
}
break;
 
case ARCMSR_MESSAGE_WRITE_WQBUFFER: {
-   unsigned long *ver_addr;
+   unsigned char *ver_addr;
int32_t my_empty_len, user_len, wqbuf_firstindex, 
wqbuf_lastindex;
uint8_t *pQbuffer, *ptmpuserbuffer;
-   void *tmp;
 
-   tmp = kmalloc(1032, GFP_KERNEL|GFP_DMA);
-   ver_addr = (unsigned long *)tmp;
-   if (!tmp) {
+   ver_addr = kmalloc(1032, GFP_ATOMIC);
+   if (!ver_addr) {
retvalue = ARCMSR_MESSAGE_FAIL;
goto message_out;
}
-   ptmpuserbuffer = (uint8_t *)ver_addr;
+   ptmpuserbuffer = ver_addr;
user_len = pcmdmessagefld->cmdmessage.Length;
memcpy(ptmpuserbuffer, pcmdmessagefld->messagedatabuffer, 
user_len);
wqbuf_lastindex = acb->wqbuf_lastindex;
@@ -1492,7 +1488,7 @@ static int arcmsr_iop_message_xfer(struct 
AdapterControlBlock *acb, \
retvalue = ARCMSR_MESSAGE_FAIL;
}
}
-   kfree(tmp);
+   kfree(ver_addr);
}
break;
 
-- 
1.5.4

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: arcmsr + archttp64 calls dma_free_coherent() with irqs disabled - dmesg filled with warnings

[Daniel Drake]

Daniel Drake wrote:

Here is a patch to address your comments.
Joshua, would you mind testing this before I submit it properly? It will 
apply cleanly to 2.6.24 on top of the previous patch you tested. I have 
compile-tested it.


It would help to include the patch.
>From 0a9cd6133fe4f0c3a8906d6be1b9d1ef083345dc Mon Sep 17 00:00:00 2001
From: Daniel Drake <[EMAIL PROTECTED]>
Date: Sat, 16 Feb 2008 23:25:02 +
Subject: [PATCH] arcmsr: fix message allocation

---
 drivers/scsi/arcmsr/arcmsr_hba.c |   26 +++---
 1 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 4f9ff32..f91f79c 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -1387,18 +1387,16 @@ static int arcmsr_iop_message_xfer(struct 
AdapterControlBlock *acb, \
switch(controlcode) {
 
case ARCMSR_MESSAGE_READ_RQBUFFER: {
-   unsigned long *ver_addr;
+   unsigned char *ver_addr;
uint8_t *pQbuffer, *ptmpQbuffer;
int32_t allxfer_len = 0;
-   void *tmp;
 
-   tmp = kmalloc(1032, GFP_KERNEL|GFP_DMA);
-   ver_addr = (unsigned long *)tmp;
-   if (!tmp) {
+   ver_addr = kmalloc(1032, GFP_ATOMIC);
+   if (!ver_addr) {
retvalue = ARCMSR_MESSAGE_FAIL;
goto message_out;
}
-   ptmpQbuffer = (uint8_t *) ver_addr;
+   ptmpQbuffer = ver_addr;
while ((acb->rqbuf_firstindex != acb->rqbuf_lastindex)
&& (allxfer_len < 1031)) {
pQbuffer = &acb->rqbuffer[acb->rqbuf_firstindex];
@@ -1427,26 +1425,24 @@ static int arcmsr_iop_message_xfer(struct 
AdapterControlBlock *acb, \
}
arcmsr_iop_message_read(acb);
}
-   memcpy(pcmdmessagefld->messagedatabuffer, (uint8_t *)ver_addr, 
allxfer_len);
+   memcpy(pcmdmessagefld->messagedatabuffer, ver_addr, 
allxfer_len);
pcmdmessagefld->cmdmessage.Length = allxfer_len;
pcmdmessagefld->cmdmessage.ReturnCode = 
ARCMSR_MESSAGE_RETURNCODE_OK;
-   kfree(tmp);
+   kfree(ver_addr);
}
break;
 
case ARCMSR_MESSAGE_WRITE_WQBUFFER: {
-   unsigned long *ver_addr;
+   unsigned char *ver_addr;
int32_t my_empty_len, user_len, wqbuf_firstindex, 
wqbuf_lastindex;
uint8_t *pQbuffer, *ptmpuserbuffer;
-   void *tmp;
 
-   tmp = kmalloc(1032, GFP_KERNEL|GFP_DMA);
-   ver_addr = (unsigned long *)tmp;
-   if (!tmp) {
+   ver_addr = kmalloc(1032, GFP_ATOMIC);
+   if (!ver_addr) {
retvalue = ARCMSR_MESSAGE_FAIL;
goto message_out;
}
-   ptmpuserbuffer = (uint8_t *)ver_addr;
+   ptmpuserbuffer = ver_addr;
user_len = pcmdmessagefld->cmdmessage.Length;
memcpy(ptmpuserbuffer, pcmdmessagefld->messagedatabuffer, 
user_len);
wqbuf_lastindex = acb->wqbuf_lastindex;
@@ -1492,7 +1488,7 @@ static int arcmsr_iop_message_xfer(struct 
AdapterControlBlock *acb, \
retvalue = ARCMSR_MESSAGE_FAIL;
}
}
-   kfree(tmp);
+   kfree(ver_addr);
}
break;
 
-- 
1.5.4

Re: arcmsr + archttp64 calls dma_free_coherent() with irqs disabled - dmesg filled with warnings

[Daniel Drake]

James Bottomley wrote:

On Sat, 2008-02-16 at 11:49 +, Daniel Drake wrote:
I assume you're aware that this patch is just a subset of commit 
76d78300a6eb8 which you've already pushed up to Linus. Adding Nick Cheng 
(commit author) to CC so that he can go over the feedback.


Well, in case it's not obvious by now:  The way to get bad code upstream
is to send a patch that combines many changes (the more the better) so
that any potential reviewer has no idea which change is meant by which
hunk and then to make sure Andrew picks it up so he'll hound the
subsystem Maintainer until it's applied.  Best of all, mention that it
fixes a bug and you're made.


Sorry, I didn't mean to sound that as a criticism. I'm sure you have a 
lot of patches flowing by you at any one time.


Here is a patch to address your comments.
Joshua, would you mind testing this before I submit it properly? It will 
apply cleanly to 2.6.24 on top of the previous patch you tested. I have 
compile-tested it.



The odd thing is, it should have triggered a might_sleep() warning under
testing ... do you know why it didn't?


No, and I can see that scsi_dispatch_command does invoke ->queuecommand 
under a spinlock so it must be atomic context...
I'm not sure which might_sleep() codepath you are looking at though. At 
a guess it depends on SLUB vs SLAB?


Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: arcmsr + archttp64 calls dma_free_coherent() with irqs disabled - dmesg filled with warnings

[Daniel Drake]

I assume you're aware that this patch is just a subset of commit 
76d78300a6eb8 which you've already pushed up to Linus. Adding Nick Cheng 
(commit author) to CC so that he can go over the feedback.


James Bottomley wrote:

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index f4a202e..4f9ff32 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -1380,12 +1388,13 @@ static int arcmsr_iop_message_xfer(struct 
AdapterControlBlock *acb, \
 
 	case ARCMSR_MESSAGE_READ_RQBUFFER: {

unsigned long *ver_addr;
-   dma_addr_t buf_handle;
uint8_t *pQbuffer, *ptmpQbuffer;
int32_t allxfer_len = 0;
+   void *tmp;
 
-		ver_addr = pci_alloc_consistent(acb->pdev, 1032, &buf_handle);

-   if (!ver_addr) {
+   tmp = kmalloc(1032, GFP_KERNEL|GFP_DMA);

GFP_DMA is pretty pointless for a buffer which never actually gets anywhere 
near a DMA, isn't it?


+   ver_addr = (unsigned long *)tmp;

No cast needed from void *


+   if (!tmp) {
retvalue = ARCMSR_MESSAGE_FAIL;
goto message_out;
}
@@ -1421,18 +1430,19 @@ static int arcmsr_iop_message_xfer(struct 
AdapterControlBlock *acb, \
memcpy(pcmdmessagefld->messagedatabuffer, (uint8_t *)ver_addr, 
allxfer_len);
pcmdmessagefld->cmdmessage.Length = allxfer_len;
pcmdmessagefld->cmdmessage.ReturnCode = 
ARCMSR_MESSAGE_RETURNCODE_OK;
-   pci_free_consistent(acb->pdev, 1032, ver_addr, buf_handle);
+   kfree(tmp);
}
break;
 
 	case ARCMSR_MESSAGE_WRITE_WQBUFFER: {

unsigned long *ver_addr;
-   dma_addr_t buf_handle;
int32_t my_empty_len, user_len, wqbuf_firstindex, 
wqbuf_lastindex;
uint8_t *pQbuffer, *ptmpuserbuffer;
+   void *tmp;
 
-		ver_addr = pci_alloc_consistent(acb->pdev, 1032, &buf_handle);

-   if (!ver_addr) {
+   tmp = kmalloc(1032, GFP_KERNEL|GFP_DMA);


Actually, also all the code around here implies we're in atomic context,
so that GFP_KERNEL can't be right either.


Further confirmed by the fact that dma_free_coherent() was complaining 
about IRQs being disabled.



-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: arcmsr + archttp64 calls dma_free_coherent() with irqs disabled - dmesg filled with warnings

[Daniel Drake]

Joshua Hoblitt wrote:

I've got a few compilation errors on head.  Before I try it, is it
possible to backport the 76d78300a6eb8b7f08e47703b7e68a659ffc2053 change
to 2.6.24.y?


Please just try this patch instead, it's what I am planning to submit to 
-stable and add to Gentoo if you report success.


Daniel

From: Nick Cheng <[EMAIL PROTECTED]>

Partial backport of 76d78300 ("arcmsr: updates (1.20.00.15)") by
Daniel Drake <[EMAIL PROTECTED]>. Removes pci_alloc_consistent usage, which
should not be used when IRQs are disabled for portability reasons. Fixes a
spew of dma_alloc_coherent irqs_disabled() warnings.

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index f4a202e..4f9ff32 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -1380,12 +1388,13 @@ static int arcmsr_iop_message_xfer(struct 
AdapterControlBlock *acb, \
 
case ARCMSR_MESSAGE_READ_RQBUFFER: {
unsigned long *ver_addr;
-   dma_addr_t buf_handle;
uint8_t *pQbuffer, *ptmpQbuffer;
int32_t allxfer_len = 0;
+   void *tmp;
 
-   ver_addr = pci_alloc_consistent(acb->pdev, 1032, &buf_handle);
-   if (!ver_addr) {
+   tmp = kmalloc(1032, GFP_KERNEL|GFP_DMA);
+   ver_addr = (unsigned long *)tmp;
+   if (!tmp) {
retvalue = ARCMSR_MESSAGE_FAIL;
goto message_out;
}
@@ -1421,18 +1430,19 @@ static int arcmsr_iop_message_xfer(struct 
AdapterControlBlock *acb, \
memcpy(pcmdmessagefld->messagedatabuffer, (uint8_t *)ver_addr, 
allxfer_len);
pcmdmessagefld->cmdmessage.Length = allxfer_len;
pcmdmessagefld->cmdmessage.ReturnCode = 
ARCMSR_MESSAGE_RETURNCODE_OK;
-   pci_free_consistent(acb->pdev, 1032, ver_addr, buf_handle);
+   kfree(tmp);
}
break;
 
case ARCMSR_MESSAGE_WRITE_WQBUFFER: {
unsigned long *ver_addr;
-   dma_addr_t buf_handle;
int32_t my_empty_len, user_len, wqbuf_firstindex, 
wqbuf_lastindex;
uint8_t *pQbuffer, *ptmpuserbuffer;
+   void *tmp;
 
-   ver_addr = pci_alloc_consistent(acb->pdev, 1032, &buf_handle);
-   if (!ver_addr) {
+   tmp = kmalloc(1032, GFP_KERNEL|GFP_DMA);
+   ver_addr = (unsigned long *)tmp;
+   if (!tmp) {
retvalue = ARCMSR_MESSAGE_FAIL;
goto message_out;
}
@@ -1482,7 +1492,7 @@ static int arcmsr_iop_message_xfer(struct 
AdapterControlBlock *acb, \
retvalue = ARCMSR_MESSAGE_FAIL;
}
}
-   pci_free_consistent(acb->pdev, 1032, ver_addr, 
buf_handle);
+   kfree(tmp);
}
break;
 

Re: [PATCH] sr: update to follow tray status correctly

[Daniel Drake]

James Bottomley wrote:

However, I think you're right, the vanilla TUR does eat NOT_READY for
removable media, which CDs are.  Does this fix it?


Works great, thanks!

Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sr: update to follow tray status correctly

[Daniel Drake]

Hi James,

James Bottomley wrote:

It's been a while with no status on this one, so I corrected the patch
based on my original input.  The attached is what I think is the best
way of doing this (I've replaced the home grown test_unit_ready routine
with the SCSI one and updated some of the sense check conditions).


Many thanks for following up on this. The user has not responded to my 
request for him to test your patch, so I decided to try it myself. I'm 
using Linus git as of today, I noticed the patch is merged.


It doesn't work for me. On unpatched 2.6.24, I get the behaviour 
previously described - the ioctl always gives CDS_TRAY_OPEN regardless 
of tray status. On 2.6.24-git which includes your patch, I instead 
always get CDS_DISC_OK. I have confirmed that on 2 systems this is 
because scsi_test_unit_ready() is returning 0.


Before I dig further, does this work for you? I have attached the test 
program I am using.


Thanks!
Daniel

#include 
#include 
#include 
#include 
#include 
#include 

int main(void)
{
int fd = open("/dev/sr0", O_RDONLY | O_NONBLOCK);
int ret;
if (fd < 0) {
perror("open");
return 1;
}

ret = ioctl(fd, CDROM_DRIVE_STATUS, 0);
printf("ioctl result %d\n", ret);

switch(ret) {
case CDS_NO_INFO: printf("CDS_NO_INFO\n"); break;
case CDS_NO_DISC: printf("CDS_NO_DISC\n"); break;
case CDS_TRAY_OPEN: printf("CDS_TRAY_OPEN\n"); break;
case CDS_DRIVE_NOT_READY: printf("CDS_DRIVE_NOT_READY\n"); 
break;
case CDS_DISC_OK: printf("CDS_DISC_OK\n"); break;
default: printf("Unknown\n"); break;
}
return 0;
}

Perc5i / MegaRaid / Linux SCSI subsystem issue

[Daniel Jabbour]

Hi,

I've recently acquired some Dell PowerEdge 2950 servers with integrated Perc 5i 
controllers (which use the MegaRaid driver).  I loaded Fedora Core 6 on them 
fine, did a yum update and much to my surprise the box kernel panics on boot.  
I haven't loaded anything else on the box other than a stock OS.

I did some investigation and determined the issue only occurs with the latest 
(2.6.22.1-32) kernel in FC6.  It did not occur with 2.6.20 or 2.6.18.  It seems 
that the driver for the Perc 5i controller (megaraid_sas) is seeing through the 
BIOS of the controller and showing all 6 disks rather than one logical volume 
in the startup messages (I'm seeing SD 0-5).  Since the volume is actually RAID 
10, it cannot mount the root filesystem.

This problem seem very similar to one detailed in an older kernel on the Ubuntu 
bug tracker:
https://bugs.launchpad.net/ubuntu/+source/linux-source-2.6.15/+bug/55138

I also noticed that the bug has a patch applied in Ubuntu:
https://launchpad.net/ubuntu/+source/linux-source-2.6.15/+bug/57265


I have done some further diagnosis:

- I loaded the kernel.org sources for 2.6.22.1, with no luck, same issue.

- I downloaded the megaraid drivers from Dell and tried installing them for 
2.6.22.1.  It prompted me that the drivers were older than the ones in 2.6.22, 
I forced it to install, rebuilt the initrd, and the same issue continues.  This 
leads me to believe that the issue might not be with the megaraid driver, but 
with something else in the kernel scsi subsystem, but I'm no kernel hacker.

What steps need to be taken to get this resolved?  I tried involving Dell, and 
have an open case with them, but since 2.6.22 isn't in RHEL yet, they say they 
have no escalation path and can't help.  Any suggestions from the list?  Thanks,

--
Daniel Jabbour
Network / Sytems Engineer
Intronis Technologies
Direct: (800) 569-0155 x242
Email: [EMAIL PROTECTED]
www.intronis.com


-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] 3w-xxxx: Add tw_sysfs_init() so udev will create /dev/twe* devices.

[Joshua Daniel Franklin]

--- adam radford wrote:
> I have one problem with this patch:
> 
> You are calling "class_device_create()" from 0 to "TW_MAX_UNITS"
> which is 16...
> shouldn't you be calling this only once per each controller instance
> that is discovered
> via twa_probe() ?

Well, I was just trying to emulate what the userspace tools do,
which is create all 16 devices whether or not they're physically
present. I actually tried with only twe0 (I have only one card in my
test system) and smartd seems to try to find twe1-15 anyway. I'm
not sure what it's doing, but I assume it's talking to the driver.

> Also, no other scsi LLDD drivers are calling "class_device_create()",
> so I wonder if
> this is acceptable to scsi maintainers?

I wondered the same thing. I think perhaps the only issue is with
SELinux mentioned below which is rare and so it's a low priority 
to use sysfs. There may be other reasons I'm not aware of, though.

> Right now we have our userspace tools creating the /dev/tweX devices.

Yes, unfortunately that leaves the /dev/tweX devices with the 
wrong SELinux security context and [EMAIL PROTECTED] suggested this
approach:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=232218
I'd be happy to hear alternative solutions, but it sounds like
they do not want to use a custom udev rule.


   

Get the free Yahoo! toolbar and rest assured with the added security of spyware 
protection.
http://new.toolbar.yahoo.com/toolbar/features/norton/index.php
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] 3w-xxxx: Add tw_sysfs_init() so udev will create /dev/twe* devices.

[Joshua Daniel Franklin]

Add tw_sysfs_init() to register 3w- driver with sysfs. This
also causes udev to create /dev/twe* devices with the correct
SELinux security context.

Signed-off-by: Joshua Daniel Franklin <[EMAIL PROTECTED]>
---



   

Be a better Heartthrob. Get better relationship answers from someone who knows. 
Yahoo! Answers - Check it out. 
http://answers.yahoo.com/dir/?link=list&sid=396545433--- 3w-.c.orig	2007-07-05 13:01:16.0 -0700
+++ 3w-.c	2007-07-06 15:32:08.0 -0700
@@ -194,6 +194,7 @@
1.26.02.002 - Free irq handler in __tw_shutdown().
  Turn on RCD bit for caching mode page.
  Serialize reset code.
+   1.26.02.003 - Add tw_sysfs_init() so udev will create /dev/twe* devices.
 */
 
 #include 
@@ -217,11 +218,13 @@
 #include "3w-.h"
 
 /* Globals */
-#define TW_DRIVER_VERSION "1.26.02.002"
+#define TW_DRIVER_VERSION "1.26.02.003"
 static TW_Device_Extension *tw_device_extension_list[TW_MAX_SLOT];
 static int tw_device_extension_count = 0;
 static int twe_major = -1;
 
+static struct class *tw_sysfs_class;
+
 /* Module parameters */
 MODULE_AUTHOR("AMCC");
 MODULE_DESCRIPTION("3ware Storage Controller Linux Driver");
@@ -1047,6 +1050,33 @@ static const struct file_operations tw_f
 	.release	= NULL
 };
 
+/* This function will create the twe* sysfs entries */
+static int tw_sysfs_init(void) {
+	int i;
+	struct class_device *tw_class_member;
+
+	twe_major = register_chrdev(0, "twe", &tw_fops);
+	if (twe_major < 0) {
+		printk(KERN_WARNING "3w-: Failed to register character device.\n");
+		return twe_major;
+	}
+	/* register twe* devices with sysfs */
+	tw_sysfs_class = class_create(THIS_MODULE, "twe");
+	if (IS_ERR(tw_sysfs_class)) {
+		printk(KERN_WARNING "3w-: Failed to create sysfs class.\n");
+		return PTR_ERR(tw_sysfs_class);
+	}
+
+	for(i = 0; i < TW_MAX_UNITS; i++) {
+		tw_class_member = class_device_create(tw_sysfs_class, NULL, MKDEV(twe_major,i), NULL, "twe%d",i);
+		if (IS_ERR(tw_class_member)) {
+			printk(KERN_WARNING "3w-: Unable to add sysfs class member twe%d\n", i);
+			return PTR_ERR(tw_class_member);
+		}
+	}
+	return 0;
+} /* End tw_sysfs_init() */
+
 /* This function will free up device extension resources */
 static void tw_free_device_extension(TW_Device_Extension *tw_dev)
 {
@@ -2420,8 +2450,9 @@ static int __devinit tw_probe(struct pci
 	scsi_scan_host(host);
 
 	if (twe_major == -1) {
-		if ((twe_major = register_chrdev (0, "twe", &tw_fops)) < 0)
-			printk(KERN_WARNING "3w-: Failed to register character device.");
+		if (tw_sysfs_init() < 0) {
+			printk(KERN_WARNING "3w-: Failed to initialize sysfs for device.");
+		}
 	}
 	return 0;
 
@@ -2449,6 +2480,7 @@ static void tw_remove(struct pci_dev *pd
 	/* Unregister character device */
 	if (twe_major >= 0) {
 		unregister_chrdev(twe_major, "twe");
+		class_destroy(tw_sysfs_class);
 		twe_major = -1;
 	}
 

Re: [PATCH] bsg: fix a blocking read bug

[Daniel . E . Messinger]

Thanks!   bsg now waits for a response.

Dan



   
 FUJITA Tomonori   
  To 
 No Phone Info [EMAIL PROTECTED]   
 Available  cc 
   linux-scsi@vger.kernel.org, 
   [EMAIL PROTECTED]  
 05/08/2007 08:21  Subject 
 AM[PATCH] bsg: fix a blocking read
   bug 
   
   
   
   
   
   




This patch fixes a bug that read() returns ENODATA even with a
blocking file descriptor when there are no commands pending.

This also includes some cleanups.

Signed-off-by: FUJITA Tomonori <[EMAIL PROTECTED]>

...


-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

bidi bsg is non-blocking

[Daniel . E . Messinger]

Greetings to all,

I'm attempting to use the bidi variant of bsg to talk to an OSD target
device. I've run into an undesirable situation.

My application has a free-running receive loop (doing a read() on the bsg
device) waiting for responses to commands sent to bsg by another thread.
The problem is that the receive thread is getting ENODATA from the read()
when there are no commands pending.  I have NOT set non-blocking.

Note that the old sg driver was quite willing to block until there was a
response available. I find this scenario greatly preferable.

Could bsg be fixed so that read() blocks when there is nothing to return?



Dan Messinger
Seagate Technology Research


-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SCSI: Update ASC list and make it const

[Daniel J. Priem]

"Martin K. Petersen" <[EMAIL PROTECTED]> writes:

>>>>>> "Daniel" == Daniel J Priem <[EMAIL PROTECTED]> writes:
>
> Daniel> - * http://www.t10.org/lists/asc-num.txt+ */ 
> Daniel> + * http://www.t10.org/lists/asc-num.txt */
>
> Daniel> its a typo.  
>
> The diff looks fine both here and in the archives.  Maybe your mailer
> mangled it?
Yes. you are right. i crosschecked against
http://permalink.gmane.org/gmane.linux.scsi/30188 ...

-- 
 .''`.
 : :' :  Daniel J. Priem
 `. `'   http://flexserv.de
   `-

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SCSI: Update ASC list and make it const

[Daniel J. Priem]

"Martin K. Petersen" <[EMAIL PROTECTED]> writes:
> + * The canonical list of T10 Additional Sense Codes is available at:
> + * http://www.t10.org/lists/asc-num.txt+ */

Please do a

- * http://www.t10.org/lists/asc-num.txt+ */
+ * http://www.t10.org/lists/asc-num.txt */

its a typo.
Regards
Daniel

-- 
 .''`.
 : :' :  Daniel J. Priem
 `. `'   http://flexserv.de
   `-

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Fwd: AIC79XX abort -- hardware fault?]

[Daniel Pittman]

James Bottomley <[EMAIL PROTECTED]> writes:
> On Tue, 2006-12-12 at 08:18 -0800, Sean Bruno wrote:
>> > G'day.  One of the machines I maintain is having real trouble with the
>> > AIC79XX HBA or the tape drive attached to it.  I believe this is a
>> > hardware fault, but I am not certain where the problem lies.
>> > 
>> > Normally I would blame the cable or, maybe, the tape drive, but the
>> > early stage of the fault and the reported SCSI driver state make me
>> > wonder if this is perhaps an HBA fault?
>
> The first question is what kernel version?  There was an aic79xx fix
> that went in just prior to 2.6.19 that might help with this.

Linux genesys 2.6.18-028test007.1-ovz 

That is the 2.6.18 kernel (with RHES configuration) plus the OpenVZ
virtualization patches.  Unfortunately, that latter is in active use on
the system and limits the kernel versions that this could be seen with.

The OpenVZ support doesn't touch drivers, so I don't believe it is
responsible for the issue; we are certainly able to reproduce it with
OpenVZ inactive on the system.


Further reading in the Linux SCSI mailing list suggests that the
'slowcrc' command line option has resolved a similar looking issue for
another user, and that ramping down the speed for the device may also
help.

I am happy to try and test any appropriate patches back-ported to that
kernel version, but will schedule in trying the slowcrc option and
arrange to ramp down the bus speed some time soon as well.

Thanks for your help, and to Sean for moving this to a more correct
location.

Regards,
Daniel
-- 
Digital Infrastructure Solutions -- making IT simple, stable and secure
Phone: 0401 155 707email: [EMAIL PROTECTED]
 http://digital-infrastructure.com.au/

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] SCSI: Don't be so verbose if no disc is present

[Daniel Drake]

Running a simple "touch /dev/scd0" on a SCSI cdrom will cause the following 
message to appear in the logs if no disc is present:


Device not ready. Make sure there is a disc in the drive.

This results in the logs being flooded for some users by those userspace 
agents which repeatedly poll the device for media change.


This patch demotes the message to a quieter scsi-logging message only.

Signed-off-by: Daniel Drake <[EMAIL PROTECTED]>

--- linux/drivers/scsi/scsi_ioctl.c.orig	2005-09-09 23:10:07.0 +0100
+++ linux/drivers/scsi/scsi_ioctl.c	2005-09-09 23:13:17.0 +0100
@@ -118,8 +118,9 @@ static int ioctl_internal_command(struct
 			break;
 		case NOT_READY:	/* This happens if there is no disc in drive */
 			if (sdev->removable && (cmd[0] != TEST_UNIT_READY)) {
-printk(KERN_INFO "Device not ready. Make sure"
-   " there is a disc in the drive.\n");
+SCSI_LOG_IOCTL(2, printk(
+   "Device not ready. Make sure"
+   " there is a disc in the drive.\n"));
 break;
 			}
 		case UNIT_ATTENTION:

[PATCH] Allow both megaraid drivers to be built

[Daniel Drake]

Christoph Hellwig wrote:

ifdef on other drivers is bogus, please remove the pci ids completely
from the old driver.  It'd probably be a good idea to rename it to
megaraid_legacy aswell.  I've also asked the LSI Engineers whether they
could help identifying what code could be remove when only supporting
the two oldes chips but haven't gotten a reply yet.  I hope they'll
be able to answer when they're less busy.


Thanks for the feedback. Here's a new patch.



Rename megaraid to megaraid_legacy, changing the filenames, sysfs name, module 
name, and the messages printed out via printk. I have left the procfs name as 
"megaraid" as presumably userspace tools rely on this.


Remove hardware ID's from megaraid_legacy which overlap with the newgen 
megaraid drivers.


Allow megaraid_legacy to be built alongside the newgen driver.

Signed-off-by: Daniel Drake <[EMAIL PROTECTED]>



As the patch is large (300K) I am not including it in this mail. You can get 
it here:


http://dev.gentoo.org/~dsd/megaraid_legacy.patch

Thanks,
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Allow both megaraid drivers to be built

[Daniel Drake]

Currently, the two megaraid drivers are exclusive - if you build the "new 
generation" megaraid driver then you aren't able to build the old one.


I've been investigating this as this is not ideal: the old driver supports a 
lot of common hardware which the new driver does not, and vice-versa.


The two drivers compile together ok but will clash at runtime, due to the same 
name in the pci_driver struct. However I presume that the main reason that 
these drivers exclude each other is because there is some hardware support 
overlap - the new driver supports some hardware which would also be claimed by 
the old, not a sensible thing to do if both can be built and run in parallel.


The hardware overlap isn't straightforward - the newgen driver supports mostly 
a specific set of pid/vid/sspid/ssvid whereas the legacy one supports pid/vid 
pairs and accepts _any_ subsystem id's.


For example, the legacy driver will claim any device with vendor=101E 
product=1960. The newgen driver also lists those ID numbers but only against 5 
different subsystem id pairs, it won't claim "the whole lot".


I have repeatedly tried to contact LSI Logic to find out more about this 
overlap. In the above example, are there devices which have the ID's 
vendor=101E product=1960 and a subsystem _not_ listed in the newgen driver? 
These devices would be claimed by the old driver and not the new - is there a 
reason for this - would they not work with the newgen driver? Do they even 
exist at all?


I have recieved no real response from LSIL so I have produced this patch, 
which has been included on Gentoo's latest release media where both drivers 
have been built alongside each other and we have not recieved any negative 
reports about it. (hopefully those legacy users are over the moon because they 
can install gentoo again!)


This patch makes the assumption that in the above example there are no 
101E/1960 adapters anywhere in the world that have subsystem ID's not listed 
in the newgen driver, and the same for the other device support overlaps. This 
results in the newgen driver being favoured (which apparently performs much 
better than the legacy on the devices which are supported by both) when both 
are built together -- however the device support in the old driver will be 
*unchanged* if you do not build the new one alongside it.


I know that this solution is not ideal, but as LSIL are being uncooperative, I 
think this is the best compromise we can do for now. Please apply.


Signed-off-by: Daniel Drake <[EMAIL PROTECTED]>
diff -urNp linux-2.6.12/drivers/scsi-orig/megaraid/Kconfig.megaraid linux-2.6.12/drivers/scsi/megaraid/Kconfig.megaraid
--- linux-2.6.12/drivers/scsi-orig/megaraid/Kconfig.megaraid	2005-06-29 16:19:55.0 +0100
+++ linux-2.6.12/drivers/scsi/megaraid/Kconfig.megaraid	2005-06-29 16:22:12.0 +0100
@@ -64,7 +64,6 @@ config MEGARAID_MAILBOX
 	To compile this driver as a module, choose M here: the
 	module will be called megaraid_mbox
 
-if MEGARAID_NEWGEN=n
 config MEGARAID_LEGACY
 	tristate "LSI Logic Legacy MegaRAID Driver"
 	depends on PCI && SCSI
@@ -75,4 +74,4 @@ config MEGARAID_LEGACY
 
 	To compile this driver as a module, choose M here: the
 	module will be called megaraid
-endif
+
diff -urNp linux-2.6.12/drivers/scsi-orig/megaraid.c linux-2.6.12/drivers/scsi/megaraid.c
--- linux-2.6.12/drivers/scsi-orig/megaraid.c	2005-06-29 16:19:55.0 +0100
+++ linux-2.6.12/drivers/scsi/megaraid.c	2005-06-30 13:07:41.0 +0100
@@ -5033,28 +5033,34 @@ megaraid_shutdown(struct device *dev)
 }
 
 static struct pci_device_id megaraid_pci_tbl[] = {
+#ifndef CONFIG_MEGARAID_NEWGEN
 	{PCI_VENDOR_ID_DELL, PCI_DEVICE_ID_DISCOVERY,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
 	{PCI_VENDOR_ID_DELL, PCI_DEVICE_ID_PERC4_DI,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, BOARD_64BIT},
 	{PCI_VENDOR_ID_LSI_LOGIC, PCI_DEVICE_ID_PERC4_QC_VERDE,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, BOARD_64BIT},
+#endif
 	{PCI_VENDOR_ID_AMI, PCI_DEVICE_ID_AMI_MEGARAID,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
 	{PCI_VENDOR_ID_AMI, PCI_DEVICE_ID_AMI_MEGARAID2,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
+#ifndef CONFIG_MEGARAID_NEWGEN
 	{PCI_VENDOR_ID_AMI, PCI_DEVICE_ID_AMI_MEGARAID3,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
+#endif
 	{PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_AMI_MEGARAID3,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
+#ifndef CONFIG_MEGARAID_NEWGEN
 	{PCI_VENDOR_ID_LSI_LOGIC, PCI_DEVICE_ID_AMI_MEGARAID3,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
+#endif
 	{0,}
 };
 MODULE_DEVICE_TABLE(pci, megaraid_pci_tbl);
 
 static struct pci_driver megaraid_pci_driver = {
-	.name		= "megaraid",
+	.name		= "megaraid_legacy",
 	.id_table	= megaraid_pci_tbl,
 	.probe		= megaraid_probe_one,
 	.remove		= __devexit_p(megaraid_remove_one),

RE: [PATCH 2.6.13-rc6] fix copying stack memory in cpqfcTScontrol.c::cpqfcTSPutLinkQue()

[Horák Daniel]

> First of all: since the request for a working version of this 
> driver is not
> longer there at the moment I'll only send out what I 
> currently have to get it
> into list archives. Merging this would be fine to decrease 
> the amount of bugs
> in this (and the size of it). I removed everyone but 
> linux-scsi from recipient
> list because of this. Removing the stack error is rather 
> simple now, but I 
> don't do it for now to make sure noone uses this beast until 
> it's fixed.

I really would like to see this driver working in 2.6, because it is the
only way how to connect old Compaq FC arrays. In one or two weeks I will
have a test hardware available, so I can help at least with testing the
driver.


Dan
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

scsi/libata bootup sysfs oops with 2.6.12 + 2.6.13-rc3

[Daniel Drake]

Hi,

After upgrading kernel, Edward (on CC) is having trouble booting up.

The kernel hangs after reporting it can't mount root on /dev/sda3, which is 
supposed to be a partition on a SATA disk, connected to a sata_nv controller.


As serial console is not available, we stripped down the kernel in hope that 
the SATA disk detection would appear on the same screen so that it could be 
caught on camera :)


After removing the more verbose parts of the kernel (USB, ACPI, etc) in 
attempt to get disk detection messages on the same screen, we ran into another 
issue. The kernel oops's on boot up, and tries to kill init. So its not even 
getting as far this time (last time, it got all the way to trying to mount root).


This problem did not exist in 2.6.10 which can still be booted right now. It 
is reprocable on both 2.6.12 and 2.6.13-rc3.


Under 2.6.10, these messages appear during disk detection:

ata1: SATA max UDMA/133 cmd 0x9F0 ctl 0xBF2 bmdma 0xF200 irq 23
ata2: SATA max UDMA/133 cmd 0x970 ctl 0xB72 bmdma 0xF208 irq 23
ata1: dev 0 cfg 49:2f00 82:7c6b 83:7b09 84:4003 85:7c69 86:3a01 87:4003 88:407f
ata1: dev 0 ATA, max UDMA/133, 240121728 sectors:
nv_sata: Primary device added
nv_sata: Primary device removed
nv_sata: Secondary device removed
ata1: dev 0 configured for UDMA/133
scsi0 : sata_nv
ata2: no device found (phy stat )
scsi1 : sata_nv
  Vendor: ATA   Model: Maxtor 6Y120M0Rev: YAR5
  Type:   Direct-Access  ANSI SCSI revision: 05
st: Version 20041025, fixed bufsize 32768, s/g segs 256
SCSI device sda: 240121728 512-byte hdwr sectors (122942 MB)
SCSI device sda: drive cache: write back
SCSI device sda: 240121728 512-byte hdwr sectors (122942 MB)
SCSI device sda: drive cache: write back
 /dev/scsi/host0/bus0/target0/lun0: p1 p2 p3
Attached scsi disk sda at scsi0, channel 0, id 0, lun 0
Attached scsi generic sg0 at scsi0, channel 0, id 0, lun 0,  type 0


Under a minimal 2.6.13-rc3, this happens instead:

ata1: SATA max UDMA/133 cmd 0x9F0 ctl 0xBF2 bmdma 0xF200 irq 0
ata2: SATA max UDMA/133 cmd 0x970 ctl 0xB72 bmdma 0xF208 irq 0
Unable to handle kernel NULL pointer dereference at <...> RIP: 
sysfs_hash_and_remove+16


PGD 0
Oops:  [1] SMP
CPU 0
Modules linked in:
Pid 1, comm: swapper Not tainted 2.6.13-rc3
RIP: sysfs_hash_and_remove+16

Call trace:
class_device_del   class_device_unregister
scsi_remove_host   setup_irq
request_irq   ata_host_remove
ata_device_add   pci_conf1_write
pcibios_set_master   nv_init_one
pci_device_probe   driver_probe_device
__driver_attach   __driver_attach
__driver_attach   bus_for_each_dev
bus_add_driver   pci_register_driver
init   child_rip
init   child_rip

I can provide a full jpeg if required.


It looks like dir->d_inode is null, although I don't have much idea where the 
real bug exists.


(gdb) list *sysfs_hash_and_remove+16
0x5b0 is in sysfs_hash_and_remove (semaphore.h:107).
102  * This is ugly, but we want the default case to fall through.
103  * "__down_failed" is a special asm handler that calls the C
104  * routine that actually waits. See arch/x86_64/kernel/semaphore.c
105  */
106 static inline void down(struct semaphore * sem)
107 {
108 might_sleep();
109
110 __asm__ __volatile__(
111 "# atomic down operation\n\t"


Any ideas or suggestions?

Thanks,
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: MO drives (2048 byte block vfat fs) in lk 2.4

[Daniel Kobras]

On Sun, Apr 22, 2001 at 10:59:18PM -0400, Douglas Gilbert wrote:
> One error report stated that a MO drive with a vfat
> fs based on 2048 byte sectors can be mounted and read

Read? I don't think so. bread, yes, but read follows a NULL pointer and
was never seen again.

> but any significant write causes a system lockup. I
> have been able to replicate this behaviour. Luckily
> Alt-SysRq-P did work. Pressing this sequence multiple
> times gave similar addresses. Rebooting the machine
> and rerunning the experiment multiple time gave 
> addresses in the same area.

bigblock_fat_bread() in fs/fat/buffer.c kmalloc()s 2k dummy bhs for each
512 byte buffer but only partly initialise them. This works as long as those
bogus bhs don't leave the fat realm. Unfortunately, generic_file_write and
friends call into the generic block layer that wants to do such evil things
on bhs as using their wait_queues or checking their state for BH_Locked.
None of which are initialised, and while I haven't checked in detail,
certainly lead the way into deadlock country. But even if these minor
disturbances magically didn't screw you yet, the fat layer will hand out
a buffer address calculated for 512 byte buffers for your 2k buffer, and your
data goes bye, bye.

The preferred fix seems to be to teach the loop device about reblocking and
rip all of the bigblock support from fat. I've spent this weekend to cure
my utter and complete ignorance of the blkdev layer, in order to be able to
implement a working set up at least. Please allow me another couple of days
to spare a little hacking time. I'll take care of the issue. Promised.

Regards,

Daniel.

-- 
GNU/Linux Audio Mechanics - http://www.glame.de
  Cutting Edge Office - http://www.c10a02.de
  GPG Key ID 89BF7E2B - http://www.keyserver.net
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]

PROBLEM: Linux 2.2.19 system crash with Oops in scsi_do_cmd during mirror rebuild on megaraid

[Daniel Deimert]

[1.] One line summary of the problem:

Linux 2.2.19 system crash with Oops in scsi_do_cmd during mirror rebuild
on megaraid

[2.] Full description of the problem/report:

Seconds after starting a rebuild of a mirrored partition on a megaraid
controller, the
system crashed. It might be reproducable but since this is a production
system I have
not attempted to do it again.

Before I attempted to rebuild the mirror set, I got several kernel
messages like this
kernel: scsidisk I/O error: dev 08:11, sector 71077808
This is on a mirrored partition that the megaraid utility verified to be
flawless.
After the crash, fsck could not repair the file system completely - when
it tried
to write new block descriptors, it got "short write" write errors.
Again, the
megaraid bios said the mirror set was healthy and the individual disks
had not
returned any hardware errors. "mke2fs" had no problems to write
anywhere, and it
also stopped the error messages.

[3.] Keywords (i.e., modules, networking, kernel):

filesystem, scsi, megaraid, kupdate

[4.] Kernel version (from /proc/version):

Linux version 2.2.19 (root@localhost) (gcc version egcs-2.91.66
19990314/Linux (egcs-1.1.2 release)) #1 SMP Tue Apr 3 21:56:41 CEST 2001



[5.] Output of Oops.. message (if applicable) with symbolic information
 resolved (see Documentation/oops-tracing.txt)

I had to type this in by hand and some of it had scrolled of screen so
you will have
to forgive any minor errors--  couldn't you guys please put one of the
crash
dump patches into the mainstream kernel?


kernel: EFLAGS: 00010092
kernel: eax:    ebx: bff78078   ecx: bff7bba8   edx: bff7a534
kernel: esi: bff7b68c   edi: bff78800   ebp: bffdfe10   esp: bffdfdb8
kernel: ds: 0018   es: 0018   ss: 0018
kernel: Process kupdate (pid: ?, process nr: ?, stackpage=bffdf000)
kernel: Stack: 801edbbb bffbdc00 801f23fc 0b40 bffdfef0 bffbdc00
bffdfee8 bfff7400
kernel:0002    0001 
80222ef7 002f9956
kernel:63222f40 0009  bffd7aa0 8025480d bffdfe20
bf4462a0 801fa4e4
kernel: Call Trace: [scsi_do_cmd+1190/1264] [scsi_old_done+0/1404]
[__delay+19/36]
kernel: [RCSid+3757/51072] [requeue_sd_request+4072/4088]
[rw_intr+0/1604]
kernel: [scsi_do_cmd+1190/1264] [scsi_old_done+0/1404]
[tcp_v4_do_rcv+112/380]
kernel: [sys_pipe+12/120] [make_request+1870/1992]
[ll_rw_block+335/440]
kernel: [sync_old_buffers+214/452] [tvecs+12075/14016]
[kupdate+132/136]
kernel: [get_options+0/116] [kernel_thread+35/48]
kernel: Code: 80 78 45 81 0f 85 a0 00 00 00 c7 46 60 00 00 00 c7 46 64


[7.] Environment
DELL 6300
DELL PERC Megaraid firmware [U.84:1.63]
Red Hat Linux 6.2
Linux 2.2.19 stock tar.gz
Intel e100 network card driver module v1.5.5 loaded
Intel iANS network module v1.2.29 loaded

Gnu C  egcs-2.91.66
Gnu make   3.77
binutils   2.9.1.0.24
util-linux 2.10m
modutils   2.1.121
e2fsprogs  1.19
reiserfsprogs  3.x.0j
PPP2.3.10
Linux C Library2.1.3
Dynamic linker (ldd)   2.1.3
Procps 2.0.4
Net-tools  1.53
Console-tools  1999.03.02
Sh-utils   2.0
Modules Loaded e100 ians


[X.] Other notes


--
[EMAIL PROTECTED] Intermec Printers, Gothenburg, Sweden
http://www.intermec.com/



-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]

Re: [PATCH] consolidating data direction tables

[Daniel Eisenbud]

On Wed, Feb 14, 2001 at 08:16:30AM +0100, Oliver Neukum 
<[EMAIL PROTECTED]> wrote:
> 
> > However, in light of other email in this thread, it looks like maybe the
> > table can go altogether, so hopefully this question is now moot.
> 
> Oh yes, I saw the mail you are refering to.
> Let me put it this way: If we get rid of 'direction unknown'
> then I'd be comfortable with killing it.
> I'll put a printk into the drivers I use the test this.

I don't know if you looked at my patch, but all the drivers in question
assume that if they don't know a command is a write, then it's a read
command.  There are only eight drivers that do things this way, and they
don't seem to be particularly new ones, in general, though I might be
wrong about that with a few of them.  So I think that trusting what the
kernel thinks is the direction might be as good as trusting what the
driver thinks is the direction currently.  On the other hand, given that
MESH at least locks up hard when the driver guesses wrong, it's
important to verify that the kernel is right at least as often as the
drivers currently are about the direction.

-Daniel

-- 
Daniel E. Eisenbud
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]

Re: [PATCH] consolidating data direction tables

[Daniel Eisenbud]

On Tue, Feb 13, 2001 at 10:24:14AM +0100, Oliver Neukum 
<[EMAIL PROTECTED]> wrote:
> > > The attached patch does two things: it creates a new header file called
> > > scsi_dataout.h, which has a single copy of the switch statement (as a
> > > static function -- is that all right?) and is included by the relevant
> > > drivers.  I checked that all the drivers had exactly equivalent switch
> 
> This means that it would be compiled into the kernel several times.
> You should add it to the core scsi code and _not_ declare it static.

I considered doing it this way.  As I saw it, it's a tradeoff between
including the code even if none of the drivers in question are
configured in, or compiling it multiple times if several of them are.  I
guess there's also the possibility of an ifdef so that it will be
compiled if at least one of drivers is configured in.

However, in light of other email in this thread, it looks like maybe the
table can go altogether, so hopefully this question is now moot.

-Daniel

-- 
Daniel E. Eisenbud
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]

Re: [PATCH] consolidating data direction tables

[Daniel Eisenbud]

On Tue, Feb 13, 2001 at 09:40:36AM -0500, Douglas Gilbert <[EMAIL PROTECTED]> wrote:
> Daniel Eisenbud wrote:
> > 
> > Eight different SCSI drivers have large switch statements to determine
> > the direction in which data will be transferred for a given SCSI
> > command.  I discovered this when trying to figure out why the MESH
> > (powermac SCSI) driver locked up when (and only when) trying to burn a
> > CD-R in disk-at-once mode.  It turns out that there's a command that's
> > not in any of these tables (cdrecord refers to it as send_cue_sheet) and
> > it's absence is enough to kill the MESH driver (and apparently also the
> > mac53c94 driver.) 
> 
> Daniel,
> Does that driver take any account of Scsi_Cmnd::sc_data_direction_flag?
> All upper level drivers (including sg) in the lk 2.4 series set
> the corresponding Scsi_Request::sr_data_direction_flag .
[...]

The driver appears not to use sc_data_directon_flag yet.  I will look
into making it do so (and the other drivers in question too, possibly,
though I have no way of testing any but mesh.c and mac53c94.c).

> Data direction tables shouldn't be needed in lk 2.4 .

Great!

As for 2.2, would the equivalent of the patch that I sent be acceptable
for inclusion?  If that's too big a change, what about a patch that just
adds SEND_CUE_SHEET to include/scsi/scsi.h and each of the case
statements in the drivers in question?

-Daniel

-- 
Daniel E. Eisenbud
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]

[PATCH] consolidating data direction tables

[Daniel Eisenbud]

Eight different SCSI drivers have large switch statements to determine
the direction in which data will be transferred for a given SCSI
command.  I discovered this when trying to figure out why the MESH
(powermac SCSI) driver locked up when (and only when) trying to burn a
CD-R in disk-at-once mode.  It turns out that there's a command that's
not in any of these tables (cdrecord refers to it as send_cue_sheet) and
it's absence is enough to kill the MESH driver (and apparently also the
mac53c94 driver.)  The other drivers in question may or may not be able
to recover from the missing item in their tables, but the fact that
these tables are in the drivers at all implies that they work best if
the tables are correct.

The attached patch does two things: it creates a new header file called
scsi_dataout.h, which has a single copy of the switch statement (as a
static function -- is that all right?) and is included by the relevant
drivers.  I checked that all the drivers had exactly equivalent switch
statements.  The new switch statement is that switch statement with
SEND_CUE_SHEET (which I've added to include/scsi/scsi.h) added.  This
way if other commands need to be added to the switch statements in the
future, it will be easy to do it in one central place.

Clearly it is a problem that the mesh and mac53c94 drivers actually lock
up hard when they get a command that they haven't seen before.  When I
have some time, I will learn more about the SCSI layer and see if I can
fix this.  But in the meantime, this patch prevents the known case that
locks up, and is a cleanup to boot.  Please let me know if there's
anything I need to do differently to have a chance of this getting added
to the kernel: I'm rather new to kernel hacking altogether.

Thanks,
Daniel Eisenbud

-- 
Daniel E. Eisenbud
[EMAIL PROTECTED]


diff -durpN linuxppc_2_4/drivers/acorn/scsi/acornscsi.c 
linuxppc_2_4-scsifix/drivers/acorn/scsi/acornscsi.c
--- linuxppc_2_4/drivers/acorn/scsi/acornscsi.c Fri Jan 12 16:57:57 2001
+++ linuxppc_2_4-scsifix/drivers/acorn/scsi/acornscsi.c Sat Feb  3 12:02:04 2001
@@ -152,6 +152,7 @@
 #include 

 #include "../../scsi/scsi.h"
+#include "../../scsi/scsi_dataout.h"
 #include "../../scsi/hosts.h"
 #include "../../scsi/constants.h"
 #include "acornscsi.h"
@@ -600,34 +601,6 @@ cmdtype_t acornscsi_cmdtype(int command)
 }

 /*
- * Prototype: int acornscsi_datadirection(int command)
- * Purpose  : differentiate between commands that have a DATA IN phase
- *   and a DATA OUT phase
- * Params   : command - command to interpret
- * Returns  : DATADIR_OUT - data out phase expected
- *   DATADIR_IN  - data in phase expected
- */
-static
-datadir_t acornscsi_datadirection(int command)
-{
-switch (command) {
-case CHANGE_DEFINITION:case COMPARE:   case COPY:
-case COPY_VERIFY:  case LOG_SELECT:case MODE_SELECT:
-case MODE_SELECT_10:   case SEND_DIAGNOSTIC:   case WRITE_BUFFER:
-case FORMAT_UNIT:  case REASSIGN_BLOCKS:   case RESERVE:
-case SEARCH_EQUAL: case SEARCH_HIGH:   case SEARCH_LOW:
-case WRITE_6:  case WRITE_10:  case WRITE_VERIFY:
-case UPDATE_BLOCK: case WRITE_LONG:case WRITE_SAME:
-case SEARCH_HIGH_12:   case SEARCH_EQUAL_12:   case SEARCH_LOW_12:
-case WRITE_12: case WRITE_VERIFY_12:   case SET_WINDOW:
-case MEDIUM_SCAN:  case SEND_VOLUME_TAG:   case 0xea:
-   return DATADIR_OUT;
-default:
-   return DATADIR_IN;
-}
-}
-
-/*
  * Purpose  : provide values for synchronous transfers with 33C93.
  * Copyright: Copyright (c) 1996 John Shifflett, GeoLog Consulting
  * Modified by Russell King for 8MHz WD33C93A
@@ -2552,7 +2525,7 @@ int acornscsi_queuecmd(Scsi_Cmnd *SCpnt,
 SCpnt->host_scribble = NULL;
 SCpnt->result = 0;
 SCpnt->tag = 0;
-SCpnt->SCp.phase = (int)acornscsi_datadirection(SCpnt->cmnd[0]);
+SCpnt->SCp.phase = Scsi_data_goes_out(SCpnt);
 SCpnt->SCp.sent_command = 0;
 SCpnt->SCp.scsi_xferred = 0;
 SCpnt->SCp.Status = 0;
diff -durpN linuxppc_2_4/drivers/acorn/scsi/acornscsi.h 
linuxppc_2_4-scsifix/drivers/acorn/scsi/acornscsi.h
--- linuxppc_2_4/drivers/acorn/scsi/acornscsi.h Fri Jan 12 16:57:57 2001
+++ linuxppc_2_4-scsifix/drivers/acorn/scsi/acornscsi.h Fri Feb  2 18:36:48 2001
@@ -288,14 +288,6 @@ typedef enum { /* command 
type */
 CMD_MISC,  /* Others  
 */
 } cmdtype_t;

-/*
- * Data phase direction
- */
-typedef enum { /* Data direction  
 */
-DATADIR_IN,/* Data in phase expected  
 */
-DATADIR_OUT/*