Re: [PATCH 3/3] Storvsc: Select channel based on available percentage of ring buffer to write

2018-03-23 Thread kbuild test robot
Hi Long,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.16-rc6]
[cannot apply to linus/master net-next/master net/master next-20180323]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Long-Li/Vmbus-Add-function-to-report-available-ring-buffer-to-write-in-total-ring-size-percentage/20180324-124431
config: x86_64-rhel (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers//scsi/storvsc_drv.c: In function 'storvsc_do_io':
>> drivers//scsi/storvsc_drv.c:1402:1: warning: the frame size of 2064 bytes is 
>> larger than 2048 bytes [-Wframe-larger-than=]
}
^

vim +1402 drivers//scsi/storvsc_drv.c

d86adf48 drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2016-12-14  1287  
d86adf48 drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2016-12-14  1288  
c1b3d067 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1289  
static int storvsc_do_io(struct hv_device *device,
d86adf48 drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2016-12-14  1290 
 struct storvsc_cmd_request *request, u16 q_num)
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1291  {
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1292 
struct storvsc_device *stor_device;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1293 
struct vstor_packet *vstor_packet;
e75adf20 drivers/scsi/storvsc_drv.c   Long Li  2018-03-22  1294 
struct vmbus_channel *outgoing_channel, *channel;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1295 
int ret = 0;
e75adf20 drivers/scsi/storvsc_drv.c   Long Li  2018-03-22  1296 
struct cpumask alloced_mask, other_numa_mask;
d86adf48 drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2016-12-14  1297 
int tgt_cpu;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1298  
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1299 
vstor_packet = >vstor_packet;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1300 
stor_device = get_out_stor_device(device);
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1301  
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1302 
if (!stor_device)
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1303 
return -ENODEV;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1304  
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1305  
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27  1306 
request->device  = device;
6f94d5de drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2013-06-04  1307 
/*
6f94d5de drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2013-06-04  1308 
 * Select an an appropriate channel to send the request out.
6f94d5de drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2013-06-04  1309 
 */
d86adf48 drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2016-12-14  1310 
if (stor_device->stor_chns[q_num] != NULL) {
d86adf48 drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2016-12-14  1311 
outgoing_channel = stor_device->stor_chns[q_num];
e75adf20 drivers/scsi/storvsc_drv.c   Long Li  2018-03-22  1312 
if (outgoing_channel->target_cpu == q_num) {
d86adf48 drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2016-12-14  1313 
/*
d86adf48 drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2016-12-14  1314 
 * Ideally, we want to pick a different channel if
d86adf48 drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2016-12-14  1315 
 * available on the same NUMA node.
d86adf48 drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2016-12-14  1316 
 */
d86adf48 drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2016-12-14  1317 
cpumask_and(_mask, _device->alloced_cpus,
d86adf48 drivers/scsi/storvsc_drv.c   K. Y. Srinivasan 2016-12-14  1318 
cpumask_of_node(cpu_to_node(q_num)));
e75adf20 drivers/scsi/storvsc_drv.c   Long Li  2018-03-22  1319  
e75adf20 drivers/scsi/storvsc_drv.c   Long Li  2018-03-22  1320 
for_each_cpu_wrap(tgt_cpu, _mask, q_num + 1) {
e75adf20 drivers/scsi/storvsc_drv.c   Long Li  2018-03-22  1321 
if (tgt_cpu == q_num)
e75adf20 drivers/scsi/storvsc_drv.c   Long Li  2018-03-22  1322 
continue;
e75adf20 drivers/scsi/storvsc_drv.c

[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #22 from Bart Van Assche (bvanass...@acm.org) ---
This ticket has category IO/Storage; SCSI. That category does not cover
mounting filesystems. I'm fine with closing this ticket and creating a new
ticket if for the mount issue if necessary.

-- 
You are receiving this mail because:
You are the assignee for the bug.


[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #21 from Chuck Burt (chuck.burt+kernel@gmail.com) ---
Thank you again.  Should we close this issue as duplicate / resolves elsewhere?

-- 
You are receiving this mail because:
You are the assignee for the bug.


Re: [PATCH 1/6] esas2r: remove initialization / cleanup dead wood

2018-03-23 Thread Bradley Grove


Looks fine.

Acked-by: Bradley Grove 


On 03/19/2018 03:37 AM, Christoph Hellwig wrote:

esas2r has been converted to hotplug style initialization long ago, but
kept various remant of the old-style scsi_module.c initialization around.
Remove those.

Signed-off-by: Christoph Hellwig 
---
  drivers/scsi/esas2r/esas2r.h  |  2 --
  drivers/scsi/esas2r/esas2r_init.c | 21 
  drivers/scsi/esas2r/esas2r_main.c | 72 +++
  3 files changed, 4 insertions(+), 91 deletions(-)

diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
index 1da6407ee142..858c3b33db78 100644
--- a/drivers/scsi/esas2r/esas2r.h
+++ b/drivers/scsi/esas2r/esas2r.h
@@ -962,7 +962,6 @@ struct esas2r_adapter {
   * Function Declarations
   * SCSI functions
   */
-int esas2r_release(struct Scsi_Host *);
  const char *esas2r_info(struct Scsi_Host *);
  int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
struct esas2r_sas_nvram *data);
@@ -984,7 +983,6 @@ int esas2r_target_reset(struct scsi_cmnd *cmd);
  /* Internal functions */
  int esas2r_init_adapter(struct Scsi_Host *host, struct pci_dev *pcid,
int index);
-int esas2r_cleanup(struct Scsi_Host *host);
  int esas2r_read_fw(struct esas2r_adapter *a, char *buf, long off, int count);
  int esas2r_write_fw(struct esas2r_adapter *a, const char *buf, long off,
int count);
diff --git a/drivers/scsi/esas2r/esas2r_init.c 
b/drivers/scsi/esas2r/esas2r_init.c
index 5b14dd29b764..9dffcb28c9b7 100644
--- a/drivers/scsi/esas2r/esas2r_init.c
+++ b/drivers/scsi/esas2r/esas2r_init.c
@@ -661,27 +661,6 @@ void esas2r_kill_adapter(int i)
}
  }
  
-int esas2r_cleanup(struct Scsi_Host *host)

-{
-   struct esas2r_adapter *a;
-   int index;
-
-   if (host == NULL) {
-   int i;
-
-   esas2r_debug("esas2r_cleanup everything");
-   for (i = 0; i < MAX_ADAPTERS; i++)
-   esas2r_kill_adapter(i);
-   return -1;
-   }
-
-   esas2r_debug("esas2r_cleanup called for host %p", host);
-   a = (struct esas2r_adapter *)host->hostdata;
-   index = a->index;
-   esas2r_kill_adapter(index);
-   return index;
-}
-
  int esas2r_suspend(struct pci_dev *pdev, pm_message_t state)
  {
struct Scsi_Host *host = pci_get_drvdata(pdev);
diff --git a/drivers/scsi/esas2r/esas2r_main.c 
b/drivers/scsi/esas2r/esas2r_main.c
index 4eb14301a497..e07eac5be087 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -235,7 +235,6 @@ static struct scsi_host_template driver_template = {
.module = THIS_MODULE,
.show_info  = esas2r_show_info,
.name   = ESAS2R_LONGNAME,
-   .release= esas2r_release,
.info   = esas2r_info,
.ioctl  = esas2r_ioctl,
.queuecommand   = esas2r_queuecommand,
@@ -520,44 +519,16 @@ static int esas2r_probe(struct pci_dev *pcid,
  
  static void esas2r_remove(struct pci_dev *pdev)

  {
-   struct Scsi_Host *host;
-   int index;
-
-   if (pdev == NULL) {
-   esas2r_log(ESAS2R_LOG_WARN, "esas2r_remove pdev==NULL");
-   return;
-   }
-
-   host = pci_get_drvdata(pdev);
-
-   if (host == NULL) {
-   /*
-* this can happen if pci_set_drvdata was already called
-* to clear the host pointer.  if this is the case, we
-* are okay; this channel has already been cleaned up.
-*/
-
-   return;
-   }
+   struct Scsi_Host *host = pci_get_drvdata(pdev);
+   struct esas2r_adapter *a = (struct esas2r_adapter *)host->hostdata;
  
  	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),

   "esas2r_remove(%p) called; "
   "host:%p", pdev,
   host);
  
-	index = esas2r_cleanup(host);

-
-   if (index < 0)
-   esas2r_log_dev(ESAS2R_LOG_WARN, &(pdev->dev),
-  "unknown host in %s",
-  __func__);
-
+   esas2r_kill_adapter(a->index);
found_adapters--;
-
-   /* if this was the last adapter, clean up the rest of the driver */
-
-   if (found_adapters == 0)
-   esas2r_cleanup(NULL);
  }
  
  static int __init esas2r_init(void)

@@ -638,30 +609,7 @@ static int __init esas2r_init(void)
for (i = 0; i < MAX_ADAPTERS; i++)
esas2r_adapters[i] = NULL;
  
-	/* initialize */

-
-   driver_template.module = THIS_MODULE;
-
-   if (pci_register_driver(_pci_driver) != 0)
-   esas2r_log(ESAS2R_LOG_CRIT, "pci_register_driver FAILED");
-   else
-   esas2r_log(ESAS2R_LOG_INFO, 

[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #20 from Bart Van Assche (bvanass...@acm.org) ---
As far as I can see merging Dave's tree pulled in only the following three SCSI
changes:
* qed*: Utilize Firmware 8.15.3.0
* qedf: fix wrong le16 conversion
* netlink: extended ACK reporting

Unless you are using the qedi or qedf driver I think that's it's unlikely that
these changes are related to the issue you reported.

-- 
You are receiving this mail because:
You are the assignee for the bug.


[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #19 from Chuck Burt (chuck.burt+kernel@gmail.com) ---
(Also, to clarify, the mounting does not actually fail... it produces that
error as a dialog in the GUI, but mounting does actually succeed.)

-- 
You are receiving this mail because:
You are the assignee for the bug.


[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #18 from Chuck Burt (chuck.burt+kernel@gmail.com) ---
Yet it appears there were numerous revisions in the `drivers/scsi` area...? 

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=8d65b08debc7e62b2c6032d7fe7389d895b92cbc

I'm a newbie, so... I could obviously be reading this completely wrong...

-- 
You are receiving this mail because:
You are the assignee for the bug.


[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #17 from Bart Van Assche (bvanass...@acm.org) ---
At the end of bisect log 2 I found the following:

first bad commit: [8d65b08debc7e62b2c6032d7fe7389d895b92cbc] Merge
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next

It seems unlikely to me that any of the commits in the networking tree would
cause mounting of a local filesystem to fail.

-- 
You are receiving this mail because:
You are the assignee for the bug.


[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #16 from Chuck Burt (chuck.burt+kernel@gmail.com) ---
Git Bisect Log 2 - https://bugzilla.kernel.org/attachment.cgi?id=274897
This identifies a separate issue (should I file a separate bug for this?) where
mounting/unmounting caused error:

> Device /dev/sdb3 is already mounted at `/media/temp/[identifier]`. 
> (udisks-error-quark, 6)

Unmounting gives a similar error about being unable to unmount (I can provide
the exact error in a bit if you need it).


This mounting/unmounting error still exists in the v4.15 kernel and was
introduced in the commit isolated in the above bisect (Git Bisect Log 2).

-- 
You are receiving this mail because:
You are the assignee for the bug.


[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #15 from Bart Van Assche (bvanass...@acm.org) ---
Sorry but I lost track. What was the second issue?

-- 
You are receiving this mail because:
You are the assignee for the bug.


[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #14 from Chuck Burt (chuck.burt+kernel@gmail.com) ---
I tested with v4.15.0-041500.  Great news, the hang is resolved!

The second issue I found still exists, but that is not nearly as severe (it
doesn't block my usage).  It also occurs on more drives.  Should I break that
into a separate issue?

Thank you very very much for your help, Bart.

-- 
You are receiving this mail because:
You are the assignee for the bug.


Re: [PATCH 2/2] target: remove spin_lock_assert() in __target_(attach|detach)_tg_pt_gp()

2018-03-23 Thread Bart Van Assche
On Fri, 2018-03-23 at 18:50 +0100, bige...@linutronix.de wrote:
> On 2018-03-23 17:44:46 [+], Bart Van Assche wrote:
> > In other words, do we really need to remove these checks? I think that these
> > checks are useful as documentation to people who read the SCSI target code.
> > The target code is already hard to follow so I think any documentation,
> > especially documentation in the form of code that is checked at runtime, is
> > welcome.
> 
> so if I remove those two and add a kernel doc comment instead, saying
> that the caller needs to ensure that "lun->lun_tg_pt_gp_lock" is held
> then we would remove the obvious runtime check and add a piece of
> documentation. Would that work?

Comments are not verified at runtime and hence can become outdated if the code
is modified. assert_spin_locked() and lockdep_assert_held() assertions however
are verified at runtime with the proper kernel configuration options enabled.
Hence my preference for assert_spin_locked()/lockdep_assert_held() over source
code comments.

Thanks,

Bart.




Re: [PATCH 2/2] target: remove spin_lock_assert() in __target_(attach|detach)_tg_pt_gp()

2018-03-23 Thread bige...@linutronix.de
On 2018-03-23 17:44:46 [+], Bart Van Assche wrote:
> On Fri, 2018-03-23 at 18:19 +0100, bige...@linutronix.de wrote:
> > __target_attach_tg_pt_gp() and __target_detach_tg_pt_gp() check if the 
> > caller
> > holds lun_tg_pt_gp_lock(). Both functions are static, the callers are
> > acquiring the lock before the invocation of the function so the check
> > looks superfluous.
> > Remove it.
> 
> Does this check cause trouble to anyone or to a specific kernel configuration?
Those two do not.

> In other words, do we really need to remove these checks? I think that these
> checks are useful as documentation to people who read the SCSI target code.
> The target code is already hard to follow so I think any documentation,
> especially documentation in the form of code that is checked at runtime, is
> welcome.

so if I remove those two and add a kernel doc comment instead, saying
that the caller needs to ensure that "lun->lun_tg_pt_gp_lock" is held
then we would remove the obvious runtime check and add a piece of
documentation. Would that work?

> Thanks,
> 
> Bart.

Sebastian


Re: [PATCH 1/2 v2] target: drop spin_lock_assert() + irqs_disabled() combo checks

2018-03-23 Thread Bart Van Assche
On Fri, 2018-03-23 at 18:36 +0100, Sebastian Andrzej Siewior wrote:
> There are a few functions which check for if the lock is held
> (spin_lock_assert()) and the interrupts are disabled (irqs_disabled()).
> > From looking at the code, each function is static, the caller is near by
> 
> and does spin_lock_irq|safe(). As Linus puts it:
> 
> > It's not like this is some function that is exported to random users,
> > and we should check that the calling convention is right.
> > 
> > This looks like "it may have been useful during coding to document
> > things, but it's not useful long-term".
> 
> Remove those checks.

Reviewed-by: Bart Van Assche 





Re: [PATCH v2 13/38] cxlflash: Support adapter file descriptors for OCXL

2018-03-23 Thread Uma Krishnan


> On Mar 22, 2018, at 12:12 PM, Frederic Barrat  
> wrote:
> 
> 
> 
> Le 26/02/2018 à 23:21, Uma Krishnan a écrit :
>> Allocate a file descriptor for an adapter context when requested. In order
>> to allocate inodes for the file descriptors, a pseudo filesystem is created
>> and used.
>> Signed-off-by: Uma Krishnan 
>> Acked-by: Matthew R. Ochs 
>> ---
> 
> 
> We've touched the subject before, and I don't have a magic solution, but it 
> feels like something could be shared here with cxl, or maybe even other 
> drivers?
> 
Yes, perhaps we could look at refactoring in a future series.

> I only took a quick read of the inode allocator.
> 
>  Fred
> 
> 
> 
> 
>>  drivers/scsi/cxlflash/ocxl_hw.c | 200 
>> 
>>  drivers/scsi/cxlflash/ocxl_hw.h |   1 +
>>  2 files changed, 201 insertions(+)
>> diff --git a/drivers/scsi/cxlflash/ocxl_hw.c 
>> b/drivers/scsi/cxlflash/ocxl_hw.c
>> index 6472210..59e9003 100644
>> --- a/drivers/scsi/cxlflash/ocxl_hw.c
>> +++ b/drivers/scsi/cxlflash/ocxl_hw.c
>> @@ -12,13 +12,144 @@
>>   * 2 of the License, or (at your option) any later version.
>>   */
>> +#include 
>>  #include 
>> +#include 
>> +#include 
>>  #include 
>>  #include "backend.h"
>>  #include "ocxl_hw.h"
>> +/*
>> + * Pseudo-filesystem to allocate inodes.
>> + */
>> +
>> +#define OCXLFLASH_FS_MAGIC  0x1697698f
>> +
>> +static int ocxlflash_fs_cnt;
>> +static struct vfsmount *ocxlflash_vfs_mount;
>> +
>> +static const struct dentry_operations ocxlflash_fs_dops = {
>> +.d_dname= simple_dname,
>> +};
>> +
>> +/*
>> + * ocxlflash_fs_mount() - mount the pseudo-filesystem
>> + * @fs_type:File system type.
>> + * @flags:  Flags for the filesystem.
>> + * @dev_name:   Device name associated with the filesystem.
>> + * @data:   Data pointer.
>> + *
>> + * Return: pointer to the directory entry structure
>> + */
>> +static struct dentry *ocxlflash_fs_mount(struct file_system_type *fs_type,
>> + int flags, const char *dev_name,
>> + void *data)
>> +{
>> +return mount_pseudo(fs_type, "ocxlflash:", NULL, _fs_dops,
>> +OCXLFLASH_FS_MAGIC);
>> +}
>> +
>> +static struct file_system_type ocxlflash_fs_type = {
>> +.name   = "ocxlflash",
>> +.owner  = THIS_MODULE,
>> +.mount  = ocxlflash_fs_mount,
>> +.kill_sb= kill_anon_super,
>> +};
>> +
>> +/*
>> + * ocxlflash_release_mapping() - release the memory mapping
>> + * @ctx:Context whose mapping is to be released.
>> + */
>> +static void ocxlflash_release_mapping(struct ocxlflash_context *ctx)
>> +{
>> +if (ctx->mapping)
>> +simple_release_fs(_vfs_mount, _fs_cnt);
>> +ctx->mapping = NULL;
>> +}
>> +
>> +/*
>> + * ocxlflash_getfile() - allocate pseudo filesystem, inode, and the file
>> + * @dev:Generic device of the host.
>> + * @name:   Name of the pseudo filesystem.
>> + * @fops:   File operations.
>> + * @priv:   Private data.
>> + * @flags:  Flags for the file.
>> + *
>> + * Return: pointer to the file on success, ERR_PTR on failure
>> + */
>> +static struct file *ocxlflash_getfile(struct device *dev, const char *name,
>> +  const struct file_operations *fops,
>> +  void *priv, int flags)
>> +{
>> +struct qstr this;
>> +struct path path;
>> +struct file *file;
>> +struct inode *inode = NULL;
>> +int rc;
>> +
>> +if (fops->owner && !try_module_get(fops->owner)) {
>> +dev_err(dev, "%s: Owner does not exist\n", __func__);
>> +rc = -ENOENT;
>> +goto err1;
>> +}
>> +
>> +rc = simple_pin_fs(_fs_type, _vfs_mount,
>> +   _fs_cnt);
>> +if (unlikely(rc < 0)) {
>> +dev_err(dev, "%s: Cannot mount ocxlflash pseudofs rc=%d\n",
>> +__func__, rc);
>> +goto err2;
>> +}
>> +
>> +inode = alloc_anon_inode(ocxlflash_vfs_mount->mnt_sb);
>> +if (IS_ERR(inode)) {
>> +rc = PTR_ERR(inode);
>> +dev_err(dev, "%s: alloc_anon_inode failed rc=%d\n",
>> +__func__, rc);
>> +goto err3;
>> +}
>> +
>> +this.name = name;
>> +this.len = strlen(name);
>> +this.hash = 0;
>> +path.dentry = d_alloc_pseudo(ocxlflash_vfs_mount->mnt_sb, );
>> +if (!path.dentry) {
>> +dev_err(dev, "%s: d_alloc_pseudo failed\n", __func__);
>> +rc = -ENOMEM;
>> +goto err4;
>> +}
>> +
>> +path.mnt = mntget(ocxlflash_vfs_mount);
>> +d_instantiate(path.dentry, inode);
>> +
>> +file = alloc_file(, OPEN_FMODE(flags), fops);
>> +if (IS_ERR(file)) {
>> +rc = PTR_ERR(file);
>> +dev_err(dev, "%s: alloc_file failed rc=%d\n",
>> +__func__, rc);
>> +

Re: [PATCH 2/2] target: remove spin_lock_assert() in __target_(attach|detach)_tg_pt_gp()

2018-03-23 Thread Bart Van Assche
On Fri, 2018-03-23 at 18:19 +0100, bige...@linutronix.de wrote:
> __target_attach_tg_pt_gp() and __target_detach_tg_pt_gp() check if the caller
> holds lun_tg_pt_gp_lock(). Both functions are static, the callers are
> acquiring the lock before the invocation of the function so the check
> looks superfluous.
> Remove it.

Does this check cause trouble to anyone or to a specific kernel configuration?
In other words, do we really need to remove these checks? I think that these
checks are useful as documentation to people who read the SCSI target code.
The target code is already hard to follow so I think any documentation,
especially documentation in the form of code that is checked at runtime, is
welcome.

Thanks,

Bart.





RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring buffer percentage

2018-03-23 Thread Long Li
> Subject: RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring
> buffer percentage
> 
> 
> 
> > -Original Message-
> > From: Haiyang Zhang
> > Sent: Friday, March 23, 2018 8:01 AM
> > To: Long Li ; KY Srinivasan
> > ; Stephen Hemminger ;
> James
> > E . J . Bottomley ; Martin K . Petersen
> > ; de...@linuxdriverproject.org; linux-
> > s...@vger.kernel.org; linux-ker...@vger.kernel.org
> > Cc: Long Li 
> > Subject: RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate
> > ring buffer percentage
> >
> >
> >
> > > -Original Message-
> > > From: Long Li 
> > > Sent: Thursday, March 22, 2018 8:16 PM
> > > To: KY Srinivasan ; Haiyang Zhang
> > > ; Stephen Hemminger
> > ;
> > > James E . J . Bottomley ; Martin K . Petersen
> > > ; de...@linuxdriverproject.org; linux-
> > > s...@vger.kernel.org; linux-ker...@vger.kernel.org
> > > Cc: Long Li 
> > > Subject: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate
> > > ring
> > buffer
> > > percentage
> > >
> > > From: Long Li 
> > >
> > > In Vmbus, we have defined a function to calculate available ring
> > > buffer percentage to write.
> > >
> > > Use that function and remove duplicate netvsc code.
> > >
> > > Signed-off-by: Long Li 
> > > ---
> > >  drivers/net/hyperv/netvsc.c | 17 +++--
> > >  drivers/net/hyperv/netvsc_drv.c |  3 ---
> > >  2 files changed, 3 insertions(+), 17 deletions(-)
> 
> Why is the patch being sent to the scsi list and not to the network mailing 
> list
> and Dave Miller.

I will re-send the patch.

> 
> K. Y
> > >
> > > diff --git a/drivers/net/hyperv/netvsc.c
> > > b/drivers/net/hyperv/netvsc.c
> > index
> > > 0265d703eb03..8af0069e4d8c 100644
> > > --- a/drivers/net/hyperv/netvsc.c
> > > +++ b/drivers/net/hyperv/netvsc.c
> > > @@ -31,7 +31,6 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > -#include 
> > >
> > >  #include 
> > >
> > > @@ -590,17 +589,6 @@ void netvsc_device_remove(struct hv_device
> > *device)
> > > #define RING_AVAIL_PERCENT_HIWATER 20  #define
> > > RING_AVAIL_PERCENT_LOWATER 10
> > >
> > > -/*
> > > - * Get the percentage of available bytes to write in the ring.
> > > - * The return value is in range from 0 to 100.
> > > - */
> > > -static u32 hv_ringbuf_avail_percent(const struct
> > > hv_ring_buffer_info
> > > *ring_info) -{
> > > - u32 avail_write = hv_get_bytes_to_write(ring_info);
> > > -
> > > - return reciprocal_divide(avail_write  * 100, netvsc_ring_reciprocal);
> > > -}
> > > -
> > >  static inline void netvsc_free_send_slot(struct netvsc_device
> > *net_device,
> > >u32 index)
> > >  {
> > > @@ -649,7 +637,8 @@ static void netvsc_send_tx_complete(struct
> > > netvsc_device *net_device,
> > >   wake_up(_device->wait_drain);
> > >
> > >   if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx))
> > &&
> > > - (hv_ringbuf_avail_percent(>outbound) >
> > > RING_AVAIL_PERCENT_HIWATER ||
> > > + (hv_get_avail_to_write_percent(>outbound) >
> > > +  RING_AVAIL_PERCENT_HIWATER ||
> > >queue_sends < 1)) {
> > >   netif_tx_wake_queue(netdev_get_tx_queue(ndev, q_idx));
> > >   ndev_ctx->eth_stats.wake_queue++;  @@ -757,7 +746,7 @@
> static
> > >inline int netvsc_send_pkt(
> > >   struct netdev_queue *txq = netdev_get_tx_queue(ndev, packet-
> > >q_idx);
> > >   u64 req_id;
> > >   int ret;
> > > - u32 ring_avail = hv_ringbuf_avail_percent(_channel-
> > >outbound);
> > > + u32 ring_avail =
> > > +hv_get_avail_to_write_percent(_channel->outbound);
> > >
> > >   nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
> > >   if (skb)
> > > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > b/drivers/net/hyperv/netvsc_drv.c
> > > index faea0be18924..b0b1c2fd2b7b 100644
> > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > @@ -35,7 +35,6 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > -#include 
> > >
> > >  #include 
> > >  #include 
> > > @@ -55,7 +54,6 @@ static unsigned int ring_size __ro_after_init =
> > > 128; module_param(ring_size, uint, S_IRUGO);
> > MODULE_PARM_DESC(ring_size,
> > > "Ring buffer size (# of pages)");  unsigned int netvsc_ring_bytes
> > __ro_after_init;
> > > -struct reciprocal_value netvsc_ring_reciprocal __ro_after_init;
> > >
> > >  static const u32 default_msg = NETIF_MSG_DRV | NETIF_MSG_PROBE |
> > >   NETIF_MSG_LINK | NETIF_MSG_IFUP | @@ -
> 2186,7 +2184,6 @@ static
> > > int __init netvsc_drv_init(void)
> > >   ring_size);
> > >   }
> > >   netvsc_ring_bytes = ring_size * PAGE_SIZE;
> > > - netvsc_ring_reciprocal = 

[PATCH 1/2 v2] target: drop spin_lock_assert() + irqs_disabled() combo checks

2018-03-23 Thread Sebastian Andrzej Siewior
There are a few functions which check for if the lock is held
(spin_lock_assert()) and the interrupts are disabled (irqs_disabled()).
>From looking at the code, each function is static, the caller is near by
and does spin_lock_irq|safe(). As Linus puts it:

|It's not like this is some function that is exported to random users,
|and we should check that the calling convention is right.
|
|This looks like "it may have been useful during coding to document
|things, but it's not useful long-term".

Remove those checks.

Reported-by: Arnaldo Carvalho de Melo 
Suggested-by: Linus Torvalds 
Signed-off-by: Sebastian Andrzej Siewior 
---
v1…2: Add credits.

 drivers/target/target_core_tmr.c   | 2 --
 drivers/target/target_core_transport.c | 6 --
 2 files changed, 8 deletions(-)

diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index 9c7bc1ca341a..3d35dad1de2c 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -114,8 +114,6 @@ static bool __target_check_io_state(struct se_cmd *se_cmd,
 {
struct se_session *sess = se_cmd->se_sess;
 
-   assert_spin_locked(>sess_cmd_lock);
-   WARN_ON_ONCE(!irqs_disabled());
/*
 * If command already reached CMD_T_COMPLETE state within
 * target_complete_cmd() or CMD_T_FABRIC_STOP due to shutdown,
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 74b646f165d4..2c3ddb3a4124 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2954,9 +2954,6 @@ __transport_wait_for_tasks(struct se_cmd *cmd, bool 
fabric_stop,
__acquires(>t_state_lock)
 {
 
-   assert_spin_locked(>t_state_lock);
-   WARN_ON_ONCE(!irqs_disabled());
-
if (fabric_stop)
cmd->transport_state |= CMD_T_FABRIC_STOP;
 
@@ -3241,9 +3238,6 @@ static int __transport_check_aborted_status(struct se_cmd 
*cmd, int send_status)
 {
int ret;
 
-   assert_spin_locked(>t_state_lock);
-   WARN_ON_ONCE(!irqs_disabled());
-
if (!(cmd->transport_state & CMD_T_ABORTED))
return 0;
/*
-- 
2.16.2


[PATCH 1/2] target: drop spin_lock_assert() + irqs_disabled() combo checks

2018-03-23 Thread bige...@linutronix.de
There are a few functions which check for if the lock is held
(spin_lock_assert()) and the interrupts are disabled (irqs_disabled()).
>From looking at the code, each function is static, the caller is near by
and does spin_lock_irq|safe(). As Linus puts it:

|It's not like this is some function that is exported to random users,
|and we should check that the calling convention is right.
|
|This looks like "it may have been useful during coding to document
|things, but it's not useful long-term".

Remove those checks.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/target/target_core_tmr.c   | 2 --
 drivers/target/target_core_transport.c | 6 --
 2 files changed, 8 deletions(-)

diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index 9c7bc1ca341a..3d35dad1de2c 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -114,8 +114,6 @@ static bool __target_check_io_state(struct se_cmd *se_cmd,
 {
struct se_session *sess = se_cmd->se_sess;
 
-   assert_spin_locked(>sess_cmd_lock);
-   WARN_ON_ONCE(!irqs_disabled());
/*
 * If command already reached CMD_T_COMPLETE state within
 * target_complete_cmd() or CMD_T_FABRIC_STOP due to shutdown,
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 74b646f165d4..2c3ddb3a4124 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2954,9 +2954,6 @@ __transport_wait_for_tasks(struct se_cmd *cmd, bool 
fabric_stop,
__acquires(>t_state_lock)
 {
 
-   assert_spin_locked(>t_state_lock);
-   WARN_ON_ONCE(!irqs_disabled());
-
if (fabric_stop)
cmd->transport_state |= CMD_T_FABRIC_STOP;
 
@@ -3241,9 +3238,6 @@ static int __transport_check_aborted_status(struct se_cmd 
*cmd, int send_status)
 {
int ret;
 
-   assert_spin_locked(>t_state_lock);
-   WARN_ON_ONCE(!irqs_disabled());
-
if (!(cmd->transport_state & CMD_T_ABORTED))
return 0;
/*
-- 
2.16.2



[PATCH 2/2] target: remove spin_lock_assert() in __target_(attach|detach)_tg_pt_gp()

2018-03-23 Thread bige...@linutronix.de
__target_attach_tg_pt_gp() and __target_detach_tg_pt_gp() check if the caller
holds lun_tg_pt_gp_lock(). Both functions are static, the callers are
acquiring the lock before the invocation of the function so the check
looks superfluous.
Remove it.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/target/target_core_alua.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/target/target_core_alua.c 
b/drivers/target/target_core_alua.c
index e46ca968009c..e5bda674bdbd 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -1843,8 +1843,6 @@ static void __target_attach_tg_pt_gp(struct se_lun *lun,
 {
struct se_dev_entry *se_deve;
 
-   assert_spin_locked(>lun_tg_pt_gp_lock);
-
spin_lock(_pt_gp->tg_pt_gp_lock);
lun->lun_tg_pt_gp = tg_pt_gp;
list_add_tail(>lun_tg_pt_gp_link, _pt_gp->tg_pt_gp_lun_list);
@@ -1868,8 +1866,6 @@ void target_attach_tg_pt_gp(struct se_lun *lun,
 static void __target_detach_tg_pt_gp(struct se_lun *lun,
struct t10_alua_tg_pt_gp *tg_pt_gp)
 {
-   assert_spin_locked(>lun_tg_pt_gp_lock);
-
spin_lock(_pt_gp->tg_pt_gp_lock);
list_del_init(>lun_tg_pt_gp_link);
tg_pt_gp->tg_pt_gp_members--;
-- 
2.16.2



Re: [PATCH] target: Use WARNON_NON_RT(!irqs_disabled())

2018-03-23 Thread Linus Torvalds
On Fri, Mar 23, 2018 at 9:25 AM, Bart Van Assche  wrote:
>
> Have you considered to delete the WARN_ON_ONCE(!irqs_disabled()) statement?
> I think that check duplicates functionality that already exists in lockdep
> since lockdep is already able to detect spinlock use inconsistencies.

Please just delete both lines.

There is exactly two callers of that static function, and both of them do

spin_lock_irq(>t_state_lock);

right above the call.

It's not like this is some function that is exported to random users,
and we should check that the calling convention is right.

So honestly, even lockdep annotations look like you don't need them.

This looks like "it may have been useful during coding to document
things, but it's not useful long-term".

Sure, the annotation is not wrong, but even if you go "verification is
good", you should ask yourself whether there are maybe better places
that would catch more relevant problems, than putting verification
into some static function with two trivially correct callers wrt this
verification?

  Linus


Re: [PATCH] target: Use WARNON_NON_RT(!irqs_disabled())

2018-03-23 Thread bige...@linutronix.de
On 2018-03-23 16:25:25 [+], Bart Van Assche wrote:
> On Fri, 2018-03-23 at 16:55 +0100, Sebastian Andrzej Siewior wrote:
> > I am going take this into -RT tree for now until we have different
> > solution.
> 
> Have you considered to delete the WARN_ON_ONCE(!irqs_disabled()) statement?
> I think that check duplicates functionality that already exists in lockdep
> since lockdep is already able to detect spinlock use inconsistencies.

correct. That is why I suggested to use lockdep_assert_held() instead of
this IRQ-check + the spin_lock_assert().
The only downside is that this code (as of now) works with lockdep
disabled. On the other hand, lockdep_assert_held() gives you a splat
instead of a BUG() statement like spin_lock_assert() does so I clearly
promote lockdep here :)

> Bart.

Sebastian


Re: [PATCH] target: Use WARNON_NON_RT(!irqs_disabled())

2018-03-23 Thread Bart Van Assche
On Fri, 2018-03-23 at 16:55 +0100, Sebastian Andrzej Siewior wrote:
> I am going take this into -RT tree for now until we have different
> solution.

Have you considered to delete the WARN_ON_ONCE(!irqs_disabled()) statement?
I think that check duplicates functionality that already exists in lockdep
since lockdep is already able to detect spinlock use inconsistencies.

Bart.




[PATCH 5/8] scsi: hisi_sas: consolidate command check in hisi_sas_get_ata_protocol()

2018-03-23 Thread John Garry
From: Xiaofei Tan 

Currently we check the fis->command value in 2 locations in
hisi_sas_get_ata_protocol() switch statement. Fix this by
consolidating the check for fis->command value to 1 location
only.

Signed-off-by: Xiaofei Tan 
Signed-off-by: John Garry 
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 9563dfa..8557fd0 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -78,22 +78,23 @@ u8 hisi_sas_get_ata_protocol(struct host_to_dev_fis *fis, 
int direction)
case ATA_CMD_STANDBYNOW1:
case ATA_CMD_ZAC_MGMT_OUT:
return HISI_SAS_SATA_PROTOCOL_NONDATA;
+
+   case ATA_CMD_SET_MAX:
+   switch (fis->features) {
+   case ATA_SET_MAX_PASSWD:
+   case ATA_SET_MAX_LOCK:
+   return HISI_SAS_SATA_PROTOCOL_PIO;
+
+   case ATA_SET_MAX_PASSWD_DMA:
+   case ATA_SET_MAX_UNLOCK_DMA:
+   return HISI_SAS_SATA_PROTOCOL_DMA;
+
+   default:
+   return HISI_SAS_SATA_PROTOCOL_NONDATA;
+   }
+
default:
{
-   if (fis->command == ATA_CMD_SET_MAX) {
-   switch (fis->features) {
-   case ATA_SET_MAX_PASSWD:
-   case ATA_SET_MAX_LOCK:
-   return HISI_SAS_SATA_PROTOCOL_PIO;
-
-   case ATA_SET_MAX_PASSWD_DMA:
-   case ATA_SET_MAX_UNLOCK_DMA:
-   return HISI_SAS_SATA_PROTOCOL_DMA;
-
-   default:
-   return HISI_SAS_SATA_PROTOCOL_NONDATA;
-   }
-   }
if (direction == DMA_NONE)
return HISI_SAS_SATA_PROTOCOL_NONDATA;
return HISI_SAS_SATA_PROTOCOL_PIO;
-- 
1.9.1



[PATCH 4/8] scsi: hisi_sas: use dma_zalloc_coherent()

2018-03-23 Thread John Garry
From: Xiang Chen 

This is a warning coming from Coccinelle, and need to use
new interface dma_zalloc_coherent() instead of
dma_alloc_coherent()/memset().

Signed-off-by: Xiang Chen 
Signed-off-by: John Garry 
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index a216795..9563dfa 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -1822,13 +1822,11 @@ int hisi_sas_alloc(struct hisi_hba *hisi_hba, struct 
Scsi_Host *shost)
goto err_out;
 
s = HISI_SAS_MAX_ITCT_ENTRIES * sizeof(struct hisi_sas_itct);
-   hisi_hba->itct = dma_alloc_coherent(dev, s, _hba->itct_dma,
+   hisi_hba->itct = dma_zalloc_coherent(dev, s, _hba->itct_dma,
GFP_KERNEL);
if (!hisi_hba->itct)
goto err_out;
 
-   memset(hisi_hba->itct, 0, s);
-
hisi_hba->slot_info = devm_kcalloc(dev, max_command_entries,
   sizeof(struct hisi_sas_slot),
   GFP_KERNEL);
-- 
1.9.1



[PATCH 1/8] scsi: hisi_sas: make SAS address of SATA disks unique

2018-03-23 Thread John Garry
From: Xiang Chen 

When directly connected with SATA disks in different SAS cores,
fill SAS address with scsi_host's id to make it's fake SAS address
unique.

Signed-off-by: Xiang Chen 
Signed-off-by: John Garry 
---
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index f89fb9a..89b9505 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3295,6 +3295,7 @@ static irqreturn_t sata_int_v2_hw(int irq_no, void *p)
sas_phy->oob_mode = SATA_OOB_MODE;
/* Make up some unique SAS address */
attached_sas_addr[0] = 0x50;
+   attached_sas_addr[6] = hisi_hba->shost->host_no;
attached_sas_addr[7] = phy_no;
memcpy(sas_phy->attached_sas_addr, attached_sas_addr, SAS_ADDR_SIZE);
memcpy(sas_phy->frame_rcvd, fis, sizeof(struct dev_to_host_fis));
-- 
1.9.1



[PATCH 3/8] scsi: hisi_sas: delete timer when removing hisi_sas driver

2018-03-23 Thread John Garry
From: Xiang Chen 

Delete timer for v1 and v3 hw when removing hisi_sas driver.

Signed-off-by: Xiang chen 
Signed-off-by: John Garry 
---
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 3 +++
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 3 ---
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +++
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 49c1fa6..a216795 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -2177,6 +2177,9 @@ int hisi_sas_remove(struct platform_device *pdev)
struct hisi_hba *hisi_hba = sha->lldd_ha;
struct Scsi_Host *shost = sha->core.shost;
 
+   if (timer_pending(_hba->timer))
+   del_timer(_hba->timer);
+
sas_unregister_ha(sha);
sas_remove_host(sha->core.shost);
 
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 89b9505..bed6afb 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3599,9 +3599,6 @@ static int hisi_sas_v2_remove(struct platform_device 
*pdev)
struct sas_ha_struct *sha = platform_get_drvdata(pdev);
struct hisi_hba *hisi_hba = sha->lldd_ha;
 
-   if (timer_pending(_hba->timer))
-   del_timer(_hba->timer);
-
hisi_sas_kill_tasklets(hisi_hba);
 
return hisi_sas_remove(pdev);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index df5414a..efe64bc 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2183,6 +2183,9 @@ static void hisi_sas_v3_remove(struct pci_dev *pdev)
struct hisi_hba *hisi_hba = sha->lldd_ha;
struct Scsi_Host *shost = sha->core.shost;
 
+   if (timer_pending(_hba->timer))
+   del_timer(_hba->timer);
+
sas_unregister_ha(sha);
sas_remove_host(sha->core.shost);
 
-- 
1.9.1



[PATCH 2/8] scsi: hisi_sas: update RAS feature for later revision of v3 HW

2018-03-23 Thread John Garry
From: Xiaofei Tan 

There is an modification for later revision of v3 hw. More HW errors
are reported through RAS interrupt. These errors were originally
reported only through MSI.

When report to RAS, some combinations are done to port AXI errors and
FIFO OMIT errors. For example, each port has 4 AXI errors, and they are
combined to one when report to RAS.

This patch does two things.
1. Enable RAS interrupt of these errors and handle them in PCI
error handlers.
2. Disable MSI interrupts of these errors for this later revision hw.

Signed-off-by: Xiaofei Tan 
Signed-off-by: John Garry 
---
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 60 --
 1 file changed, 58 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 6f3e5ba..df5414a 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -216,6 +216,9 @@
 #define SAS_RAS_INTR1  (RAS_BASE + 0x04)
 #define SAS_RAS_INTR0_MASK (RAS_BASE + 0x08)
 #define SAS_RAS_INTR1_MASK (RAS_BASE + 0x0c)
+#define CFG_SAS_RAS_INTR_MASK  (RAS_BASE + 0x1c)
+#define SAS_RAS_INTR2  (RAS_BASE + 0x20)
+#define SAS_RAS_INTR2_MASK (RAS_BASE + 0x24)
 
 /* HW dma structures */
 /* Delivery queue header */
@@ -392,6 +395,7 @@ static u32 hisi_sas_phy_read32(struct hisi_hba *hisi_hba,
 
 static void init_reg_v3_hw(struct hisi_hba *hisi_hba)
 {
+   struct pci_dev *pdev = hisi_hba->pci_dev;
int i;
 
/* Global registers init */
@@ -409,7 +413,10 @@ static void init_reg_v3_hw(struct hisi_hba *hisi_hba)
hisi_sas_write32(hisi_hba, ENT_INT_SRC3, 0x);
hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK1, 0xfefefefe);
hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK2, 0xfefefefe);
-   hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK3, 0xfffe20ff);
+   if (pdev->revision >= 0x21)
+   hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK3, 0x7fff);
+   else
+   hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK3, 0xfffe20ff);
hisi_sas_write32(hisi_hba, CHNL_PHYUPDOWN_INT_MSK, 0x0);
hisi_sas_write32(hisi_hba, CHNL_ENT_INT_MSK, 0x0);
hisi_sas_write32(hisi_hba, HGC_COM_INT_MSK, 0x0);
@@ -428,7 +435,12 @@ static void init_reg_v3_hw(struct hisi_hba *hisi_hba)
hisi_sas_phy_write32(hisi_hba, i, CHL_INT1, 0x);
hisi_sas_phy_write32(hisi_hba, i, CHL_INT2, 0x);
hisi_sas_phy_write32(hisi_hba, i, RXOP_CHECK_CFG_H, 0x1000);
-   hisi_sas_phy_write32(hisi_hba, i, CHL_INT1_MSK, 0xff87);
+   if (pdev->revision >= 0x21)
+   hisi_sas_phy_write32(hisi_hba, i, CHL_INT1_MSK,
+   0x);
+   else
+   hisi_sas_phy_write32(hisi_hba, i, CHL_INT1_MSK,
+   0xff87);
hisi_sas_phy_write32(hisi_hba, i, CHL_INT2_MSK, 0xbfe);
hisi_sas_phy_write32(hisi_hba, i, PHY_CTRL_RDY_MSK, 0x0);
hisi_sas_phy_write32(hisi_hba, i, PHYCTRL_NOT_RDY_MSK, 0x0);
@@ -503,6 +515,8 @@ static void init_reg_v3_hw(struct hisi_hba *hisi_hba)
/* RAS registers init */
hisi_sas_write32(hisi_hba, SAS_RAS_INTR0_MASK, 0x0);
hisi_sas_write32(hisi_hba, SAS_RAS_INTR1_MASK, 0x0);
+   hisi_sas_write32(hisi_hba, SAS_RAS_INTR2_MASK, 0x0);
+   hisi_sas_write32(hisi_hba, CFG_SAS_RAS_INTR_MASK, 0x0);
 }
 
 static void config_phy_opt_mode_v3_hw(struct hisi_hba *hisi_hba, int phy_no)
@@ -1319,6 +1333,13 @@ static irqreturn_t int_chnl_int_v3_hw(int irq_no, void 
*p)
 CHL_INT1);
u32 irq_value2 = hisi_sas_phy_read32(hisi_hba, phy_no,
 CHL_INT2);
+   u32 irq_msk1 = hisi_sas_phy_read32(hisi_hba, phy_no,
+   CHL_INT1_MSK);
+   u32 irq_msk2 = hisi_sas_phy_read32(hisi_hba, phy_no,
+   CHL_INT2_MSK);
+
+   irq_value1 &= ~irq_msk1;
+   irq_value2 &= ~irq_msk2;
 
if ((irq_msk & (4 << (phy_no * 4))) &&
irq_value1) {
@@ -1448,6 +1469,7 @@ static irqreturn_t fatal_axi_int_v3_hw(int irq_no, void 
*p)
hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK3, irq_msk | 0x1df00);
 
irq_value = hisi_sas_read32(hisi_hba, ENT_INT_SRC3);
+   irq_value &= ~irq_msk;
 
for (i = 0; i < ARRAY_SIZE(fatal_axi_error); i++) {
const struct hisi_sas_hw_error *error = _axi_error[i];
@@ -,6 +2244,29 @@ static void hisi_sas_v3_remove(struct pci_dev *pdev)
{ .irq_msk = BIT(31), 

[PATCH 6/8] scsi: hisi_sas: check IPTT is valid before using it for v3 hw

2018-03-23 Thread John Garry
From: Xiaofei Tan 

There is a bug of v3 hw development version. When AXI error
happen, hw may return an abnormal CQ that IPTT value is 0x.
This will cause IPTT out-of-bounds reference.

This patch add an check of IPTT in cq_tasklet_v3_hw(), and
discard invalid slot. This workaround scheme is just to enhance
fault-tolerance of the driver. So, we will apply this scheme for
all version of v3 hw, although release version has fixed this SoC
bug.

Signed-off-by: Xiaofei Tan 
Signed-off-by: John Garry 
---
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index efe64bc..aa52d5e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -1731,15 +1731,19 @@ static void cq_tasklet_v3_hw(unsigned long val)
 
while (rd_point != wr_point) {
struct hisi_sas_complete_v3_hdr *complete_hdr;
+   struct device *dev = hisi_hba->dev;
int iptt;
 
complete_hdr = _queue[rd_point];
 
iptt = (complete_hdr->dw1) & CMPLT_HDR_IPTT_MSK;
-   slot = _hba->slot_info[iptt];
-   slot->cmplt_queue_slot = rd_point;
-   slot->cmplt_queue = queue;
-   slot_complete_v3_hw(hisi_hba, slot);
+   if (likely(iptt < HISI_SAS_COMMAND_ENTRIES_V3_HW)) {
+   slot = _hba->slot_info[iptt];
+   slot->cmplt_queue_slot = rd_point;
+   slot->cmplt_queue = queue;
+   slot_complete_v3_hw(hisi_hba, slot);
+   } else
+   dev_err(dev, "IPTT %d is invalid, discard it.\n", iptt);
 
if (++rd_point >= HISI_SAS_QUEUE_SLOTS)
rd_point = 0;
-- 
1.9.1



[PATCH 0/8] hisi_sas: some misc changes

2018-03-23 Thread John Garry
This patchset introduces some minor, more trivial patches,
some of which have been sitting on our internal dev branch
for a while.

The only bug fixes (hw workaround or sw) are the IPTT range
check fix and a timer fix for module removal. Others are
trivial.

John Garry (2):
  scsi: hisi_sas: print device id for errors
  scsi: hisi_sas: remove some unneeded structure members

Xiang Chen (3):
  scsi: hisi_sas: make SAS address of SATA disks unique
  scsi: hisi_sas: delete timer when removing hisi_sas driver
  scsi: hisi_sas: use dma_zalloc_coherent()

Xiaofei Tan (3):
  scsi: hisi_sas: update RAS feature for later revision of v3 HW
  scsi: hisi_sas: consolidate command check in
hisi_sas_get_ata_protocol()
  scsi: hisi_sas: check IPTT is valid before using it for v3 hw

 drivers/scsi/hisi_sas/hisi_sas.h   |  3 --
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 53 +-
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c |  8 ++--
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 81 +-
 4 files changed, 94 insertions(+), 51 deletions(-)

-- 
1.9.1



[PATCH 7/8] scsi: hisi_sas: print device id for errors

2018-03-23 Thread John Garry
When we find an erroneous slot completion, to help aid
debugging add the device index to the current debug
log.

Signed-off-by: John Garry 
Reviewed-by: Xiang Chen 
---
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 4 ++--
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index bed6afb..a5abde8 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -2459,10 +2459,10 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
slot_err_v2_hw(hisi_hba, task, slot, 2);
 
if (ts->stat != SAS_DATA_UNDERRUN)
-   dev_info(dev, "erroneous completion iptt=%d task=%p "
+   dev_info(dev, "erroneous completion iptt=%d task=%p dev 
id=%d "
"CQ hdr: 0x%x 0x%x 0x%x 0x%x "
"Error info: 0x%x 0x%x 0x%x 0x%x\n",
-   slot->idx, task,
+   slot->idx, task, sas_dev->device_id,
complete_hdr->dw0, complete_hdr->dw1,
complete_hdr->act, complete_hdr->dw3,
error_info[0], error_info[1],
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index aa52d5e..760724a 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -1641,10 +1641,10 @@ static irqreturn_t fatal_axi_int_v3_hw(int irq_no, void 
*p)
 
slot_err_v3_hw(hisi_hba, task, slot);
if (ts->stat != SAS_DATA_UNDERRUN)
-   dev_info(dev, "erroneous completion iptt=%d task=%p "
+   dev_info(dev, "erroneous completion iptt=%d task=%p dev 
id=%d "
"CQ hdr: 0x%x 0x%x 0x%x 0x%x "
"Error info: 0x%x 0x%x 0x%x 0x%x\n",
-   slot->idx, task,
+   slot->idx, task, sas_dev->device_id,
complete_hdr->dw0, complete_hdr->dw1,
complete_hdr->act, complete_hdr->dw3,
error_info[0], error_info[1],
-- 
1.9.1



[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #13 from Bart Van Assche (bvanass...@acm.org) ---
Thanks for having run a bisect, that really helps.

Recently the following commit went upstream:

commit c9f926000fe3b84135a81602a9f7e63a6a7898e2 (mkp-scsi/4.15/scsi-fixes)
Author: Hannes Reinecke 
Date:   Wed Jan 10 09:34:02 2018 +0100

scsi: libsas: Disable asynchronous aborts for SATA devices

Handling CD-ROM devices from libsas is decidedly odd, as libata relies
on SCSI EH to be started to figure out that no medium is present.  So we
cannot do asynchronous aborts for SATA devices.

Fixes: 909657615d9 ("scsi: libsas: allow async aborts")
Cc:  # 4.12+
Signed-off-by: Hannes Reinecke 
Reviewed-by: Christoph Hellwig 
Tested-by: Yves-Alexis Perez 
Signed-off-by: Martin K. Petersen 

So you may want to try one of the kernel versions that includes that fix, e.g.
v4.14.15 or v4.15.

-- 
You are receiving this mail because:
You are the assignee for the bug.


[PATCH 8/8] scsi: hisi_sas: remove some unneeded structure members

2018-03-23 Thread John Garry
This patch removes unneeded structure elements:
- hisi_sas_phy.dev_sas_addr: only ever written
- Also remove associated function which writes it,
  hisi_sas_init_add().
- hisi_sas_device.attached_phy: only ever written
- Also remove code to set it in hisi_sas_dev_found()

Signed-off-by: John Garry 
Reviewed-by: Xiang Chen 
---
 drivers/scsi/hisi_sas/hisi_sas.h   |  3 ---
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 17 +
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c |  2 --
 3 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index d1153e8..d413d05 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -137,7 +137,6 @@ struct hisi_sas_phy {
struct asd_sas_phy  sas_phy;
struct sas_identify identify;
u64 port_id; /* from hw */
-   u64 dev_sas_addr;
u64 frame_rcvd_size;
u8  frame_rcvd[32];
u8  phy_attached;
@@ -174,7 +173,6 @@ struct hisi_sas_device {
struct completion *completion;
struct hisi_sas_dq  *dq;
struct list_headlist;
-   u64 attached_phy;
enum sas_device_typedev_type;
int device_id;
int sata_idx;
@@ -440,7 +438,6 @@ struct hisi_sas_slot_buf_table {
 extern struct scsi_host_template *hisi_sas_sht;
 
 extern void hisi_sas_stop_phys(struct hisi_hba *hisi_hba);
-extern void hisi_sas_init_add(struct hisi_hba *hisi_hba);
 extern int hisi_sas_alloc(struct hisi_hba *hisi_hba, struct Scsi_Host *shost);
 extern void hisi_sas_free(struct hisi_hba *hisi_hba);
 extern u8 hisi_sas_get_ata_protocol(struct host_to_dev_fis *fis,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 8557fd0..d1a61b1 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -577,10 +577,8 @@ static int hisi_sas_dev_found(struct domain_device *device)
for (phy_no = 0; phy_no < phy_num; phy_no++) {
phy = _dev->ex_dev.ex_phy[phy_no];
if (SAS_ADDR(phy->attached_sas_addr) ==
-   SAS_ADDR(device->sas_addr)) {
-   sas_dev->attached_phy = phy_no;
+   SAS_ADDR(device->sas_addr))
break;
-   }
}
 
if (phy_no == phy_num) {
@@ -2079,17 +2077,6 @@ static struct Scsi_Host *hisi_sas_shost_alloc(struct 
platform_device *pdev,
return NULL;
 }
 
-void hisi_sas_init_add(struct hisi_hba *hisi_hba)
-{
-   int i;
-
-   for (i = 0; i < hisi_hba->n_phy; i++)
-   memcpy(_hba->phy[i].dev_sas_addr,
-  hisi_hba->sas_addr,
-  SAS_ADDR_SIZE);
-}
-EXPORT_SYMBOL_GPL(hisi_sas_init_add);
-
 int hisi_sas_probe(struct platform_device *pdev,
 const struct hisi_sas_hw *hw)
 {
@@ -2143,8 +2130,6 @@ int hisi_sas_probe(struct platform_device *pdev,
sha->sas_port[i] = _hba->port[i].sas_port;
}
 
-   hisi_sas_init_add(hisi_hba);
-
rc = scsi_add_host(shost, >dev);
if (rc)
goto err_out_ha;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 760724a..33735a7 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2134,8 +2134,6 @@ static int soft_reset_v3_hw(struct hisi_hba *hisi_hba)
sha->sas_port[i] = _hba->port[i].sas_port;
}
 
-   hisi_sas_init_add(hisi_hba);
-
rc = scsi_add_host(shost, dev);
if (rc)
goto err_out_ha;
-- 
1.9.1



Re: [PATCH] target: Use WARNON_NON_RT(!irqs_disabled())

2018-03-23 Thread Sebastian Andrzej Siewior
On 2018-03-22 06:37:45 [-0300], Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 21, 2018 at 11:43:58AM -0700, Linus Torvalds escreveu:
> > [ Adding PeterZ to participants due to query about lockdep_assert() ]
> > 
> > On Wed, Mar 21, 2018 at 8:38 AM, Arnaldo Carvalho de Melo
> >  wrote:
> > >
> > > assert_spin_locked(>t_state_lock);
> > > -   WARN_ON_ONCE(!irqs_disabled());
> > > +   WARN_ON_ONCE_NONRT(!irqs_disabled());
> > 
> > Ugh.
> > 
> > Can't we just replace both of those with a lockdep annotation?
> 
> Huh, even better, when that feature gets finished (tglx said it was
> being developed, not there yet tho) it'll allow further reduction of the
> PREEMPT_RT_FULL patchkit.

I am going take this into -RT tree for now until we have different
solution. I will try to be kind and do the same change in
__transport_wait_for_tasks().
Arnaldo, please do "[PATCH RT]" while sending patches. Then the bots
don't complain if it applies but does not compile on !RT kernel (or so
I've been told).

Technically speaking the code wants to ensure that the lock is held and
the interrupts are disabled because the lock is always taken with
disabled interrupts. This kind of check could be done with
lockdep_assert_held(>t_state_lock);

but would require lockdep to be switched on. Nicholas, would you mind
such a change?

> - Arnaldo

Sebastian


Multi-Actuator SAS HDD First Look

2018-03-23 Thread Tim Walker
Seagate announced their split actuator SAS drive, which will probably
require some kernel changes for full support. It's targeted at cloud
provider JBODs and RAID.

Here are some of the drive's architectural points. Since the two LUNs
share many common components (e.g. spindle) Seagate allocated some
SCSI operations to be LUN specific and some to affect the entire
device, that is, both LUNs.

1. Two LUNs, 0 & 1, each with independent lba space, and each
connected to an independent read channel, actuator, and set of heads.
2. Each actuator addresses 1/2 of the media - no media is shared
across the actuators. They seek independently.
3. One World Wide Name (WWN) is assigned to the port for device
address. Each Logical Unit has a separate World Wide Name for
identification in VPD page.
4. 128 deep command queue, shared across both LUNs
5. Each LUN can pull commands from the queue independently, so they
can implement their own sorting and optimization.
6. Ordered tag attribute causes the command to be ordered across both
Logical Units
7. Head of Queue attribute causes the command to be ordered with
respect to a single Logical Unit
8. Mode pages are device-based (shared across both Logical Units)
9. Log pages are device-based.
10. Inquiry VPD pages (with minor exceptions) are device based.
11. Device health features (SMART, etc) are device based

Seagate wants the multi-actuator design to integrate into the stack as
painlessly as possible.The interface design is still in the early
stages, so I am gathering requirements and recommendations, and also
providing any information necessary to help scope integrating a
multi-LUN device into the MQ stack. So, I am soliciting any pertinent
feedback including:

1. Painful incompatibilities between the Seagate proposal and current
MQ architecture
2. Linux changes needed
3. Drive interface changes needed
4. Anything else I may have overlooked

Please feel free to send any questions or comments.

Tim Walker
Product Design Systems Engineering, Seagate Technology
(303) 775-3770


RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring buffer percentage

2018-03-23 Thread KY Srinivasan


> -Original Message-
> From: Haiyang Zhang
> Sent: Friday, March 23, 2018 8:01 AM
> To: Long Li ; KY Srinivasan
> ; Stephen Hemminger ;
> James E . J . Bottomley ; Martin K . Petersen
> ; de...@linuxdriverproject.org; linux-
> s...@vger.kernel.org; linux-ker...@vger.kernel.org
> Cc: Long Li 
> Subject: RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring
> buffer percentage
> 
> 
> 
> > -Original Message-
> > From: Long Li 
> > Sent: Thursday, March 22, 2018 8:16 PM
> > To: KY Srinivasan ; Haiyang Zhang
> > ; Stephen Hemminger
> ;
> > James E . J . Bottomley ; Martin K . Petersen
> > ; de...@linuxdriverproject.org; linux-
> > s...@vger.kernel.org; linux-ker...@vger.kernel.org
> > Cc: Long Li 
> > Subject: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring
> buffer
> > percentage
> >
> > From: Long Li 
> >
> > In Vmbus, we have defined a function to calculate available ring buffer
> > percentage to write.
> >
> > Use that function and remove duplicate netvsc code.
> >
> > Signed-off-by: Long Li 
> > ---
> >  drivers/net/hyperv/netvsc.c | 17 +++--
> >  drivers/net/hyperv/netvsc_drv.c |  3 ---
> >  2 files changed, 3 insertions(+), 17 deletions(-)

Why is the patch being sent to the scsi list and not to the network mailing 
list and
Dave Miller.

K. Y
> >
> > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index
> > 0265d703eb03..8af0069e4d8c 100644
> > --- a/drivers/net/hyperv/netvsc.c
> > +++ b/drivers/net/hyperv/netvsc.c
> > @@ -31,7 +31,6 @@
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> >
> >  #include 
> >
> > @@ -590,17 +589,6 @@ void netvsc_device_remove(struct hv_device
> *device)
> > #define RING_AVAIL_PERCENT_HIWATER 20  #define
> > RING_AVAIL_PERCENT_LOWATER 10
> >
> > -/*
> > - * Get the percentage of available bytes to write in the ring.
> > - * The return value is in range from 0 to 100.
> > - */
> > -static u32 hv_ringbuf_avail_percent(const struct hv_ring_buffer_info
> > *ring_info) -{
> > -   u32 avail_write = hv_get_bytes_to_write(ring_info);
> > -
> > -   return reciprocal_divide(avail_write  * 100, netvsc_ring_reciprocal);
> > -}
> > -
> >  static inline void netvsc_free_send_slot(struct netvsc_device
> *net_device,
> >  u32 index)
> >  {
> > @@ -649,7 +637,8 @@ static void netvsc_send_tx_complete(struct
> > netvsc_device *net_device,
> > wake_up(_device->wait_drain);
> >
> > if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx))
> &&
> > -   (hv_ringbuf_avail_percent(>outbound) >
> > RING_AVAIL_PERCENT_HIWATER ||
> > +   (hv_get_avail_to_write_percent(>outbound) >
> > +RING_AVAIL_PERCENT_HIWATER ||
> >  queue_sends < 1)) {
> > netif_tx_wake_queue(netdev_get_tx_queue(ndev, q_idx));
> > ndev_ctx->eth_stats.wake_queue++;
> > @@ -757,7 +746,7 @@ static inline int netvsc_send_pkt(
> > struct netdev_queue *txq = netdev_get_tx_queue(ndev, packet-
> >q_idx);
> > u64 req_id;
> > int ret;
> > -   u32 ring_avail = hv_ringbuf_avail_percent(_channel-
> >outbound);
> > +   u32 ring_avail =
> > +hv_get_avail_to_write_percent(_channel->outbound);
> >
> > nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
> > if (skb)
> > diff --git a/drivers/net/hyperv/netvsc_drv.c
> b/drivers/net/hyperv/netvsc_drv.c
> > index faea0be18924..b0b1c2fd2b7b 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -35,7 +35,6 @@
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> >
> >  #include 
> >  #include 
> > @@ -55,7 +54,6 @@ static unsigned int ring_size __ro_after_init = 128;
> > module_param(ring_size, uint, S_IRUGO);
> MODULE_PARM_DESC(ring_size,
> > "Ring buffer size (# of pages)");  unsigned int netvsc_ring_bytes
> __ro_after_init;
> > -struct reciprocal_value netvsc_ring_reciprocal __ro_after_init;
> >
> >  static const u32 default_msg = NETIF_MSG_DRV | NETIF_MSG_PROBE |
> > NETIF_MSG_LINK | NETIF_MSG_IFUP |
> > @@ -2186,7 +2184,6 @@ static int __init netvsc_drv_init(void)
> > ring_size);
> > }
> > netvsc_ring_bytes = ring_size * PAGE_SIZE;
> > -   netvsc_ring_reciprocal = reciprocal_value(netvsc_ring_bytes);
> >
> > ret = vmbus_driver_register(_drv);
> > if (ret)
> > --
> 
> 
> Please also remove netvsc_ring_reciprocal from hyperv_net.h
> Thanks.
> 
> Reviewed-by: Haiyang Zhang 


RE: [PATCH 1/3] Vmbus: Add function to report available ring buffer to write in total ring size percentage

2018-03-23 Thread Haiyang Zhang


> -Original Message-
> From: Long Li 
> Sent: Thursday, March 22, 2018 8:16 PM
> To: KY Srinivasan ; Haiyang Zhang
> ; Stephen Hemminger ;
> James E . J . Bottomley ; Martin K . Petersen
> ; de...@linuxdriverproject.org; linux-
> s...@vger.kernel.org; linux-ker...@vger.kernel.org
> Cc: Long Li 
> Subject: [PATCH 1/3] Vmbus: Add function to report available ring buffer to
> write in total ring size percentage
> 
> From: Long Li 
> 
> Netvsc has a similar function to calculate how much ring buffer in percentage
> is available to write. This function is useful for storvsc and other vmbus 
> devices.
> 
> Define a similar function in vmbus to be used by storvsc.
> 
> Signed-off-by: Long Li 
> ---

Reviewed-by: Haiyang Zhang 



RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring buffer percentage

2018-03-23 Thread Haiyang Zhang


> -Original Message-
> From: Long Li 
> Sent: Thursday, March 22, 2018 8:16 PM
> To: KY Srinivasan ; Haiyang Zhang
> ; Stephen Hemminger ;
> James E . J . Bottomley ; Martin K . Petersen
> ; de...@linuxdriverproject.org; linux-
> s...@vger.kernel.org; linux-ker...@vger.kernel.org
> Cc: Long Li 
> Subject: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring buffer
> percentage
> 
> From: Long Li 
> 
> In Vmbus, we have defined a function to calculate available ring buffer
> percentage to write.
> 
> Use that function and remove duplicate netvsc code.
> 
> Signed-off-by: Long Li 
> ---
>  drivers/net/hyperv/netvsc.c | 17 +++--
>  drivers/net/hyperv/netvsc_drv.c |  3 ---
>  2 files changed, 3 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index
> 0265d703eb03..8af0069e4d8c 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -31,7 +31,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
> 
>  #include 
> 
> @@ -590,17 +589,6 @@ void netvsc_device_remove(struct hv_device *device)
> #define RING_AVAIL_PERCENT_HIWATER 20  #define
> RING_AVAIL_PERCENT_LOWATER 10
> 
> -/*
> - * Get the percentage of available bytes to write in the ring.
> - * The return value is in range from 0 to 100.
> - */
> -static u32 hv_ringbuf_avail_percent(const struct hv_ring_buffer_info
> *ring_info) -{
> - u32 avail_write = hv_get_bytes_to_write(ring_info);
> -
> - return reciprocal_divide(avail_write  * 100, netvsc_ring_reciprocal);
> -}
> -
>  static inline void netvsc_free_send_slot(struct netvsc_device *net_device,
>u32 index)
>  {
> @@ -649,7 +637,8 @@ static void netvsc_send_tx_complete(struct
> netvsc_device *net_device,
>   wake_up(_device->wait_drain);
> 
>   if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx)) &&
> - (hv_ringbuf_avail_percent(>outbound) >
> RING_AVAIL_PERCENT_HIWATER ||
> + (hv_get_avail_to_write_percent(>outbound) >
> +  RING_AVAIL_PERCENT_HIWATER ||
>queue_sends < 1)) {
>   netif_tx_wake_queue(netdev_get_tx_queue(ndev, q_idx));
>   ndev_ctx->eth_stats.wake_queue++;
> @@ -757,7 +746,7 @@ static inline int netvsc_send_pkt(
>   struct netdev_queue *txq = netdev_get_tx_queue(ndev, packet->q_idx);
>   u64 req_id;
>   int ret;
> - u32 ring_avail = hv_ringbuf_avail_percent(_channel->outbound);
> + u32 ring_avail =
> +hv_get_avail_to_write_percent(_channel->outbound);
> 
>   nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
>   if (skb)
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index faea0be18924..b0b1c2fd2b7b 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -35,7 +35,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
> 
>  #include 
>  #include 
> @@ -55,7 +54,6 @@ static unsigned int ring_size __ro_after_init = 128;
> module_param(ring_size, uint, S_IRUGO);  MODULE_PARM_DESC(ring_size,
> "Ring buffer size (# of pages)");  unsigned int netvsc_ring_bytes 
> __ro_after_init;
> -struct reciprocal_value netvsc_ring_reciprocal __ro_after_init;
> 
>  static const u32 default_msg = NETIF_MSG_DRV | NETIF_MSG_PROBE |
>   NETIF_MSG_LINK | NETIF_MSG_IFUP |
> @@ -2186,7 +2184,6 @@ static int __init netvsc_drv_init(void)
>   ring_size);
>   }
>   netvsc_ring_bytes = ring_size * PAGE_SIZE;
> - netvsc_ring_reciprocal = reciprocal_value(netvsc_ring_bytes);
> 
>   ret = vmbus_driver_register(_drv);
>   if (ret)
> --


Please also remove netvsc_ring_reciprocal from hyperv_net.h
Thanks.

Reviewed-by: Haiyang Zhang 


[PATCH] scsi: qla2xxx: Fix small memory leak in qla2x00_probe_one on probe failure

2018-03-23 Thread Bill Kuzeja
The code that fixes the crashes in the following commit introduced a
small memory leak:

commit 6a2cf8d3663e ("scsi: qla2xxx: Fix crashes in qla2x00_probe_one on probe 
failure")

Fixing this requires a bit of reworking, which I've explained. Also provide
some code cleanup.

Fixes: 6a2cf8d3663e ("scsi: qla2xxx: Fix crashes in qla2x00_probe_one on probe 
failure")
Signed-off-by: Bill Kuzeja 

---

There is a small window in qla2x00_probe_one where if qla2x00_alloc_queues
fails, we end up never freeing req and rsp and leak 0xc0 and 0xc8 bytes
respectively (the sizes of req and rsp).

I originally put in checks to test for this condition which were based on
the incorrect assumption that if ha->rsp_q_map and ha->req_q_map were
allocated, then rsp and req were allocated as well. This is incorrect.
There is a window between these allocations:

   ret = qla2x00_mem_alloc(ha, req_length, rsp_length, , );
goto probe_hw_failed;

[if successful, both rsp and req allocated]

   base_vha = qla2x00_create_host(sht, ha);
goto probe_hw_failed;

   ret = qla2x00_request_irqs(ha, rsp);
goto probe_failed;

   if (qla2x00_alloc_queues(ha, req, rsp)) {
goto probe_failed;

[if successful, now ha->rsp_q_map and ha->req_q_map allocated]

To simplify this, we should just set req and rsp to NULL after we free
them. Sounds simple enough? The problem is that req and rsp are pointers
defined in the qla2x00_probe_one and they are not always passed by
reference to the routines that free them.

Here are paths which can free req and rsp:

PATH 1:
qla2x00_probe_one
   ret = qla2x00_mem_alloc(ha, req_length, rsp_length, , );
   [req and rsp are passed by reference, but if this fails, we currently
do not NULL out req and rsp. Easily fixed]

PATH 2:
qla2x00_probe_one
   failing in qla2x00_request_irqs or qla2x00_alloc_queues
  probe_failed:
 qla2x00_free_device(base_vha);
qla2x00_free_req_que(ha, req)
qla2x00_free_rsp_que(ha, rsp)

PATH 3:
qla2x00_probe_one:
   failing in qla2x00_mem_alloc or qla2x00_create_host
  probe_hw_failed:
 qla2x00_free_req_que(ha, req)
 qla2x00_free_rsp_que(ha, rsp)

PATH 1: This should currently work, but it doesn't because rsp and rsp
are not set to NULL in qla2x00_mem_alloc. Easily remedied.

PATH 2: req and rsp aren't passed in at all to qla2x00_free_device but are
derived from ha->req_q_map[0] and ha->rsp_q_map[0]. These are only set up
if qla2x00_alloc_queues succeeds.

In qla2x00_free_queues, we are protected from crashing if these don't
exist because req_qid_map and rsp_qid_map are only set on their
allocation. We are guarded in this way:

for (cnt = 0; cnt < ha->max_req_queues; cnt++) {
if (!test_bit(cnt, ha->req_qid_map))
continue;

PATH 3: This works. We haven't freed req or rsp yet (or they were never
allocated if qla2x00_mem_alloc failed), so we'll attempt to free them
here.

To summarize, there are a few small changes to make this work correctly and
(and for some cleanup):

1) (For PATH 1) Set *rsp and *req to NULL in case of failure in
qla2x00_mem_alloc so these are correctly set to NULL back in
qla2x00_probe_one

2) After jumping to probe_failed: and calling qla2x00_free_device,
explicitly set rsp and req to NULL so further calls with these pointers
do not crash, i.e. the free queue calls in the probe_hw_failed section
we fall through to.

3) Fix return code check in the call to qla2x00_alloc_queues. We currently
drop the return code on the floor. The probe fails but the caller of the
probe doesn't have an error code, so it attaches to pci. This can result
in a crash on module shutdown.

4) Remove unnecessary NULL checks in qla2x00_free_req_que,
qla2x00_free_rsp_que, and the egregious NULL checks before kfrees and
vfrees in qla2x00_mem_free.

I tested this out running a scenario where the card breaks at various
times during initialization. I made sure I forced every error exit path
in qla2x00_probe_one.

---
 drivers/scsi/qla2xxx/qla_os.c | 44 +--
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 5c5dcca4..f70d7eb 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -471,9 +471,6 @@ static int qla2x00_alloc_queues(struct qla_hw_data *ha, 
struct req_que *req,
 
 static void qla2x00_free_req_que(struct qla_hw_data *ha, struct req_que *req)
 {
-   if (!ha->req_q_map)
-   return;
-
if (IS_QLAFX00(ha)) {
if (req && req->ring_fx00)
dma_free_coherent(>pdev->dev,
@@ -484,17 +481,14 @@ static void qla2x00_free_req_que(struct qla_hw_data *ha, 
struct req_que *req)
(req->length + 1) * sizeof(request_t),
req->ring, req->dma);
 
-   if (req) {
+   

[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #12 from Chuck Burt (chuck.burt+kernel@gmail.com) ---
I finally got around to bisecting.

I had to do it twice as I identified two issues here.

Git Bisect Log 1 - https://bugzilla.kernel.org/attachment.cgi?id=274895
This identifies the commit where processes would full on hang as a result of
the drive being connected.

Git Bisect Log 2 - https://bugzilla.kernel.org/attachment.cgi?id=274897
This identifies a separate issue (should I file a separate bug for this?) where
mounting/unmounting caused error:

> Device /dev/sdb3 is already mounted at `/media/temp/[identifier]`. 
> (udisks-error-quark, 6)

-- 
You are receiving this mail because:
You are the assignee for the bug.


[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #11 from Chuck Burt (chuck.burt+kernel@gmail.com) ---
Created attachment 274897
  --> https://bugzilla.kernel.org/attachment.cgi?id=274897=edit
Git Bisect Log 2 - 20180323

-- 
You are receiving this mail because:
You are the assignee for the bug.


[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2018-03-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #10 from Chuck Burt (chuck.burt+kernel@gmail.com) ---
Created attachment 274895
  --> https://bugzilla.kernel.org/attachment.cgi?id=274895=edit
Git Bisect Log 1 - 20180323

-- 
You are receiving this mail because:
You are the assignee for the bug.


[PATCH] scsi: don't look for NULL devices handlers by name

2018-03-23 Thread Johannes Thumshirn
Currently scsi_dh_lookup() doesn't check for NULL as a device
name. This combined with nvme over dm-mapth results in the following
messages emitted by device-mapper:

 device-mapper: multipath: Could not failover device 259:67: Handler 
scsi_dh_(null) error 14.

Let scsi_dh_lookup() fail fast on NULL names.

Signed-off-by: Johannes Thumshirn 
---
 drivers/scsi/scsi_dh.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/scsi_dh.c b/drivers/scsi/scsi_dh.c
index b88b5dbbc444..188f30572aa1 100644
--- a/drivers/scsi/scsi_dh.c
+++ b/drivers/scsi/scsi_dh.c
@@ -112,6 +112,9 @@ static struct scsi_device_handler *scsi_dh_lookup(const 
char *name)
 {
struct scsi_device_handler *dh;
 
+   if (!name || strlen(name) == 0)
+   return NULL;
+
dh = __scsi_dh_lookup(name);
if (!dh) {
request_module("scsi_dh_%s", name);
-- 
2.13.6