Re: [PATCH v2 3/7] drivers/base: Probe devices concurrently if requested by the driver

2018-10-18 Thread Greg KH
On Thu, Oct 18, 2018 at 09:51:03AM -0700, Alexander Duyck wrote:
> On 10/18/2018 9:46 AM, Bart Van Assche wrote:
> > On Thu, 2018-10-18 at 08:25 -0700, Alexander Duyck wrote:
> > > On 10/17/2018 5:54 PM, Dan Williams wrote:
> > > > On Wed, Oct 17, 2018 at 4:41 PM Bart Van Assche  
> > > > wrote:
> > > > > 
> > > > > Instead of probing devices sequentially in the 
> > > > > PROBE_PREFER_ASYNCHRONOUS
> > > > > mode, scan devices concurrently. This helps when the wall clock time 
> > > > > for
> > > > > a single probe is significantly above the CPU time needed for a single
> > > > > probe, e.g. when scanning SCSI LUNs over a storage network.
> > > > 
> > > > Alex had a similar patch here [1] that I don't think has been accepted
> > > > yet, in any event some collaboration is needed:
> > > > 
> > > > [1]: https://lkml.org/lkml/2018/9/27/14
> > > 
> > > The patch set referenced is a little out of date. The latest set is:
> > > https://lore.kernel.org/lkml/20181015150305.29520.86363.stgit@localhost.localdomain/
> > > 
> > > I'm also not quite sure what the point of this patch is. I don't think
> > > it is doing what it says it is doing. From what I can tell it is just
> > > allowing the driver init code to ignore if the driver wants to be probed
> > > asynchronously or not. Further comments inline below.
> > 
> > Hi Alexander,
> > 
> > Thanks for the pointer to the latest version of your patch series. I was not
> > yet aware of your work when I posted this patch series. Now that I have had 
> > a
> > look at your patch series I like your approach better than what I did in 
> > this
> > patch. Since it could take a while before agreement is reached about the 
> > async
> > domain patches in the same patch series, how about you submitting patches 
> > 3/6
> > and 4/6 from your patch series to Greg for kernel version v4.20? If I drop
> > the driver core patches from my patch series and replace these with your
> > driver core patches I achieve the same results. If you Cc me when you 
> > resubmit
> > these patches I will review them.
> > 
> > Thanks,
> > 
> > Bart.
> 
> Actually the async and workqueue patches have already been reviewed and last
> I knew they were okay with the workqueue guys. These patches are already
> submitted to Greg for 4.20.

It's too late for 4.20 now, sorry.  They will have to wait.  Given that
4.19-final could have come out last weekend, this shouldn't be a
supprise.

They are in my review queue and I'll get to them after 4.20-rc1 is out.

thanks,

greg k-h


Re: [PATCH v2 3/7] drivers/base: Probe devices concurrently if requested by the driver

2018-10-17 Thread Greg KH
On Wed, Oct 17, 2018 at 05:54:56PM -0700, Dan Williams wrote:
> On Wed, Oct 17, 2018 at 4:41 PM Bart Van Assche  wrote:
> >
> > Instead of probing devices sequentially in the PROBE_PREFER_ASYNCHRONOUS
> > mode, scan devices concurrently. This helps when the wall clock time for
> > a single probe is significantly above the CPU time needed for a single
> > probe, e.g. when scanning SCSI LUNs over a storage network.
> 
> Alex had a similar patch here [1] that I don't think has been accepted
> yet, in any event some collaboration is needed:
> 
> [1]: https://lkml.org/lkml/2018/9/27/14

Yes, they both need to work together here...

thanks for pointing this out.

greg k-h


Re: [PATCH 2/2] ata: Fix ZBC_OUT all bit handling

2018-06-26 Thread Greg KH
On Tue, Jun 26, 2018 at 04:18:38PM +0900, Damien Le Moal wrote:
> If the ALL bit is set in the ZBC_OUT command, the command zone ID field
> (block) should be ignored.
> 
> Reported-by: David Butterfield 
> Signed-off-by: Damien Le Moal 
> ---
>  drivers/ata/libata-scsi.c | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.




Re: [PATCH 1/2] ata: Fix ZBC_OUT command block check

2018-06-26 Thread Greg KH
On Tue, Jun 26, 2018 at 04:18:37PM +0900, Damien Le Moal wrote:
> The block (LBA) specified must not exceed the last addressable LBA,
> which is dev->nr_sectors - 1. So fix the correct check is
> "if (block >= dev->n_sectors)" and not "if (block > dev->n_sectords)".
> 
> Additionally, the asc/ascq to return for an LBA that is not a zone start
> LBA should be ILLEGAL REQUEST, regardless if the bad LBA is out of
> range.
> 
> Reported-by: David Butterfield 
> Signed-off-by: Damien Le Moal 



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.




Re: [PATCH] [SCSI] mpt3sas: Fix secure erase premature termination (v4)

2018-04-24 Thread Greg KH
On Mon, Apr 23, 2018 at 06:28:03PM +, Igor Rybak wrote:
> Hi,
> 
> We are running kernel 4.4.0-22 and the patch below does not seem to be 
> present in the mpt3sas driver. Can you please confirm?

Please update your kernel, this patch was in the 4.4.36 kernel release
which came out December 2, 2016, well over a full year ago.

thanks,

greg k-h


Re: [PATCH 00/15] mpt3sas: Enhancements and Defect fixes.

2018-03-30 Thread Greg KH
On Fri, Mar 30, 2018 at 03:07:09PM +0530, Chaitra P B wrote:
> Chaitra P B (15):
>   mpt3sas: Fixed warnings.
>   mpt3sas: Pre-allocate RDPQ Array at driver boot time.
>   mpt3sas: Add sanity checks for scsi tracker before accessing it.
>   mpt3sas: Lockless access for chain buffers.
>   mpt3sas: Optimize I/O memory consumption in driver.
>   mpt3sas: Enhanced handling of Sense Buffer.
>   mpt3sas: Added support for SAS Device Discovery Error Event.
>   mpt3sas: Increase event log buffer to support 24 port HBA's.
>   mpt3sas: Allow processing of events during driver unload.
>   mpt3sas: Cache enclosure pages during enclosure add.
>   mpt3sas: Report Firmware Package Version from HBA Driver.
>   mpt3sas: Update MPI Headers
>   mpt3sas: For NVME device, issue a protocol level reset instead of
> hot reset and use TM timeout value exposed in PCIe Device Page  2.
>   mpt3sas: fix possible memory leak.
>   mpt3sas: Update driver version "25.100.00.00"
> 
>  drivers/scsi/mpt3sas/mpi/mpi2.h  |   9 +-
>  drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h |  30 +-
>  drivers/scsi/mpt3sas/mpi/mpi2_init.h |   2 +-
>  drivers/scsi/mpt3sas/mpi/mpi2_ioc.h  |   7 +-
>  drivers/scsi/mpt3sas/mpt3sas_base.c  | 484 ++---
>  drivers/scsi/mpt3sas/mpt3sas_base.h  |  62 +++-
>  drivers/scsi/mpt3sas/mpt3sas_ctl.c   |  38 ++-
>  drivers/scsi/mpt3sas/mpt3sas_ctl.h   |   2 +-
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 503 
> +--
>  drivers/scsi/mpt3sas/mpt3sas_warpdrive.c |   3 +-
>  10 files changed, 840 insertions(+), 300 deletions(-)
> 
> -- 
> 1.8.3.1




This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.




Re: [PATCH] scsi: storvsc: missing error code in storvsc_probe()

2018-02-09 Thread Greg KH
On Thu, Feb 08, 2018 at 04:50:40PM -0700, Long Li wrote:
> From: Long Li 

No, Dan wrote the first patch here, don't change the authorship of a
patch :(

Now fixed up by hand...


Re: [PATCH v4 01/10] ufs: sysfs: attribute group for existing sysfs entries.

2018-02-04 Thread Greg KH
On Sun, Feb 04, 2018 at 01:09:09PM +, Stanislav Nijnikov wrote:
> 
> 
> > -Original Message-
> > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > Sent: Sunday, February 4, 2018 2:34 PM
> > To: Stanislav Nijnikov <stanislav.nijni...@wdc.com>
> > Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org;
> > jaeg...@kernel.org; Alex Lemberg <alex.lemb...@wdc.com>
> > Subject: Re: [PATCH v4 01/10] ufs: sysfs: attribute group for existing sysfs
> > entries.
> > 
> > On Sun, Feb 04, 2018 at 12:29:06PM +, Stanislav Nijnikov wrote:
> > > > > + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> > > > > +  "\nAll available Runtime PM levels 
> > > > > info:\n");
> > > > > + for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
> > > > > + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - 
> > > > > curr_len),
> > > > > +  "\tRuntime PM level [%d] => 
> > > > > dev_state
> > > > [%s] link_state [%s]\n",
> > > > > + lvl,
> > > > > + ufschd_ufs_dev_pwr_mode_to_string(
> > > > > + 
> > > > > ufs_pm_lvl_states[lvl].dev_state),
> > > > > + ufschd_uic_link_state_to_string(
> > > > > + 
> > > > > ufs_pm_lvl_states[lvl].link_state));
> > > > > +
> > > >
> > > > sysfs if "one value per file", not "random text that someone has to
> > > > parse per file" please.
> > > >
> > > > Huge hint, if you ever care about checking the size of the sysfs
> > > > buffer you are writing into, you are doing something really really 
> > > > wrong.
> > > >
> > > Hi Greg
> > > It's the existing code, added by:
> > > commit 09690d5a6ae1b7e4cb5ac429c311b99d09352c12
> > > Author: subha...@codeaurora.org <subha...@codeaurora.org>
> > > Date:   Thu Dec 22 18:41:00 2016 -0800
> > >
> > > scsi: ufs: provide sysfs attribute to select the PM level
> > >
> > > This patch provides the sysfs attribute to choose the power management
> > > level for UFS runtime and system suspend.
> > >
> > > Reviewed-by: Sujit Reddy Thumma <sthu...@codeaurora.org>
> > > Signed-off-by: Subhash Jadavani <subha...@codeaurora.org>
> > > Signed-off-by: Martin K. Petersen <martin.peter...@oracle.com>
> > >
> > > I just moved it to an another file and changed the sysfs entries
> > > creation by Jaegeuk Kim' request. At the moment the entry shows the PM
> > > level, the device state, the link state and all possible PM levels. Do you
> > want me to change it?
> > 
> > Ah, you are just moving this code around.  Ok, that's fine for this patch, 
> > but
> > please fix it up as part of this patch series because this isn't an 
> > acceptable
> > sysfs file at all.  If it were documented that would be a lot more obvious 
> > as to
> > just how wrong it was :(
> > 
> > And, as it wasn't documented, you can change it as it's obvious no one used 
> > it
> > :)
> > 
> > thanks,
> > 
> > greg k-h
>  
> Can I fix these entries not in this patchset? As long as I know they are used 
> and I
> would prefer that change of the existing sysfs entries behavior be related to 
> a
> separate patch.

Ok, but it's nice to at least hope that someone will fix it up soon :)

thanks,

greg k-h


Re: [PATCH v4 01/10] ufs: sysfs: attribute group for existing sysfs entries.

2018-02-04 Thread Greg KH
On Sun, Feb 04, 2018 at 12:29:06PM +, Stanislav Nijnikov wrote:
> > > + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> > > +  "\nAll available Runtime PM levels info:\n");
> > > + for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
> > > + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> > > +  "\tRuntime PM level [%d] => dev_state
> > [%s] link_state [%s]\n",
> > > + lvl,
> > > + ufschd_ufs_dev_pwr_mode_to_string(
> > > + ufs_pm_lvl_states[lvl].dev_state),
> > > + ufschd_uic_link_state_to_string(
> > > + ufs_pm_lvl_states[lvl].link_state));
> > > +
> > 
> > sysfs if "one value per file", not "random text that someone has to parse 
> > per
> > file" please.
> > 
> > Huge hint, if you ever care about checking the size of the sysfs buffer you 
> > are
> > writing into, you are doing something really really wrong.
> > 
> Hi Greg
> It's the existing code, added by:
> commit 09690d5a6ae1b7e4cb5ac429c311b99d09352c12
> Author: subha...@codeaurora.org 
> Date:   Thu Dec 22 18:41:00 2016 -0800
> 
> scsi: ufs: provide sysfs attribute to select the PM level
> 
> This patch provides the sysfs attribute to choose the power management
> level for UFS runtime and system suspend.
> 
> Reviewed-by: Sujit Reddy Thumma 
> Signed-off-by: Subhash Jadavani 
> Signed-off-by: Martin K. Petersen 
> 
> I just moved it to an another file and changed the sysfs entries creation by
> Jaegeuk Kim' request. At the moment the entry shows the PM level, the device
> state, the link state and all possible PM levels. Do you want me to change it?

Ah, you are just moving this code around.  Ok, that's fine for this
patch, but please fix it up as part of this patch series because this
isn't an acceptable sysfs file at all.  If it were documented that would
be a lot more obvious as to just how wrong it was :(

And, as it wasn't documented, you can change it as it's obvious no one
used it :)

thanks,

greg k-h


Re: [PATCH v4 10/10] ufs: sysfs: attributes

2018-02-01 Thread Greg KH
On Thu, Feb 01, 2018 at 06:15:46PM +0200, Stanislav Nijnikov wrote:
> +#define UFS_LUN_ATTRIBUTE(_name, _uname) 
>  \
> +static ssize_t _name##_attribute_show(struct device *dev,
>  \
> + struct device_attribute *attr, char *buf) \
> +{
>  \
> + u32 value;\
> + struct scsi_device *sdev = to_scsi_device(dev);   \
> + struct ufs_hba *hba = shost_priv(sdev->host); \
> + u8 lun = ufshcd_scsi_to_upiu_lun(sdev->lun);  \
> + if (ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,   \
> + QUERY_ATTR_IDN##_uname, lun, 0, ))  \
> + return -EINVAL;   \
> + return sprintf(buf, "0x%08X\n", value);   \
> +}
>  \
> +static DEVICE_ATTR_RO(_name##_attribute)
> +
> +UFS_LUN_ATTRIBUTE(dyn_cap_needed, _DYN_CAP_NEEDED);

Why create a macro when you only have one instance of its use?

thanks,

greg k-h


Re: [PATCH v4 03/10] ufs: sysfs: interconnect descriptor

2018-02-01 Thread Greg KH
On Thu, Feb 01, 2018 at 06:15:39PM +0200, Stanislav Nijnikov wrote:
> This patch introduces a sysfs group entry for the UFS interconnect
> descriptor parameters. The group adds "interconnect_descriptor" folder
> under the UFS driver sysfs entry (/sys/bus/platform/drivers/ufshcd/*).
> The parameters are shown as hexadecimal numbers. The full information
> about the parameters could be found at UFS specifications 2.1.
> 
> Signed-off-by: Stanislav Nijnikov 
> 
> Reviewed-by: Greg Kroah-Hartman 

Nit, you should not have blank lines between these two statements,
otherwise tools can get confused.

You do that in later patches as well.

thanks,

greg k-h


Re: [PATCH v4 02/10] ufs: sysfs: device descriptor

2018-02-01 Thread Greg KH
On Thu, Feb 01, 2018 at 06:15:38PM +0200, Stanislav Nijnikov wrote:
> +#define UFS_DESC_PARAM(_name, _puname, _duname, _size)   
>  \
> +static ssize_t _name##_show(struct device *dev,  
>  \
> + struct device_attribute *attr, char *buf) \
> +{
>  \
> + struct ufs_hba *hba = dev_get_drvdata(dev);   \
> + return ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_##_duname,   \
> + 0, _duname##_DESC_PARAM##_puname, \
> + buf, UFS_PARAM_##_size##_SIZE);   \
> +}
>  \
> +static DEVICE_ATTR_RO(_name)

Nit, use tabs in your lines here to line up the trailing \

Same for other places in this patch series.

thanks,

greg k-h


Re: [PATCH v4 01/10] ufs: sysfs: attribute group for existing sysfs entries.

2018-02-01 Thread Greg KH
On Thu, Feb 01, 2018 at 06:15:37PM +0200, Stanislav Nijnikov wrote:
> This patch introduces attribute group to show existing sysfs entries.
> 
> Signed-off-by: Stanislav Nijnikov 
> ---
>  drivers/scsi/ufs/Makefile|   3 +-
>  drivers/scsi/ufs/ufs-sysfs.c | 164 
> +++
>  drivers/scsi/ufs/ufs-sysfs.h |  22 ++
>  drivers/scsi/ufs/ufshcd.c| 156 ++--
>  drivers/scsi/ufs/ufshcd.h|   2 +
>  5 files changed, 194 insertions(+), 153 deletions(-)
>  create mode 100644 drivers/scsi/ufs/ufs-sysfs.c
>  create mode 100644 drivers/scsi/ufs/ufs-sysfs.h
> 
> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> index 9310c6c..918f579 100644
> --- a/drivers/scsi/ufs/Makefile
> +++ b/drivers/scsi/ufs/Makefile
> @@ -3,6 +3,7 @@
>  obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o 
> tc-dwc-g210.o
>  obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o 
> tc-dwc-g210.o
>  obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
> -obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
> +obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> +ufshcd-core-objs := ufshcd.o ufs-sysfs.o
>  obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
>  obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
> diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
> new file mode 100644
> index 000..cc68a90
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-sysfs.c
> @@ -0,0 +1,164 @@
> +//SPDX-License-Identifier: GPL-2.0-only
> +//Copyright (C) 2018 Western Digital Corporation
> +//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; version 2.
> +//
> +//This program is distributed in the hope that it will be useful, but
> +//WITHOUT ANY WARRANTY; without even the implied warranty of
> +//MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +//General Public License for more details.

No need to put the whole "This program" crud in the file here if you
have the SPDX line already.  We are going through the kernel tree and
removing all of the 700+ different ways we have this boilerplate code in
the tree, please do not add new ones.

Also, please put a ' ' after "//", it just looks ugly like this, don't
you think so?

Please fix this for all of the files you add in this patch series.

> +#include 
> +#include 
> +
> +#include "ufs-sysfs.h"
> +
> +static const char *ufschd_uic_link_state_to_string(
> + enum uic_link_state state)
> +{
> + switch (state) {
> + case UIC_LINK_OFF_STATE:return "OFF";
> + case UIC_LINK_ACTIVE_STATE: return "ACTIVE";
> + case UIC_LINK_HIBERN8_STATE:return "HIBERN8";
> + default:return "UNKNOWN";
> + }
> +}
> +
> +static const char *ufschd_ufs_dev_pwr_mode_to_string(
> + enum ufs_dev_pwr_mode state)
> +{
> + switch (state) {
> + case UFS_ACTIVE_PWR_MODE:   return "ACTIVE";
> + case UFS_SLEEP_PWR_MODE:return "SLEEP";
> + case UFS_POWERDOWN_PWR_MODE:return "POWERDOWN";
> + default:return "UNKNOWN";
> + }
> +}
> +
> +static inline ssize_t ufs_sysfs_pm_lvl_store(struct device *dev,
> +  struct device_attribute *attr,
> +  const char *buf, size_t count,
> +  bool rpm)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + unsigned long flags, value;
> +
> + if (kstrtoul(buf, 0, ))
> + return -EINVAL;
> +
> + if (value >= UFS_PM_LVL_MAX)
> + return -EINVAL;
> +
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + if (rpm)
> + hba->rpm_lvl = value;
> + else
> + hba->spm_lvl = value;
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> + return count;
> +}
> +
> +static ssize_t rpm_lvl_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + int curr_len;
> + u8 lvl;
> +
> + curr_len = snprintf(buf, PAGE_SIZE,
> + "\nCurrent Runtime PM level [%d] => dev_state [%s] 
> link_state [%s]\n",
> + hba->rpm_lvl,
> + ufschd_ufs_dev_pwr_mode_to_string(
> + ufs_pm_lvl_states[hba->rpm_lvl].dev_state),
> + ufschd_uic_link_state_to_string(
> + ufs_pm_lvl_states[hba->rpm_lvl].link_state));
> +
> + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> +  "\nAll available Runtime PM levels info:\n");
> + for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
> + curr_len += snprintf((buf + 

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

2018-01-11 Thread Greg KH
On Thu, Jan 11, 2018 at 02:15:12PM -0800, Dan Williams wrote:
> On Sat, Jan 6, 2018 at 1:03 AM, Greg KH <gre...@linuxfoundation.org> wrote:
> > On Fri, Jan 05, 2018 at 05:10:48PM -0800, Dan Williams wrote:
> >> Static analysis reports that 'handle' may be a user controlled value
> >> that is used as a data dependency to read 'sp' from the
> >> 'req->outstanding_cmds' array.  In order to avoid potential leaks of
> >> kernel memory values, block speculative execution of the instruction
> >> stream that could issue reads based on an invalid value of 'sp'. In this
> >> case 'sp' is directly dereferenced later in the function.
> >
> > I'm pretty sure that 'handle' comes from the hardware, not from
> > userspace, from what I can tell here.  If we want to start auditing
> > __iomem data sources, great!  But that's a bigger task, and one I don't
> > think we are ready to tackle...
> 
> I think it falls in the hygiene bucket of shutting off an array index
> from a source that could be under attacker control. Should we leave
> this one un-patched while we decide if we generally have a problem
> with trusting completion 'tags' from hardware? My vote is patch it for
> now.

Hah, if you are worried about "tags" from hardware, we have a lot more
auditing to do, right?  I don't think anyone has looked into just basic
"bounds checking" for that type of information.  For USB devices we have
_just_ started doing that over the past year, the odds of anyone looking
at PCI devices for this same problem is slim-to-none.

Again, here are my questions/objections right now to this series:
- How can we audit this stuff?
- How did you audit this stuff to find these usages?
- How do you know that this series fixes all of the issues?
- What exact tree/date did you run your audit against?
- How do you know that linux-next does not contain a boatload
  more problems that we need to go back and fix after 4.16-rc1
  is out?
- How can we prevent this type of pattern showing up again?
- How can we audit the data coming from hardware correctly?

I'm all for merging this series, but if anyone things that somehow the
whole problem is now "solved" in this area, they are sorely mistaken.

thanks,

greg k-h


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

2018-01-06 Thread Greg KH
On Sat, Jan 06, 2018 at 10:03:22AM +0100, Greg KH wrote:
> On Fri, Jan 05, 2018 at 05:10:48PM -0800, Dan Williams wrote:
> > Static analysis reports that 'handle' may be a user controlled value
> > that is used as a data dependency to read 'sp' from the
> > 'req->outstanding_cmds' array.  In order to avoid potential leaks of
> > kernel memory values, block speculative execution of the instruction
> > stream that could issue reads based on an invalid value of 'sp'. In this
> > case 'sp' is directly dereferenced later in the function.
> 
> I'm pretty sure that 'handle' comes from the hardware, not from
> userspace, from what I can tell here.  If we want to start auditing
> __iomem data sources, great!  But that's a bigger task, and one I don't
> think we are ready to tackle...

And as Peter Zijlstra has already mentioned, if we have to look at those
codepaths, USB drivers are the first up for that mess, so having access
to the coverity rules would be a great help in starting that effort.

thanks,

greg k-h


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

2018-01-06 Thread Greg KH
On Fri, Jan 05, 2018 at 05:10:48PM -0800, Dan Williams wrote:
> Static analysis reports that 'handle' may be a user controlled value
> that is used as a data dependency to read 'sp' from the
> 'req->outstanding_cmds' array.  In order to avoid potential leaks of
> kernel memory values, block speculative execution of the instruction
> stream that could issue reads based on an invalid value of 'sp'. In this
> case 'sp' is directly dereferenced later in the function.

I'm pretty sure that 'handle' comes from the hardware, not from
userspace, from what I can tell here.  If we want to start auditing
__iomem data sources, great!  But that's a bigger task, and one I don't
think we are ready to tackle...

thanks,

greg k-h


Re: [PATCH v3 1/9] ufs: sysfs: device descriptor

2018-01-02 Thread Greg KH
On Tue, Jan 02, 2018 at 02:04:39PM +, Stanislav Nijnikov wrote:
> 
> 
> > -Original Message-
> > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > Sent: Friday, December 29, 2017 11:23 AM
> > To: Stanislav Nijnikov <stanislav.nijni...@wdc.com>
> > Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org; Alex Lemberg
> > <alex.lemb...@wdc.com>
> > Subject: Re: [PATCH v3 1/9] ufs: sysfs: device descriptor
> > 
> > On Thu, Dec 28, 2017 at 03:29:06PM +0200, Stanislav Nijnikov wrote:
> > > --- /dev/null
> > > +++ b/drivers/scsi/ufs/ufs-sysfs.c
> > > @@ -0,0 +1,170 @@
> > > +/*
> > > +* UFS Device Management sysfs
> > > +*
> > > +* Copyright (C) 2017 Western Digital Corporation
> > > +*
> > > +* This program is free software; you can redistribute it and/or
> > > +* modify it under the terms of the GNU General Public License version
> > > +* 2 as published by the Free Software Foundation.
> > > +*
> > > +* This program is distributed in the hope that it will be useful, but
> > > +* WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > GNU
> > > +* General Public License for more details.
> > 
> > Please use the SPDX format for all of this, like I asked last time :(
> > 
> > thanks,
> > 
> > greg k-h
>  
> Hi Greg.
> Sorry about this.
> Is this the proper legal header?

First off, don't as a developer legal questions, ask your lawyers :)

> 
> /*
> * UFS Device Management sysfs
> *
> *Copyright (C) 2018 Western Digital Corporation
> *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; version 2.
> *
> *This program is distributed in the hope that it will be useful, but 
> *WITHOUT ANY WARRANTY; without even the implied warranty of
> *MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> *General Public License for more details.
> *
> *You should have received a copy of the GNU General Public License along
> *with this program; if not, write to the Free Software Foundation, Inc.,
> *51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 , USA.
> */
> 
> It was taken from <https://spdx.org/licenses/GPL-2.0-only.html#licenseText>
> 
> Thanks and Happy New Year!

Nope, sorry, see the email thread on lkml from Thomas where he documents
how to properly set the SPDX line and why you don't want any of the
"boiler plate" text in there at all.

thanks,

greg k-h


Re: [PATCH v3 1/9] ufs: sysfs: device descriptor

2017-12-29 Thread Greg KH
On Thu, Dec 28, 2017 at 03:29:06PM +0200, Stanislav Nijnikov wrote:
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-sysfs.c
> @@ -0,0 +1,170 @@
> +/*
> +* UFS Device Management sysfs
> +*
> +* Copyright (C) 2017 Western Digital Corporation
> +*
> +* This program is free software; you can redistribute it and/or
> +* modify it under the terms of the GNU General Public License version
> +* 2 as published by the Free Software Foundation.
> +*
> +* This program is distributed in the hope that it will be useful, but
> +* WITHOUT ANY WARRANTY; without even the implied warranty of
> +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +* General Public License for more details.

Please use the SPDX format for all of this, like I asked last time :(

thanks,

greg k-h


Re: [PATCH v2 6/9] ufs: sysfs: string descriptors

2017-12-27 Thread Greg KH
On Wed, Dec 27, 2017 at 05:13:44PM +0200, Stanislav Nijnikov wrote:
> This patch introduces a sysfs group entry for the UFS string descriptors.
> The group adds "string_descriptors" folder under the UFS driver
> sysfs entry (/sys/bus/platform/drivers/ufshcd/*). The folder will contain
> 5 files that will show string values defined by the UFS spec:
> a manufacturer name, a product name, an OEM id, a serial number and a
> product revision.  The full information about the string descriptors
> could be found at UFS specifications 2.1.
> 
> Signed-off-by: Stanislav Nijnikov 

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH v2 9/9] ufs: sysfs: attributes

2017-12-27 Thread Greg KH
On Wed, Dec 27, 2017 at 05:13:47PM +0200, Stanislav Nijnikov wrote:
> This patch introduces a sysfs group entry for the UFS attributes. The
> group adds "attributes" folder under the UFS driver sysfs entry
> (/sys/bus/platform/drivers/ufshcd/*). The attributes are shown
> as hexadecimal numbers. The full information about the attributes could
> be found at UFS specifications 2.1.
> 
> Signed-off-by: Stanislav Nijnikov 
> ---
>  Documentation/ABI/testing/sysfs-driver-ufs | 141 
> -
>  drivers/scsi/ufs/ufs-sysfs.c   |  90 ++
>  drivers/scsi/ufs/ufs-sysfs.h   |   1 +
>  drivers/scsi/ufs/ufs.h |  27 +-
>  drivers/scsi/ufs/ufshcd.c  |   6 +-
>  drivers/scsi/ufs/ufshcd.h  |   2 +
>  6 files changed, 260 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-ufs 
> b/Documentation/ABI/testing/sysfs-driver-ufs
> index 832da97..804e952 100644
> --- a/Documentation/ABI/testing/sysfs-driver-ufs
> +++ b/Documentation/ABI/testing/sysfs-driver-ufs
> @@ -662,4 +662,143 @@ Contact:Stanislav Nijnikov 
> 
>  Description: This file shows whether the device FW update is permanently
>   disabled. The full information about the flag could be found
>   at UFS specifications 2.1.
> - The file is read only.
> \ No newline at end of file
> + The file is read only.
> +
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/attributes/boot_lun_enabled
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file provides the boot lun enabled UFS device attribute.
> + The full information about the attribute could be found at
> + UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/attributes/current_power_mode
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file provides the current power mode UFS device attribute.
> + The full information about the attribute could be found at
> + UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/attributes/active_icc_level
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file provides the active icc level UFS device attribute.
> + The full information about the attribute could be found at
> + UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/attributes/ooo_data_enabled
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file provides the out of order data transfer enabled UFS
> + device attribute. The full information about the attribute
> + could be found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/attributes/bkops_status
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file provides the background operations status UFS device
> + attribute. The full information about the attribute could
> + be found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/attributes/purge_status
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file provides the purge operation status UFS device
> + attribute. The full information about the attribute could
> + be found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/attributes/max_data_in_size
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file shows the maximum data size in a DATA IN
> + UPIU. The full information about the attribute could
> + be found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/attributes/max_data_out_size
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file shows the maximum number of bytes that can be
> + requested with a READY TO TRANSFER UPIU. The full information
> + about the attribute could be found at UFS specifications 2.1.
> + The file is read 

Re: [PATCH v2 8/9] ufs: sysfs: flags

2017-12-27 Thread Greg KH
On Wed, Dec 27, 2017 at 05:13:46PM +0200, Stanislav Nijnikov wrote:
> This patch introduces a sysfs group entry for the UFS flags. The group adds
> "flags" folder under the UFS driver sysfs entry
> (/sys/bus/platform/drivers/ufshcd/*). The flags are shown as boolean value
> ("true" or "false"). The full information about the UFS flags could be
> found at UFS specifications 2.1.
> 
> Signed-off-by: Stanislav Nijnikov 
> ---
>  Documentation/ABI/testing/sysfs-driver-ufs | 65 
> ++
>  drivers/scsi/ufs/ufs-sysfs.c   | 42 +++
>  drivers/scsi/ufs/ufs.h | 14 +--
>  drivers/scsi/ufs/ufshcd.c  |  1 +
>  4 files changed, 119 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-ufs 
> b/Documentation/ABI/testing/sysfs-driver-ufs
> index 5ff8dfa..832da97 100644
> --- a/Documentation/ABI/testing/sysfs-driver-ufs
> +++ b/Documentation/ABI/testing/sysfs-driver-ufs
> @@ -597,4 +597,69 @@ Contact: Stanislav Nijnikov 
>  Description: This file shows the granularity of the LUN. This is one of
>   the UFS unit descriptor parameters. The full information
>   about the descriptor could be found at UFS specifications 2.1.
> + The file is read only.
> +
> +
> +What:/sys/bus/platform/drivers/ufshcd/*/flags/device_init
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file shows the device init status. The full information
> + about the flag could be found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:/sys/bus/platform/drivers/ufshcd/*/flags/permanent_wpe
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file shows whether permanent write protection is enabled.
> + The full information about the flag could be found at
> + UFS specifications 2.1.
> + The file is read only.
> +
> +What:/sys/bus/platform/drivers/ufshcd/*/flags/power_on_wpe
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file shows whether write protection is enabled on all
> + logical units configured as power on write protected. The
> + full information about the flag could be found at
> + UFS specifications 2.1.
> + The file is read only.
> +
> +What:/sys/bus/platform/drivers/ufshcd/*/flags/bkops_enable
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file shows whether the device background operations are
> + enabled. The full information about the flag could be
> + found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/flags/life_span_mode_enable
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file shows whether the device life span mode is enabled.
> + The full information about the flag could be found at
> + UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/flags/phy_resource_removal
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file shows whether physical resource removal is enable.
> + The full information about the flag could be found at
> + UFS specifications 2.1.
> + The file is read only.
> +
> +What:/sys/bus/platform/drivers/ufshcd/*/flags/busy_rtc
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file shows whether the device is executing internal
> + operation related to real time clock. The full information
> + about the flag could be found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/flags/disable_fw_update
> +Date:August 2017
> +Contact: Stanislav Nijnikov 
> +Description: This file shows whether the device FW update is permanently
> + disabled. The full information about the flag could be found
> + at UFS specifications 2.1.
>   The file is read only.
> \ No newline at end of file
> diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
> index 509abc9..de80c20 100644
> --- a/drivers/scsi/ufs/ufs-sysfs.c
> +++ b/drivers/scsi/ufs/ufs-sysfs.c
> @@ -427,6 +427,47 @@ static const 

Re: [PATCH v2 7/9] ufs: sysfs: unit descriptor

2017-12-27 Thread Greg KH
On Wed, Dec 27, 2017 at 05:13:45PM +0200, Stanislav Nijnikov wrote:
> This patch introduces a sysfs group entry for the UFS unit descriptor
> parameters. The group adds "unit_descriptor" folder under the corresponding
> SCSI device sysfs entry (/sys/class/scsi_device/*/device/). The parameters
> are shown as hexadecimal numbers. The full information about the parameters
> could be found at UFS specifications 2.1.
> In addition the patch presents an additional field in the
> scsi_host_template structure - struct attribute_group **sdev_group.
> This field allows to define groups of attributes. It will provide an
> ability to use binary attributes in addition to device attributes and
> to group them under subfolders if necessary.
> 
> Signed-off-by: Stanislav Nijnikov 
> ---
>  Documentation/ABI/testing/sysfs-driver-ufs | 108 
> +
>  drivers/scsi/scsi_sysfs.c  |  14 
>  drivers/scsi/ufs/ufs-sysfs.c   |  58 
>  drivers/scsi/ufs/ufs-sysfs.h   |   3 +
>  drivers/scsi/ufs/ufs.h |  11 +++
>  drivers/scsi/ufs/ufshcd.c  |  23 ++
>  drivers/scsi/ufs/ufshcd.h  |  15 
>  include/scsi/scsi_host.h   |   6 ++
>  8 files changed, 222 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-ufs 
> b/Documentation/ABI/testing/sysfs-driver-ufs
> index 736280e..5ff8dfa 100644
> --- a/Documentation/ABI/testing/sysfs-driver-ufs
> +++ b/Documentation/ABI/testing/sysfs-driver-ufs
> @@ -489,4 +489,112 @@ Contact:Stanislav Nijnikov 
> 
>  Description: This file contains a product revision string. The full
>   information about the descriptor could be found at
>   UFS specifications 2.1.
> + The file is read only.
> +
> +
> +What:
> /sys/class/scsi_device/*/device/unit_descriptor/boot_lun_id
> +Date:August 2017

Minor nit for all of these, August 2017 was a few months ago :)

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH v2 5/9] ufs: sysfs: power descriptor

2017-12-27 Thread Greg KH
On Wed, Dec 27, 2017 at 05:13:43PM +0200, Stanislav Nijnikov wrote:
> This patch introduces a sysfs group entry for the UFS power descriptor
> parameters. The group adds "power_descriptor" folder under the UFS driver
> sysfs entry (/sys/bus/platform/drivers/ufshcd/*). The parameters are shown
> as hexadecimal numbers. The full information about the parameters could be
> found at UFS specifications 2.1.
> 
> Signed-off-by: Stanislav Nijnikov 

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH v2 4/9] ufs: sysfs: health descriptor

2017-12-27 Thread Greg KH
On Wed, Dec 27, 2017 at 05:13:42PM +0200, Stanislav Nijnikov wrote:
> This patch introduces a sysfs group entry for the UFS health descriptor
> parameters. The group adds "health_descriptor" folder under the UFS driver
> sysfs entry (/sys/bus/platform/drivers/ufshcd/*). The parameters are shown
> as hexadecimal numbers. The full information about the parameters could be
> found at UFS specifications 2.1.
> 
> Signed-off-by: Stanislav Nijnikov 

Reviewed-by: Greg Kroah-Hartman 



Re: [PATCH v2 3/9] ufs: sysfs: geometry descriptor

2017-12-27 Thread Greg KH
On Wed, Dec 27, 2017 at 05:13:41PM +0200, Stanislav Nijnikov wrote:
> This patch introduces a sysfs group entry for the UFS geometry descriptor
> parameters. The group adds "geometry_descriptor" folder under the UFS
> driver sysfs entry (/sys/bus/platform/drivers/ufshcd/*). The parameters
> are shown as hexadecimal numbers. The full information about the parameters
> could be found at UFS specifications 2.1.
> 
> Signed-off-by: Stanislav Nijnikov 

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH v2 2/9] ufs: sysfs: interconnect descriptor

2017-12-27 Thread Greg KH
On Wed, Dec 27, 2017 at 05:13:40PM +0200, Stanislav Nijnikov wrote:
> This patch introduces a sysfs group entry for the UFS interconnect
> descriptor parameters. The group adds "interconnect_descriptor" folder
> under the UFS driver sysfs entry (/sys/bus/platform/drivers/ufshcd/*).
> The parameters are shown as hexadecimal numbers. The full information
> about the parameters could be found at UFS specifications 2.1.
> 
> Signed-off-by: Stanislav Nijnikov 

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH v2 1/9] ufs: sysfs: device descriptor

2017-12-27 Thread Greg KH
On Wed, Dec 27, 2017 at 05:13:39PM +0200, Stanislav Nijnikov wrote:
> diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
> new file mode 100644
> index 000..63a8e68
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-sysfs.c
> @@ -0,0 +1,158 @@
> +#include 
> +#include 
> +
> +#include "ufs.h"
> +#include "ufs-sysfs.h"

No SPDX license information or copyright info for this file?  That's
brave :(

> diff --git a/drivers/scsi/ufs/ufs-sysfs.h b/drivers/scsi/ufs/ufs-sysfs.h
> new file mode 100644
> index 000..369ba21
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-sysfs.h
> @@ -0,0 +1,10 @@
> +#ifndef __UFS_SYSFS_H__
> +#define __UFS_SYSFS_H__

Same comment here.

thanks,

greg k-h


Re: [PATCH v2 1/9] ufs: sysfs: device descriptor

2017-12-27 Thread Greg KH
On Wed, Dec 27, 2017 at 05:13:39PM +0200, Stanislav Nijnikov wrote:
> +EXPORT_SYMBOL(ufs_sysfs_add_device_management);

Whhy is this exported?  What external module uses it?

> +
> +void ufs_sysfs_remove_device_management(struct ufs_hba *hba)
> +{
> + sysfs_remove_groups(>dev->kobj, ufs_sysfs_groups);
> +}
> +EXPORT_SYMBOL(ufs_sysfs_remove_device_management);
> +
> +MODULE_LICENSE("GPL");

Are you sure you didn't just put 2 module license fields in the same
module?

Other than those nits, looks good!

thanks,

greg k-h


Re: [PATCH v1 1/9] ufs: sysfs: device descriptor

2017-12-27 Thread Greg KH
On Wed, Dec 27, 2017 at 03:49:28PM +0200, Stanislav Nijnikov wrote:
> Signed-off-by: Stanislav Nijnikov 
> ---

I never want to take a patch with no changelog text at all, and to have
a 9 patch series with nothing written in them at all?  That's not good.

Please fix up.

greg k-h


Re: [PATCH 1/2] scsi: ufs: introduce static sysfs entries

2017-12-20 Thread Greg KH
On Tue, Dec 19, 2017 at 12:02:53PM -0800, Jaegeuk Kim wrote:
> From: Jaegeuk Kim <jaeg...@google.com>
> 
> This patch introduces attribute group to show existing sysfs entries.
> 
> Cc: Greg KH <gre...@linuxfoundation.org>
> Signed-off-by: Jaegeuk Kim <jaeg...@google.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 48 
> +++
>  drivers/scsi/ufs/ufshcd.h |  2 --
>  2 files changed, 19 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 011c3369082c..12ff7daebb00 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7605,7 +7605,7 @@ static inline ssize_t ufshcd_pm_lvl_store(struct device 
> *dev,
>   return count;
>  }
>  
> -static ssize_t ufshcd_rpm_lvl_show(struct device *dev,
> +static ssize_t rpm_lvl_show(struct device *dev,
>   struct device_attribute *attr, char *buf)
>  {
>   struct ufs_hba *hba = dev_get_drvdata(dev);
> @@ -7634,24 +7634,13 @@ static ssize_t ufshcd_rpm_lvl_show(struct device *dev,
>   return curr_len;
>  }
>  
> -static ssize_t ufshcd_rpm_lvl_store(struct device *dev,
> +static ssize_t rpm_lvl_store(struct device *dev,
>   struct device_attribute *attr, const char *buf, size_t count)
>  {
>   return ufshcd_pm_lvl_store(dev, attr, buf, count, true);
>  }
>  
> -static void ufshcd_add_rpm_lvl_sysfs_nodes(struct ufs_hba *hba)
> -{
> - hba->rpm_lvl_attr.show = ufshcd_rpm_lvl_show;
> - hba->rpm_lvl_attr.store = ufshcd_rpm_lvl_store;
> - sysfs_attr_init(>rpm_lvl_attr.attr);
> - hba->rpm_lvl_attr.attr.name = "rpm_lvl";
> - hba->rpm_lvl_attr.attr.mode = 0644;
> - if (device_create_file(hba->dev, >rpm_lvl_attr))
> - dev_err(hba->dev, "Failed to create sysfs for rpm_lvl\n");
> -}
> -
> -static ssize_t ufshcd_spm_lvl_show(struct device *dev,
> +static ssize_t spm_lvl_show(struct device *dev,
>   struct device_attribute *attr, char *buf)
>  {
>   struct ufs_hba *hba = dev_get_drvdata(dev);
> @@ -7680,33 +7669,34 @@ static ssize_t ufshcd_spm_lvl_show(struct device *dev,
>   return curr_len;
>  }
>  
> -static ssize_t ufshcd_spm_lvl_store(struct device *dev,
> +static ssize_t spm_lvl_store(struct device *dev,
>   struct device_attribute *attr, const char *buf, size_t count)
>  {
>   return ufshcd_pm_lvl_store(dev, attr, buf, count, false);
>  }
>  
> -static void ufshcd_add_spm_lvl_sysfs_nodes(struct ufs_hba *hba)
> -{
> - hba->spm_lvl_attr.show = ufshcd_spm_lvl_show;
> - hba->spm_lvl_attr.store = ufshcd_spm_lvl_store;
> - sysfs_attr_init(>spm_lvl_attr.attr);
> - hba->spm_lvl_attr.attr.name = "spm_lvl";
> - hba->spm_lvl_attr.attr.mode = 0644;
> - if (device_create_file(hba->dev, >spm_lvl_attr))
> - dev_err(hba->dev, "Failed to create sysfs for spm_lvl\n");
> -}
> +static DEVICE_ATTR_RW(rpm_lvl);
> +static DEVICE_ATTR_RW(spm_lvl);
> +
> +static struct attribute *ufshcd_attrs[] = {
> + _attr_rpm_lvl.attr,
> + _attr_spm_lvl.attr,
> + NULL
> +};
> +
> +static const struct attribute_group ufshcd_attr_group = {
> + .attrs = ufshcd_attrs,
> +};

ATTRIBUTE_GROUPS()?

>  static inline void ufshcd_add_sysfs_nodes(struct ufs_hba *hba)
>  {
> - ufshcd_add_rpm_lvl_sysfs_nodes(hba);
> - ufshcd_add_spm_lvl_sysfs_nodes(hba);
> + if (sysfs_create_group(>dev->kobj, _attr_group))
> + dev_err(hba->dev, "Failed to create sysfs group\n");

That will turn this into sysfs_create_groups()

But really, you should be able to do this all "at once"  And really, it
should be a "default attribute group" that the driver core adds to the
device, but that's outside the scope of this patchset.

So for now, this is just fine, the attribute groups work is for an
add-on patch.  Thanks for working to get this upstream, I'm tired of
seeing all of the different variants of this driver floating around
out-of-tree.

Acked-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>


Re: [PATCH] driver core: Make it safe to use get_device() if the reference count is zero

2017-12-14 Thread Greg KH
On Thu, Dec 14, 2017 at 04:27:56PM +0800, Jason Yan wrote:
> 
> 
> On 2017/12/14 16:10, Greg KH wrote:
> > On Thu, Dec 14, 2017 at 03:56:46PM +0800, Jason Yan wrote:
> > > 
> > > On 2017/12/14 15:42, Greg KH wrote:
> > > > On Thu, Dec 14, 2017 at 11:39:36AM +0800, Jason Yan wrote:
> > > > > Some driviers may have the chance to increase a reference count that
> > > > > has dropped to zero when using get_device() because of their design.
> > > > Then those drivers are broken :)
> > > > 
> > > > > We have met such a issue with scsi:
> > > > > https://www.spinics.net/lists/linux-scsi/msg115295.html
> > > > > 
> > > > > The scsi core will keep the scsi device object in the host list after
> > > > > it has been deleted and the iterator can still find it. All of the
> > > > > places where need iterating have to check the state of the scsi device
> > > > > and this makes a lot of code redundancy and complexity.
> > > > > 
> > > > > Provide a safe mechanism in get_device() by using
> > > > > kobject_get_unless_zero().
> > > > > 
> > > > > Suggested-by: Bart Van Assche <bart.vanass...@wdc.com>
> > > > > Signed-off-by: Jason Yan <yanai...@huawei.com>
> > > > > CC: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> > > > > CC: Bart Van Assche <bart.vanass...@wdc.com>
> > > > > CC: Ewan D. Milne <emi...@redhat.com>
> > > > > CC: James E.J. Bottomley <j...@linux.vnet.ibm.com>
> > > > > CC: Christoph Hellwig <h...@lst.de>
> > > > > ---
> > > > >drivers/base/core.c | 2 +-
> > > > >1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > > > > index 12ebd05..cc74810 100644
> > > > > --- a/drivers/base/core.c
> > > > > +++ b/drivers/base/core.c
> > > > > @@ -1916,7 +1916,7 @@ EXPORT_SYMBOL_GPL(device_register);
> > > > > */
> > > > >struct device *get_device(struct device *dev)
> > > > >{
> > > > > - return dev ? kobj_to_dev(kobject_get(>kobj)) : NULL;
> > > > > + return dev && kobject_get_unless_zero(>kobj) ? dev : NULL;
> > > > I really don't want to do this, the bus the device is on should prevent
> > > > this from happening.
> > > > 
> > > > Also, once that reference count drops to zero, the memory should be
> > > > freed, so you really have a stale pointer here, and this code would fail
> > > > if you had slab debugging enabled anyway.
> > > 
> > > Actually I don't want this either. But the design of scsi core will leave
> > > the scsi device on the host list after it is deleted, and it can  be
> > > found later and the refcount have a very big chance to increase from
> > > zero again. And after a lot of discussion it seems that the scsi layer
> > > is difficult to change the situation in the near future.
> > 
> > Keeping a 'struct device' reference counted chunk of memory on a list
> > that has a different lifetime rule from that device itself, is crazy.
> > 
> 
> The lifetime rule is the same. That device itself will delete from the
> host list in the destructor, scsi_device_dev_release_usercontext(), and
> the memory will be freed after that. That's why this issue came out.
> 
> > And yes, I remember how all of this came about, but I really don't have
> > the time to work on it myself...
> > 
> > > > So I don't even think this fixes the issue you think it fixes :)
> > > 
> > > This issue is very easy to reproduce on my machine and I have tested the
> > > patch and it really fixes the issue.
> > 
> > Even with slab debugging enabled?  If so, what is keeping that memory
> > from being freed once the reference count drops to 0?
> > 
> 
> As above, the memory is not freed yet when we increasing the refconunt from
> zero, so it's nothing to do with slab debugging enabled or not. If
> we want to free it, we have to grab host lock first to delete it from
> the list, so if we are grabing the host lock, we can increase the
> refcount safely from 0 to 1.

Wait, what?  Once that refcount goes to 0, the release callback will be
called, and the memory had better be freed.  Any pointer you might still
have to the structure is now invalid.  The fact that you are somehow
still keeping that pointer around is not ok, and slab debugging should
have caused the memory to be overwritten and garbage would result if you
tried to make this call again.

thanks,

greg k-h


Re: [PATCH] driver core: Make it safe to use get_device() if the reference count is zero

2017-12-14 Thread Greg KH
On Thu, Dec 14, 2017 at 03:56:46PM +0800, Jason Yan wrote:
> 
> On 2017/12/14 15:42, Greg KH wrote:
> > On Thu, Dec 14, 2017 at 11:39:36AM +0800, Jason Yan wrote:
> > > Some driviers may have the chance to increase a reference count that
> > > has dropped to zero when using get_device() because of their design.
> > Then those drivers are broken :)
> > 
> > > We have met such a issue with scsi:
> > > https://www.spinics.net/lists/linux-scsi/msg115295.html
> > > 
> > > The scsi core will keep the scsi device object in the host list after
> > > it has been deleted and the iterator can still find it. All of the
> > > places where need iterating have to check the state of the scsi device
> > > and this makes a lot of code redundancy and complexity.
> > > 
> > > Provide a safe mechanism in get_device() by using
> > > kobject_get_unless_zero().
> > > 
> > > Suggested-by: Bart Van Assche <bart.vanass...@wdc.com>
> > > Signed-off-by: Jason Yan <yanai...@huawei.com>
> > > CC: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> > > CC: Bart Van Assche <bart.vanass...@wdc.com>
> > > CC: Ewan D. Milne <emi...@redhat.com>
> > > CC: James E.J. Bottomley <j...@linux.vnet.ibm.com>
> > > CC: Christoph Hellwig <h...@lst.de>
> > > ---
> > >   drivers/base/core.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > > index 12ebd05..cc74810 100644
> > > --- a/drivers/base/core.c
> > > +++ b/drivers/base/core.c
> > > @@ -1916,7 +1916,7 @@ EXPORT_SYMBOL_GPL(device_register);
> > >*/
> > >   struct device *get_device(struct device *dev)
> > >   {
> > > - return dev ? kobj_to_dev(kobject_get(>kobj)) : NULL;
> > > + return dev && kobject_get_unless_zero(>kobj) ? dev : NULL;
> > I really don't want to do this, the bus the device is on should prevent
> > this from happening.
> > 
> > Also, once that reference count drops to zero, the memory should be
> > freed, so you really have a stale pointer here, and this code would fail
> > if you had slab debugging enabled anyway.
> 
> Actually I don't want this either. But the design of scsi core will leave
> the scsi device on the host list after it is deleted, and it can  be
> found later and the refcount have a very big chance to increase from
> zero again. And after a lot of discussion it seems that the scsi layer
> is difficult to change the situation in the near future.

Keeping a 'struct device' reference counted chunk of memory on a list
that has a different lifetime rule from that device itself, is crazy.

And yes, I remember how all of this came about, but I really don't have
the time to work on it myself...

> > So I don't even think this fixes the issue you think it fixes :)
> 
> This issue is very easy to reproduce on my machine and I have tested the
> patch and it really fixes the issue.

Even with slab debugging enabled?  If so, what is keeping that memory
from being freed once the reference count drops to 0?

I think you are just papering over the real issue here, which is one
reason I really do not like the get_unless_zero() function at all.

thanks,

greg k-h


Re: [PATCH] driver core: Make it safe to use get_device() if the reference count is zero

2017-12-13 Thread Greg KH
On Thu, Dec 14, 2017 at 11:39:36AM +0800, Jason Yan wrote:
> Some driviers may have the chance to increase a reference count that
> has dropped to zero when using get_device() because of their design.

Then those drivers are broken :)

> We have met such a issue with scsi:
> https://www.spinics.net/lists/linux-scsi/msg115295.html
> 
> The scsi core will keep the scsi device object in the host list after
> it has been deleted and the iterator can still find it. All of the
> places where need iterating have to check the state of the scsi device
> and this makes a lot of code redundancy and complexity.
> 
> Provide a safe mechanism in get_device() by using
> kobject_get_unless_zero().
> 
> Suggested-by: Bart Van Assche 
> Signed-off-by: Jason Yan 
> CC: Greg Kroah-Hartman 
> CC: Bart Van Assche 
> CC: Ewan D. Milne 
> CC: James E.J. Bottomley 
> CC: Christoph Hellwig 
> ---
>  drivers/base/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 12ebd05..cc74810 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1916,7 +1916,7 @@ EXPORT_SYMBOL_GPL(device_register);
>   */
>  struct device *get_device(struct device *dev)
>  {
> - return dev ? kobj_to_dev(kobject_get(>kobj)) : NULL;
> + return dev && kobject_get_unless_zero(>kobj) ? dev : NULL;

I really don't want to do this, the bus the device is on should prevent
this from happening.

Also, once that reference count drops to zero, the memory should be
freed, so you really have a stale pointer here, and this code would fail
if you had slab debugging enabled anyway.

So I don't even think this fixes the issue you think it fixes :)

thanks,

greg k-h


Re: UFS utilities

2017-11-29 Thread Greg KH
On Wed, Nov 29, 2017 at 03:39:19PM +, Bean Huo (beanhuo) wrote:
> Hi, Greg
> 
> >On Mon, Nov 27, 2017 at 11:25:47AM +, Bean Huo (beanhuo) wrote:
> >> Hi, all
> >> Is there someone knows if exists one utilis dedicated to UFS device, rather
> >than SCSI utils?
> >> I have tried sg3-utils, but it is not convenient for the embedded ARM-based
> >system.
> >> And also it doesn't support several UFS special command.
> >
> >What specific UFS commands do you need to make to the device that the
> >current driver does not support?
> 
> 
> There are some UFS/vendor native commands. They are not SCSI based.

Exactly what ones are missing?  Again, there are numerous non-scsi UFS
commands supported in sysfs files in the in-kernel, and lots in the
different forks I mentioned.  I bet if you pull all of those together,
you should cover all of the ones you need, right?

> >And yes, this is a trick question as there are about 4 different major forks 
> >that
> >I know of of the UFS driver in different vendor trees, all of which support
> >different types of UFS commands :(
> >
> >> If we don't have this kind of tool for UFS, is it necessary for us to 
> >> develop a
> >>ufs-utils?
> >
> >I doubt it, what neds to happen is getting all of the functionality that 
> >lives in
> >these different forks all merged upstream into the in-kernel driver.  Then I 
> >bet
> >all of the needed functionality you are looking for will be there.
> >
> Sometimes customers tend to use user space tool to do some configuration.
> And especially, for example the UFS FFU.

Again, that's fine, have you looked at what is currently present and
what is missing?

thanks,

greg k-h


Re: UFS utilities

2017-11-27 Thread Greg KH
On Mon, Nov 27, 2017 at 11:25:47AM +, Bean Huo (beanhuo) wrote:
> Hi, all 
> Is there someone knows if exists one utilis dedicated to UFS device, rather 
> than SCSI utils?
> I have tried sg3-utils, but it is not convenient for the embedded ARM-based 
> system.
> And also it doesn't support several UFS special command. 

What specific UFS commands do you need to make to the device that the
current driver does not support?

And yes, this is a trick question as there are about 4 different major
forks that I know of of the UFS driver in different vendor trees, all of
which support different types of UFS commands :(

> If we don't have this kind of tool for UFS, is it necessary for us to develop 
> a ufs-utils?

I doubt it, what neds to happen is getting all of the functionality that
lives in these different forks all merged upstream into the in-kernel
driver.  Then I bet all of the needed functionality you are looking for
will be there.

good luck!

greg k-h


Re: [PATCH v3] scsi: require CAP_SYS_ADMIN to write to procfs interface

2017-11-05 Thread Greg KH

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Sun, Nov 05, 2017 at 08:11:20PM +1100, Aleksa Sarai wrote:
> I've booted it on a few of my laptops, and nothing seemed to break. Is
> there a particular test-suite you'd recommend that I run?

Workloads/tools that normally interact with this file would be the best
ones, right?  Odds are, your normal "laptop booting/running" mode does
not do anything with this file.

thanks,

greg k-h


Re: [PATCH v3] scsi: require CAP_SYS_ADMIN to write to procfs interface

2017-11-05 Thread Greg KH
On Sun, Nov 05, 2017 at 01:56:35PM +1100, Aleksa Sarai wrote:
> Previously, the only capability effectively required to operate on the
> /proc/scsi interface was CAP_DAC_OVERRIDE (or for some other files,
> having an fsuid of GLOBAL_ROOT_UID was enough). This means that
> semi-privileged processes could interfere with core components of a
> system (such as causing a DoS by removing the underlying SCSI device of
> the host's / mount).

Given that the previous patch didn't even compile, I worry that you have
not tested this at all to see what breaks/changes in userspace with this
type of user-visable api change.

What did you do to test this?

thanks,

greg k-h


Re: [PATCH v2 11/15] stm class: make config_item_type const

2017-10-17 Thread Greg KH
On Mon, Oct 16, 2017 at 05:18:50PM +0200, Bhumika Goyal wrote:
> Make config_item_type structures const as they are either passed to a
> function having the argument as const or used inside a if statement or
> stored in the const "ci_type" field of a config_item structure.
> 
> Done using Coccinelle.
> 
> Signed-off-by: Bhumika Goyal 
> ---
> * Changes in v2- Combine all the followup patches and the constification
> patches into a series.

Acked-by: Greg Kroah-Hartman 


Re: [PATCH v2 01/15] configfs: make ci_type field, some pointers and function arguments const

2017-10-17 Thread Greg KH
On Mon, Oct 16, 2017 at 05:18:40PM +0200, Bhumika Goyal wrote:
> The ci_type field of the config_item structure do not modify the fields
> of the config_item_type structure it points to. And the other pointers
> initialized with ci_type do not modify the fields as well.
> So, make the ci_type field and the pointers initialized with ci_type
> as const.
> 
> Make the struct config_item_type *type function argument of functions
> config_{item/group}_init_type_name const as the argument in both the
> functions is only stored in the ci_type field of a config_item structure
> which is now made const.
> Make the argument of configfs_register_default_group const as it is
> only passed to the argument of the function config_group_init_type_name
> which is now const.
> 
> Signed-off-by: Bhumika Goyal 
> ---
> * Changes in v2- Combine all the followup patches and the constification
> patches into a series.
> 
>  fs/configfs/dir.c| 10 +-
>  fs/configfs/item.c   |  6 +++---
>  fs/configfs/symlink.c|  4 ++--
>  include/linux/configfs.h |  8 
>  4 files changed, 14 insertions(+), 14 deletions(-)

Acked-by: Greg Kroah-Hartman 


Re: [PATCH v2 00/15] make structure field, function arguments and structures const

2017-10-17 Thread Greg KH
On Tue, Oct 17, 2017 at 12:16:18PM +0200, Julia Lawall wrote:
> 
> 
> On Tue, 17 Oct 2017, Greg KH wrote:
> 
> > On Mon, Oct 16, 2017 at 05:18:39PM +0200, Bhumika Goyal wrote:
> > > Make the ci_type field and some function arguments as const. After this
> > > change, make config_item_type structures as const.
> > >
> > > * Changes in v2- Combine all the followup patches and the constification
> > > patches into a series.
> >
> > Who do you want to take these patches?  If you want, I can take them
> > through my driver-core tree, which has done other configfs stuff like
> > this in the past.
> 
> Christoph Hellwig proposed to take care of it.

Great!  I'll go ack the individual ones that I might need to...

thanks,

greg k-h


Re: [PATCH v2 00/15] make structure field, function arguments and structures const

2017-10-17 Thread Greg KH
On Mon, Oct 16, 2017 at 05:18:39PM +0200, Bhumika Goyal wrote:
> Make the ci_type field and some function arguments as const. After this
> change, make config_item_type structures as const.
> 
> * Changes in v2- Combine all the followup patches and the constification
> patches into a series.

Who do you want to take these patches?  If you want, I can take them
through my driver-core tree, which has done other configfs stuff like
this in the past.

thanks,

greg k-h


Re: [PATCH 00/18] use ARRAY_SIZE macro

2017-10-01 Thread Greg KH
On Sun, Oct 01, 2017 at 08:52:20PM -0400, Jérémy Lefaure wrote:
> On Mon, 2 Oct 2017 09:01:31 +1100
> "Tobin C. Harding"  wrote:
> 
> > > In order to reduce the size of the To: and Cc: lines, each patch of the
> > > series is sent only to the maintainers and lists concerned by the patch.
> > > This cover letter is sent to every list concerned by this series.  
> > 
> > Why don't you just send individual patches for each subsystem? I'm not a 
> > maintainer but I don't see
> > how any one person is going to be able to apply this whole series, it is 
> > making it hard for
> > maintainers if they have to pick patches out from among the series (if 
> > indeed any will bother
> > doing that).
> Yeah, maybe it would have been better to send individual patches.
> 
> From my point of view it's a series because the patches are related (I
> did a git format-patch from my local branch). But for the maintainers
> point of view, they are individual patches.

And the maintainers view is what matters here, if you wish to get your
patches reviewed and accepted...

thanks,

greg k-h


Re: [PATCH] scsi: qla2xxx: Fix an integer overflow in sysfs code

2017-08-30 Thread Greg KH
On Wed, Aug 30, 2017 at 03:21:07PM +0300, Dan Carpenter wrote:
> The value of "size" comes from the user.  When we add "start + size"
> it could lead to an integer overflow bug.
> 
> It means we vmalloc() a lot more memory than we had intended.  I believe
> that on 64 bit systems vmalloc() can succeed even if we ask it to
> allocate huge 4GB buffers.  So we would get memory corruption and likely
> a crash when we call ha->isp_ops->write_optrom() and ->read_optrom().
> 
> Only root can trigger this bug.
> 
> Fixes: b7cc176c9eb3 ("[SCSI] qla2xxx: Allow region-based flash-part 
> accesses.")
> Reported-by: shqking 
> Signed-off-by: Dan Carpenter 


Cc: stable 

perhaps?

thanks,

greg k-h


Re: [PATCH] storvsc: do not assume SG list is continuous when doing bounce buffers (for 4.1 stable only)

2017-08-23 Thread Greg KH
On Tue, Aug 22, 2017 at 11:43:19PM -0700, Christoph Hellwig wrote:
> Ok.  If the stable maintainers are ok with your small fix
> I'm not going to complain too loudly.  But I'm always worried about
> stable trees divering too much from mainline.

Given that 90% of the time we do this, something breaks, you have a
right to be worried...


Re: [PATCH] scsi: sg: Prevent potential double frees in sg driver

2017-08-03 Thread Greg KH
On Thu, Aug 03, 2017 at 12:34:51PM -0700, Nick Desaulniers wrote:
> > Why no one on the to: line?
> 
> I usually cc everyone from get_maintainer.pl. Should I be using
> --to= then explicitly for named folks, and --cc= for lists?

That's usually a good idea, many email clients throw away stuff if there
is nothing on the "To:" line.

> > And do you want this in the stable kernel trees?
> 
> Looks like I can follow up on option #2 once this patch has
> been reviewed+merged by maintainers.  I'll note to use
> option #1 next time, unless you suggest I send a v2? I can
> do so if this patch has a v2+.

If you have to resend it, then add it, otherwise please remember when it
hits Linus's tree to send the git commit id to stable@vger and the
developers there can handle it.

thanks,

greg k-h


Re: [PATCH] scsi: sg: Prevent potential double frees in sg driver

2017-08-03 Thread Greg KH
On Thu, Aug 03, 2017 at 12:02:47PM -0700, Nick Desaulniers wrote:
> From: Robb Glasser 
> 
> sg_ioctl could be spammed by requests, leading to a double free in
> __free_pages. This protects the entry points of sg_ioctl where the
> memory could be corrupted by a double call to __free_pages if multiple
> requests are happening concurrently.
> 
> Signed-off-by: Robb Glasser 
> Signed-off-by: Nick Desaulniers 
> ---
>  drivers/scsi/sg.c | 2 ++
>  1 file changed, 2 insertions(+)

Why no one on the to: line?

And do you want this in the stable kernel trees?  If so, please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

thanks,

greg k-h


[PATCH] SCSI: remove DRIVER_ATTR() usage

2017-07-19 Thread Greg KH
From: Greg Kroah-Hartman 

It's better to use the DRIVER_ATTR_RW() and DRIVER_ATTR_RO() macros to
explicitly show that this is a read/write or read/only sysfs file.  So
convert the remaining SCSI drivers that use the old style to use the
newer macros.

Bonus is that this removes some checkpatch.pl warnings :)

This is part of a series to drop DRIVER_ATTR() from the tree entirely.

Cc: "James E.J. Bottomley" 
Cc: "Martin K. Petersen" 
Cc: Kashyap Desai 
Cc: Sumit Saxena 
Cc: Shivasharan S 
Cc: Willem Riede 
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/scsi/aic94xx/aic94xx_init.c   |4 +--
 drivers/scsi/megaraid/megaraid_sas_base.c |   36 ++
 drivers/scsi/osst.c   |4 +--
 3 files changed, 16 insertions(+), 28 deletions(-)

--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -956,11 +956,11 @@ static int asd_scan_finished(struct Scsi
return 1;
 }
 
-static ssize_t asd_version_show(struct device_driver *driver, char *buf)
+static ssize_t version_show(struct device_driver *driver, char *buf)
 {
return snprintf(buf, PAGE_SIZE, "%s\n", ASD_DRIVER_VERSION);
 }
-static DRIVER_ATTR(version, S_IRUGO, asd_version_show, NULL);
+static DRIVER_ATTR_RO(version);
 
 static int asd_create_driver_attrs(struct device_driver *driver)
 {
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -7323,49 +7323,39 @@ static struct pci_driver megasas_pci_dri
 /*
  * Sysfs driver attributes
  */
-static ssize_t megasas_sysfs_show_version(struct device_driver *dd, char *buf)
+static ssize_t version_show(struct device_driver *dd, char *buf)
 {
return snprintf(buf, strlen(MEGASAS_VERSION) + 2, "%s\n",
MEGASAS_VERSION);
 }
+static DRIVER_ATTR_RO(version);
 
-static DRIVER_ATTR(version, S_IRUGO, megasas_sysfs_show_version, NULL);
-
-static ssize_t
-megasas_sysfs_show_release_date(struct device_driver *dd, char *buf)
+static ssize_t release_date_show(struct device_driver *dd, char *buf)
 {
return snprintf(buf, strlen(MEGASAS_RELDATE) + 2, "%s\n",
MEGASAS_RELDATE);
 }
+static DRIVER_ATTR_RO(release_date);
 
-static DRIVER_ATTR(release_date, S_IRUGO, megasas_sysfs_show_release_date, 
NULL);
-
-static ssize_t
-megasas_sysfs_show_support_poll_for_event(struct device_driver *dd, char *buf)
+static ssize_t support_poll_for_event_show(struct device_driver *dd, char *buf)
 {
return sprintf(buf, "%u\n", support_poll_for_event);
 }
+static DRIVER_ATTR_RO(support_poll_for_event);
 
-static DRIVER_ATTR(support_poll_for_event, S_IRUGO,
-   megasas_sysfs_show_support_poll_for_event, NULL);
-
- static ssize_t
-megasas_sysfs_show_support_device_change(struct device_driver *dd, char *buf)
+static ssize_t support_device_change_show(struct device_driver *dd, char *buf)
 {
return sprintf(buf, "%u\n", support_device_change);
 }
+static DRIVER_ATTR_RO(support_device_change);
 
-static DRIVER_ATTR(support_device_change, S_IRUGO,
-   megasas_sysfs_show_support_device_change, NULL);
-
-static ssize_t
-megasas_sysfs_show_dbg_lvl(struct device_driver *dd, char *buf)
+static ssize_t dbg_lvl_show(struct device_driver *dd, char *buf)
 {
return sprintf(buf, "%u\n", megasas_dbg_lvl);
 }
 
-static ssize_t
-megasas_sysfs_set_dbg_lvl(struct device_driver *dd, const char *buf, size_t 
count)
+static ssize_t dbg_lvl_store(struct device_driver *dd, const char *buf,
+size_t count)
 {
int retval = count;
 
@@ -7375,9 +7365,7 @@ megasas_sysfs_set_dbg_lvl(struct device_
}
return retval;
 }
-
-static DRIVER_ATTR(dbg_lvl, S_IRUGO|S_IWUSR, megasas_sysfs_show_dbg_lvl,
-   megasas_sysfs_set_dbg_lvl);
+static DRIVER_ATTR_RW(dbg_lvl);
 
 static inline void megasas_remove_scsi_device(struct scsi_device *sdev)
 {
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -5667,12 +5667,12 @@ static  struct  osst_support_data support_
  * sysfs support for osst driver parameter information
  */
 
-static ssize_t osst_version_show(struct device_driver *ddd, char *buf)
+static ssize_t version_show(struct device_driver *ddd, char *buf)
 {
return snprintf(buf, PAGE_SIZE, "%s\n", osst_version);
 }
 
-static DRIVER_ATTR(version, S_IRUGO, osst_version_show, NULL);
+static DRIVER_ATTR_RO(version);
 
 static int osst_create_sysfs_files(struct device_driver *sysfs)
 {


Re: [REGRESSION][Stable][v3.12.y][v4.4.y][v4.9.y][v4.10.y][v4.11-rc1] scsi: storvsc: properly set residual data length on errors

2017-03-30 Thread Greg KH
On Tue, Mar 28, 2017 at 04:14:09PM +, Stephen Hemminger wrote:
> I decided not to send it to stable since problem was only observed on
> 4.11 but it is probably endemic to all GEN2 VM's

So, what does this mean?  What should stable@ do?  Nothing?  Ok, now
dropped this from my patch queue :)

thanks,

greg k-h


Re: [PATCH v3 01/16] lpfc: Correct WQ creation for pagesize

2017-03-07 Thread Greg KH
On Tue, Mar 07, 2017 at 10:44:51AM -0300, Mauricio Faria de Oliveira wrote:
> Hello stable kernel maintainers,
> 
> On 03/07/2017 12:35 AM, Martin K. Petersen wrote:
> > Mauricio> Please flag this patch for stable.
> > 
> > Mauricio> This patch resolves a serious problem on IBM Power systems at
> > Mauricio> least.
> > 
> > Both patches are already upstream so I can't tag them for stable. Either
> > you or James should mail sta...@vger.kernel.org and request for the
> > patches to be queued up.
> 
> Can this commit be included on stable v4.4+ , please? (in Linus tree)
> 
> 8ea73db486cda442f0671f4bc9c03a76be398a28 "scsi: lpfc: Correct WQ
> creation for pagesize"

I need an ack from a scsi maintainer that this is an ok thing to do
before I can do so.

thanks,

greg k-h


Re: [PATCH] drivers: block: Remove unnecessary cast

2017-01-11 Thread Greg KH
On Wed, Jan 11, 2017 at 12:41:05PM -0600, Gustavo A. R. Silva wrote:
> This issue was detected using Coccinelle and the following semantic patch:
> 
> @@
> expression * e;
> expression arg1, arg2;
> type T;
> @@
> 
> - e = (T *)
> + e =
> kmalloc(arg1, arg2);
> 
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/block/cciss_scsi.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Why send this to me?
--
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] ibmvscsis: Fix srp_transfer_data fail return code

2017-01-09 Thread Greg KH
On Mon, Jan 09, 2017 at 11:23:23AM -0600, Bryant G. Ly wrote:
> On 1/9/17 10:47 AM, Greg KH wrote:
> 
> > On Mon, Jan 09, 2017 at 10:21:20AM -0600, Bryant G. Ly wrote:
> > > From: "Bryant G. Ly" <b...@us.ibm.com>
> > > 
> > > If srp_transfer_data fails within ibmvscsis_write_pending, then
> > > the most likely scenario is that the client timed out the op and
> > > removed the TCE mapping. Thus it will loop forever retrying the
> > > op that is pretty much guaranteed to fail forever. A better return
> > > code would be EIO instead of EAGAIN.
> > > 
> > > Cc: sta...@vger.kernel.org
> > > Reported-by: Steven Royer <sero...@linux.vnet.ibm.com>
> > > Tested-by: Steven Royer <sero...@linux.vnet.ibm.com>
> > > Signed-off-by: Bryant G. Ly <b...@us.ibm.com>
> > > ---
> > >   drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > Why are you sending this to me?
> > 
> > confused,
> > 
> > greg k-h
> Sorry I meant to put --to target-de...@vger.kernel.org, and just --cc you.

Why even cc: me?

--
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] ibmvscsis: Fix srp_transfer_data fail return code

2017-01-09 Thread Greg KH
On Mon, Jan 09, 2017 at 10:21:20AM -0600, Bryant G. Ly wrote:
> From: "Bryant G. Ly" 
> 
> If srp_transfer_data fails within ibmvscsis_write_pending, then
> the most likely scenario is that the client timed out the op and
> removed the TCE mapping. Thus it will loop forever retrying the
> op that is pretty much guaranteed to fail forever. A better return
> code would be EIO instead of EAGAIN.
> 
> Cc: sta...@vger.kernel.org
> Reported-by: Steven Royer 
> Tested-by: Steven Royer 
> Signed-off-by: Bryant G. Ly 
> ---
>  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Why are you sending this to me?

confused,

greg k-h
--
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: Samsung SSD 1.92TB PM863 Enterprise 2.5" SATA3 errors withc stable 4.4.34

2016-12-14 Thread Greg KH
On Wed, Dec 14, 2016 at 03:07:48PM +0300, Vasiliy Tolstov wrote:
> Hi! I have stable problems with all Samsung SSD drivers like PM863 and
> EVO 850 Pro.
> 
> Time after time scsi bus reset link with messages:
> [ 2477.973617] ata1: exception Emask 0x50 SAct 0x0 SErr 0x4090800
> action 0xe frozen
> [ 2477.975036] ata1: irq_stat 0x00400040, connection status changed
> [ 2477.976396] ata1: SError: { HostInt PHYRdyChg 10B8B DevExch }
> [ 2477.977766] ata1: hard resetting link
> [ 2478.701015] ata1: SATA link down (SStatus 0 SControl 300)
> [ 2483.700924] ata1: hard resetting link
> [ 2484.020924] ata1: SATA link down (SStatus 0 SControl 300)
> [ 2484.022257] ata1: limiting SATA link speed to 1.5 Gbps
> [ 2489.020766] ata1: hard resetting link
> [ 2489.340828] ata1: SATA link down (SStatus 0 SControl 310)
> [ 2489.342158] ata1.00: disabled
> [ 2489.343452] ata1: EH complete
> [ 2489.344806] ata1.00: detaching (SCSI 0:0:0:0)
> [ 2489.347434] sd 0:0:0:0: [sda] Stopping disk
> [ 2489.348605] sd 0:0:0:0: [sda] Start/Stop Unit failed: Result:
> hostbyte=DID_BAD_TARGET driverbyte=DRIVER_OK
> [ 3457.586929] ata1: exception Emask 0x10 SAct 0x0 SErr 0x404
> action 0xe frozen
> [ 3457.588224] ata1: irq_stat 0x0040, connection status changed
> [ 3457.589453] ata1: SError: { CommWake DevExch }
> [ 3457.590679] ata1: hard resetting link
> [ 3458.312616] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
> [ 3461.139831] ata1.00: ATA-9: SAMSUNG MZ7LM1T9HCJM-0E003, GXT3003Q,
> max UDMA/133
> [ 3461.141027] ata1.00: 3750748848 sectors, multi 16: LBA48 NCQ (depth
> 31/32), AA
> [ 3461.142882] ata1.00: configured for UDMA/133
> [ 3461.144004] ata1: EH complete
> [ 3461.145545] scsi 0:0:0:0: Direct-Access ATA SAMSUNG MZ7LM1T9 003Q
> PQ: 0 ANSI: 5
> [ 3461.147069] sd 0:0:0:0: Attached scsi generic sg0 type 0
> [ 3461.147082] sd 0:0:0:0: [sda] 3750748848 512-byte logical blocks:
> (1.92 TB/1.75 TiB)
> [ 3461.147649] sd 0:0:0:0: [sda] Write Protect is off
> [ 3461.147652] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
> [ 3461.147849] sd 0:0:0:0: [sda] Write cache: enabled, read cache:
> enabled, doesn't support DPO or FUA
> [ 3461.152457] sd 0:0:0:0: [sda] Attached SCSI removable disk
> 
> I'm try to remove drive and add it again message not appears may be
> one hour or more. I'm try different servers from HP and Supermicro and
> error is present. Also i'm try various disk from this series and
> nothing changed.
> 
> If i have massive workload like writing to ext4 fs on this ssd drivers
> i get corrupted ext4 journal and readonly fs.
> 
> My kernel version is 4.4.34
> May be some Samsung engineers presented in this mailing list and ca
> help to solve this errors? Or for server i need only Intel SSD (yes if
> i use intel ssd this error not happening, this is not intel
> advertising)

Do you also have problems with this on the 4.9 kernel release?  We can't
add any changes to 4.4 that is not already made in 4.9.

thanks,

greg k-h
--
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] mpt2sas: Fix secure erase premature termination.

2016-11-21 Thread Greg KH
On Mon, Nov 21, 2016 at 03:05:38PM +0530, Suganath Prabu Subramani wrote:
> Commit id and other details are given below:
> 
> commit 18f6084a989ba1b38702f9af37a2e4049a924be6
> Author: Andrey Grodzovsky 
> Date:   Thu Nov 10 09:35:27 2016 -0500
> 
>     scsi: mpt3sas: Fix secure erase premature termination
> 

For what?  I have no context here...


--
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] mpt2sas: Fix secure erase premature termination.

2016-11-18 Thread Greg KH
On Fri, Nov 18, 2016 at 05:12:49PM +0530, Suganath Prabu S wrote:
> Problem:
> This is a work around for a bug with LSI Fusion MPT SAS2 when
> pefroming secure erase. Due to the very long time the operation
> takes commands issued during the erase will time out and will trigger
> execution of abort hook. Even though the abort hook is called for
> the specific command which timed out this leads to entire device halt
> (scsi_state terminated) and premature termination of the secured erase.
> 
> Fix:
> Set device state to busy while erase in progress to reject any incoming
> commands until the erase is done. The device is blocked any way during
> this time and cannot execute any other command.
> 
> P.S
> This is a backport from the same fix for mpt3sas driver intended
> for pre-4.4 stable trees.

What is the git commit id of the patch in Linus's tree that matches up
with this one?

thanks,

greg k-h
--
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] mpt3sas: Fix secure erase premature termination (v3)

2016-11-10 Thread Greg KH
On Thu, Nov 10, 2016 at 08:42:52AM -0500, Andrey Grodzovsky wrote:
> Problem:
> This is a work around for a bug with LSI Fusion MPT SAS2 when
> pefroming secure erase. Due to the very long time the operation
> takes commands issued during the erase will time out and will trigger
> execution of abort hook. Even though the abort hook is called for
> the specifc command which timed out this leads to entire device halt
> (scsi_state terminated) and premature termination of the secured erase.
> 
> Fix:
> Set device state to busy while erase in progress to reject any incoming
> commands until the erase is done. The device is blocked any way during
> this time and cannot execute any other command.
> More data and logs can be found here -
> https://drive.google.com/file/d/0B9ocOHYHbbS1Q3VMdkkzeWFkTjg/view
> 
> v2: Update according to example patch by Hannes Reinecke to apply
> the blocking logic to any ATA 12/16 command.
> 
> v3: Use SCSI commands opcodes definitions instead of value and
> correct identation.
> 
> Signed-off-by: Andrey Grodzovsky 
> Cc: 
> Cc: Sathya Prakash 
> Cc: Chaitra P B 
> Cc: Suganath Prabu Subramani 
> Cc: Sreekanth Reddy 
> Cc: Hannes Reinecke 
> Cc: 
> ---
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 5a97e32..320f16c 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -3500,6 +3500,10 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 
> ioc_status)
>   SAM_STAT_CHECK_CONDITION;
>  }
>  
> +static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
> +{
> +   return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);
> +}

Please always run your patches through checkpatch.pl so you don't get a
grumpy maintainer emailing you and telling you to run your patches
through checkpatch.pl...

--
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] Avoid that SCSI device removal through sysfs triggers a deadlock

2016-11-07 Thread Greg KH
On Mon, Nov 07, 2016 at 04:32:18PM -0800, Bart Van Assche wrote:
> The SCSI core holds scan_mutex around SCSI device addition and
> removal operations. sysfs serializes attribute read and write
> operations against attribute removal through s_active. Avoid that
> grabbing scan_mutex during self-removal of a SCSI device triggers
> a deadlock by not calling __kernfs_remove() from
> kernfs_remove_by_name_ns() in case of self-removal. This patch
> avoids that self-removal triggers the following deadlock:
> 
> ===
> [ INFO: possible circular locking dependency detected ]
> 4.9.0-rc1-dbg+ #4 Not tainted
> ---
> test_02_sdev_de/12586 is trying to acquire lock:
>  (>scan_mutex){+.+.+.}, at: [] 
> scsi_remove_device+0x1e/0x40
> but task is already holding lock:
>  (s_active#336){.+}, at: [] 
> kernfs_remove_self+0xde/0x140
> which lock already depends on the new lock.
> 
> the existing dependency chain (in reverse order) is:
> -> #1 (s_active#336){.+}:
> [] lock_acquire+0xe9/0x1d0
> [] __kernfs_remove+0x24a/0x310
> [] kernfs_remove_by_name_ns+0x40/0x90
> [] remove_files.isra.1+0x30/0x70
> [] sysfs_remove_group+0x3f/0x90
> [] sysfs_remove_groups+0x29/0x40
> [] device_remove_attrs+0x59/0x80
> [] device_del+0x125/0x240
> [] __scsi_remove_device+0x143/0x180
> [] scsi_forget_host+0x64/0x70
> [] scsi_remove_host+0x75/0x120
> [] 0xa035dbbb
> [] process_one_work+0x1f5/0x690
> [] worker_thread+0x49/0x490
> [] kthread+0xeb/0x110
> [] ret_from_fork+0x27/0x40
> 
> -> #0 (>scan_mutex){+.+.+.}:
> [] __lock_acquire+0x10fc/0x1270
> [] lock_acquire+0xe9/0x1d0
> [] mutex_lock_nested+0x5f/0x360
> [] scsi_remove_device+0x1e/0x40
> [] sdev_store_delete+0x22/0x30
> [] dev_attr_store+0x13/0x20
> [] sysfs_kf_write+0x40/0x50
> [] kernfs_fop_write+0x137/0x1c0
> [] __vfs_write+0x23/0x140
> [] vfs_write+0xb0/0x190
> [] SyS_write+0x44/0xa0
> [] entry_SYSCALL_64_fastpath+0x18/0xad
> 
> other info that might help us debug this:
> 
>  Possible unsafe locking scenario:
>CPU0CPU1
>
>   lock(s_active#336);
>lock(>scan_mutex);
>lock(s_active#336);
>   lock(>scan_mutex);
> 
>  *** DEADLOCK ***
> 3 locks held by test_02_sdev_de/12586:
>  #0:  (sb_writers#4){.+.+.+}, at: [] vfs_write+0x178/0x190
>  #1:  (>mutex){+.+.+.}, at: [] 
> kernfs_fop_write+0x101/0x1c0
>  #2:  (s_active#336){.+}, at: [] 
> kernfs_remove_self+0xde/0x140
> 
> stack backtrace:
> CPU: 4 PID: 12586 Comm: test_02_sdev_de Not tainted 4.9.0-rc1-dbg+ #4
> Call Trace:
>  [] dump_stack+0x68/0x93
>  [] print_circular_bug+0x1be/0x210
>  [] __lock_acquire+0x10fc/0x1270
>  [] lock_acquire+0xe9/0x1d0
>  [] mutex_lock_nested+0x5f/0x360
>  [] scsi_remove_device+0x1e/0x40
>  [] sdev_store_delete+0x22/0x30
>  [] dev_attr_store+0x13/0x20
>  [] sysfs_kf_write+0x40/0x50
>  [] kernfs_fop_write+0x137/0x1c0
>  [] __vfs_write+0x23/0x140
>  [] vfs_write+0xb0/0x190
>  [] SyS_write+0x44/0xa0
>  [] entry_SYSCALL_64_fastpath+0x18/0xad
> 
> References: http://www.spinics.net/lists/linux-scsi/msg86551.html
> Signed-off-by: Bart Van Assche 
> Cc: Greg Kroah-Hartman 
> Cc: Eric Biederman 
> Cc: Hannes Reinecke 
> Cc: Johannes Thumshirn 
> Cc: Sagi Grimberg 
> Cc: 
> ---
>  fs/kernfs/dir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index cf4c636..44ec536 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct kernfs_node 
> *parent, const char *name,
>   mutex_lock(_mutex);
>  
>   kn = kernfs_find_ns(parent, name, ns);
> - if (kn)
> + if (kn && !(kn->flags & KERNFS_SUICIDED))
>   __kernfs_remove(kn);

Really?  What changed recently to require this?  I thought we fixed
these issues a long time ago in kernfs :(

thanks,

greg k-h
--
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 v2 6/7] megaraid_sas: Do not set MPI2_TYPE_CUDA for JBOD FP path for FW which does not support JBOD sequence map

2016-10-20 Thread Greg KH
On Thu, Oct 20, 2016 at 02:05:04AM -0700, Sumit Saxena wrote:
> CC: sta...@vger.kernel.org
> Signed-off-by: Sumit Saxena 
> Reviewed-by: Hannes Reinecke 
> Reviewed-by: Tomas Henzl 
> ---
>  drivers/scsi/megaraid/megaraid_sas_fusion.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

I know I reject patches without any changelog text in them, hopefully
the scsi maintainers also do...
--
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 v1 1/3] Synchronization of cmds during termination conditions

2016-10-12 Thread Greg KH
On Tue, Oct 11, 2016 at 05:58:03PM -0500, Michael Cyr wrote:
> Signed-off-by: Michael Cyr 

I almost never accept patches with no changelog information.  For a
patch that changes 1082 lines in a non-trivial way, you would think that
you could explain why you are doing this work...

Also, check the prefix of your subject, it give no clue as to where in
the kernel your patches touch :(

thanks,

greg k-h
--
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: introduce a quirk for false cache reporting

2016-09-09 Thread Greg KH
On Fri, Sep 09, 2016 at 07:26:17AM -0400, Martin K. Petersen wrote:
> > "Oliver" == Oliver Neukum  writes:
> 
> Oliver> On Thu, 2016-08-18 at 22:19 -0400, Martin K. Petersen wrote:
> >> > "Oliver" == Oliver Neukum  writes:
> >> 
> Oliver> Some SATA to USB bridges fail to cooperate with some drives
> Oliver> resulting in no cache being present being reported to the
> Oliver> host. That causes the host to skip sending a command to
> Oliver> synchronize caches. That causes data loss when the drive is
> Oliver> powered down.
> >> 
> >> Reviewed-by: Martin K. Petersen 
> >> 
> 
> Oliver> may I ask about the fate of this patch? Has it been lost?
> Oliver> Should I resubmit? Are any further changes necessary?
> 
> I'm OK with the change but it needs to go through the USB tree.

Ugh, ok, this is gone from my queue, Oliver, can you resend it and I'll
queue it up?

thanks,

greg k-h
--
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: [GIT PULL] [PATCH v4 00/26] Delete CURRENT_TIME and CURRENT_TIME_SEC macros

2016-08-16 Thread Greg KH
On Tue, Aug 16, 2016 at 11:18:52AM -0700, Deepa Dinamani wrote:
> Thank you for the suggestion.
> 
> > Who are you execting to pull this huge patch series?
> 
> The last pull request was addressed to Al as per Arnd's suggestion.
> I'm not completely sure who should it be addressed to.
> 
> > Why not just introduce the new api call, wait for that to be merged, and
> > then push the individual patches through the different subsystems?
> > After half of those get ignored, then provide a single set of patches
> > that can go through Andrew or my trees.
> 
> Arnd and I tried to do this a few ways.
> 
> We can try to introduce the api first like you suggest.
> 
> There are a few Acks already on the patches.
> And, patches 2-5 also need to be merged through some common tree like
> yours or Andrew's as you suggest.
> 
> So, if everyone is ok, I could do the following:
> 
> 1. Post patches 1-5 for rc-2.

-rc2 is already released, and we aren't adding new apis this late in the
release cycle, sorry.

> 2. Post all other patches to respective maintainers after rc-2
> 3. Then after patches get ignored or merged, post remaining as a
> series for you or Andrew to pick up.

The apis need to be aimed for 4.9-rc1, it's too late for 4.8, sorry.

greg k-h
--
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: [GIT PULL] [PATCH v4 00/26] Delete CURRENT_TIME and CURRENT_TIME_SEC macros

2016-08-15 Thread Greg KH
On Sat, Aug 13, 2016 at 03:48:12PM -0700, Deepa Dinamani wrote:
> The series is aimed at getting rid of CURRENT_TIME and CURRENT_TIME_SEC 
> macros.
> The macros are not y2038 safe. There is no plan to transition them into being
> y2038 safe.
> ktime_get_* api's can be used in their place. And, these are y2038 safe.

Who are you execting to pull this huge patch series?

Why not just introduce the new api call, wait for that to be merged, and
then push the individual patches through the different subsystems?
After half of those get ignored, then provide a single set of patches
that can go through Andrew or my trees.

thanks,

greg k-h
--
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 v3 1/2] scsi: fix race between simultaneous decrements of ->host_failed

2016-06-02 Thread Greg KH
On Thu, Jun 02, 2016 at 04:42:37PM +0800, Wei Fang wrote:
> sas_ata_strategy_handler() adds the works of the ata error handler
> to system_unbound_wq. This workqueue asynchronously runs work items,
> so the ata error handler will be performed concurrently on different
> CPUs. In this case, ->host_failed will be decreased simultaneously in
> scsi_eh_finish_cmd() on different CPUs, and become abnormal.
> 
> It will lead to permanently inequal between ->host_failed and
>  ->host_busy, and scsi error handler thread won't become running.
> IO errors after that won't be handled forever.
> 
> Since all scmds must have been handled in the strategy handle, just
> remove the decrement in scsi_eh_finish_cmd() and zero ->host_busy
> after the strategy handle to fix this race.
> 
> This fixes the problem introduced in
> commit 50824d6c5657 ("[SCSI] libsas: async ata-eh").
> 
> Signed-off-by: Wei Fang 




This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.


--
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 v3 2/2] Documentation/scsi: update scsi_eh.txt about ->host_failed

2016-06-02 Thread Greg KH
On Thu, Jun 02, 2016 at 04:42:38PM +0800, Wei Fang wrote:
> Update the new rules of ->host_failed.
> 
> Signed-off-by: Wei Fang 
> ---
>  Documentation/scsi/scsi_eh.txt |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)




This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.


--
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] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-05-24 Thread Greg KH
On Tue, May 24, 2016 at 09:44:43AM -0700, Bart Van Assche wrote:
> On 05/24/2016 09:34 AM, Greg KH wrote:
> > On Tue, May 24, 2016 at 09:25:05AM -0700, Bart Van Assche wrote:
> > > > +static inline long h_send_crq(struct ibmvscsis_adapter *adapter,
> > > > +   u64 word1, u64 word2)
> > > > +{
> > > > +   long rc;
> > > > +   struct vio_dev *vdev = adapter->dma_dev;
> > > > +
> > > > +   pr_debug("ibmvscsis: ibmvscsis_send_crq(0x%x, 0x%016llx, 
> > > > 0x%016llx)\n",
> > > > +   vdev->unit_address, word1, word2);
> > > > +
> > > 
> > > As Joe Perches already asked, please define pr_fmt() instead of including
> > > the kernel module name in every pr_debug() statement.
> > 
> > Even better, as this is a driver, it should be using dev_*() calls
> > instead of pr_*() calls to properly identify the device and driver that
> > is making the message.  No driver should be using pr_*() except in
> > _very_ limited usages.
> 
> Hi Greg,
> 
> The reason I recommended pr_debug() is because ibmvscsis is a LIO target
> driver and I don't think a struct device is associated with a LIO target
> driver. See e.g. target_register_template() in
> drivers/target/target_core_configfs.c.

That seems messed up, there should be a struct device somewhere to
use...
--
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] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-05-24 Thread Greg KH
On Tue, May 24, 2016 at 09:25:05AM -0700, Bart Van Assche wrote:
> > +static inline long h_send_crq(struct ibmvscsis_adapter *adapter,
> > +   u64 word1, u64 word2)
> > +{
> > +   long rc;
> > +   struct vio_dev *vdev = adapter->dma_dev;
> > +
> > +   pr_debug("ibmvscsis: ibmvscsis_send_crq(0x%x, 0x%016llx, 0x%016llx)\n",
> > +   vdev->unit_address, word1, word2);
> > +
> 
> As Joe Perches already asked, please define pr_fmt() instead of including
> the kernel module name in every pr_debug() statement.

Even better, as this is a driver, it should be using dev_*() calls
instead of pr_*() calls to properly identify the device and driver that
is making the message.  No driver should be using pr_*() except in
_very_ limited usages.

thanks,

greg k-h
--
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] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-05-24 Thread Greg KH
On Tue, May 24, 2016 at 08:52:58AM -0500, Bryant G. Ly wrote:
> From: bgly 

Really?  Please go get a proper review from the other internal IBM
developers to fix this, and the other obvious problems with your patch,
before you send it publically and force us to tell you these things...

thanks,

greg k-h
--
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/1] uas: leave can_queue as MAX_CMNDS if device reports larger qdepth

2016-05-23 Thread Greg KH
On Tue, May 24, 2016 at 12:02:43AM +0800, tom.t...@gmail.com wrote:
> From: Tom Yan 
> 
> Commit 198de51dbc34 ("USB: uas: Limit qdepth at the scsi-host level") made
> qdepth limit set in host template (`.can_queue = MAX_CMNDS`) useless.
> 
> Instead of removing the template limit, now we only change limit according
> to the qdepth reported by the device if it is smaller than MAX_CMNDS.
> 
> Signed-off-by: Tom Yan 
> 
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index 4d49fce..d7790e6 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -972,7 +972,8 @@ static int uas_probe(struct usb_interface *intf, const 
> struct usb_device_id *id)
>* 1 tag is reserved for untagged commands +
>* 1 tag to avoid off by one errors in some bridge firmwares
>*/
> - shost->can_queue = devinfo->qdepth - 2;
> + if (devinfo->qdepth - 2 < MAX_CMNDS)
> + shost->can_queue = devinfo->qdepth - 2;

What's wrong with Hans's patch for this issue instead?

--
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 0/4] Decouple X86_32 dependency from the ISA Kconfig option

2016-05-01 Thread Greg KH
On Sun, May 01, 2016 at 12:17:07PM -0400, William Breathitt Gray wrote:
> On Wed, Apr 13, 2016 at 08:18:26AM -0700, Greg KH wrote:
> >On Wed, Apr 13, 2016 at 10:48:42AM -0400, William Breathitt Gray wrote:
> >> On Wed, Apr 13, 2016 at 04:38:38PM +0200, Ingo Molnar wrote:
> >> >Ah, ok, so it's for enabling real hardware, not just a cleanup, right? 
> >> >You might 
> >> >want to put that info into the boilerplate mail or so.
> >> >
> >> >I'm perfectly fine with all the patches that touch x86 code:
> >> >
> >> >  Acked-by: Ingo Molnar <mi...@kernel.org>
> >> >
> >> >I suppose you'd like to have these in the driver tree, all in one place?
> >> >
> >> >Thanks,
> >> >
> >> >  Ingo
> >> 
> >> Ah yes, in retrospect I should have made it clear that this was for
> >> supporting hardware rather than simply code cleanup. That was an
> >> oversight on my part not to have made it more explicit.
> >> 
> >> Introducing everything to the driver tree would be most convenient, thus
> >> allowing me to quickly release my subsequent patches which will be
> >> rebased on top of these.
> >
> >Ok, I can take these.
> >
> >thanks,
> >
> >greg k-h
> 
> Forgive me for probing again; what is that status of this patchset? I'm
> anxious to see them accepted to prevent the regression introduced by the
> ISA_BUS configuration option from appearing in the next merge window.

Not all of these patches had anything to do with the subject of this 0/4
patch, and seem to have gone in through other trees.

So can you resend just the patches that are for this series that have
not been picked up?

thanks,

greg k-h
--
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: raid 5 creation fails on usb3 connected drives kernel 4.4.x, 4.5

2016-04-16 Thread Greg KH
On Sun, Apr 17, 2016 at 02:41:51PM +1000, Brian Chadwick wrote:
> On 17/04/16 02:46, Greg KH wrote:
> > On Sat, Apr 16, 2016 at 09:25:13AM -0700, James Bottomley wrote:
> > > On Sat, 2016-04-16 at 09:08 -0700, Greg KH wrote:
> > > > On Sat, Apr 09, 2016 at 08:18:26PM +1000, Brian Chadwick wrote:
> > > > > On 09/04/16 00:24, Greg KH wrote:
> > > > > > On Fri, Apr 08, 2016 at 07:37:12PM +1000, Brian Chadwick wrote:
> > > > > > > On 08/04/16 01:59, Greg KH wrote:
> > > > > > > > On Fri, Apr 08, 2016 at 01:34:51AM +1000, Brian Chadwick
> > > > > > > > wrote:
> > > > > > > > > On 08/04/16 00:44, Greg KH wrote:
> > > > > > > > > > On Thu, Apr 07, 2016 at 03:04:55PM +1000, Brian Chadwick
> > > > > > > > > > wrote:
> > > > > > > > > > > On 06/04/16 19:53, Greg KH wrote:
> > > > > > > > > > > > On Tue, Apr 05, 2016 at 06:42:51PM +1000, Brian
> > > > > > > > > > > > Chadwick wrote:
> > > > > > > > > > > > > SETUP:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > i7 16GB Computer.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 1 x PCI-x USB3 adaptor card (i think uses xhci
> > > > > > > > > > > > > -hcd)04:00.0 USB controller:
> > > > > > > > > > > > > Renesas Technology Corp. uPD720202 USB 3.0 Host
> > > > > > > > > > > > > Controller (rev 02)
> > > > > > > > > > > > >  Kernel driver in use: xhci_hcd
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 2 x USB3 to dual SATA HDD docks. uses JMicron
> > > > > > > > > > > > > JMS56x Series controllers,
> > > > > > > > > > > > > uses uas.ko kernel module
> > > > > > > > > > > > > Bus 004 Device 002: ID 152d:9561 JMicron Technology
> > > > > > > > > > > > > Corp. / JMicron USA
> > > > > > > > > > > > > Technology Corp.
> > > > > > > > > > > > > Bus 004 Device 003: ID 152d:9561 JMicron Technology
> > > > > > > > > > > > > Corp. / JMicron USA
> > > > > > > > > > > > > Technology Corp.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 3 x Western Digital 320GB 3.5" SATA drives
> > > > > > > > > > > > > 
> > > > > > > > > > > > > USE:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > RAID 5
> > > > > > > > > > > > > 
> > > > > > > > > > > > > KERNEL:
> > > > > > > > > > > > > 4.3.5
> > > > > > > > > > > > > 
> > > > > > > > > > > > > System works perfectly as expected.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Change to kernel 4.4.x or 4.5 and RAID 5 doesnt
> > > > > > > > > > > > > work. I am not sure whether
> > > > > > > > > > > > > its actually RAID 5 at fault or if its the USB
> > > > > > > > > > > > > Attached SCSI driver uas.ko,
> > > > > > > > > > > > > or maybe the host driver xhci-hcd.
> > > > > > > > > > > > Can you run 'git bisect' to try to track down the
> > > > > > > > > > > > offending commit?
> > > > 
> > > > 
> > > > > Hi, well to my surprise git bisect has come up with a commit about
> > > > > which I
> > > > > had no inkling. I may have to go to another mailing list it doesnt
> > > > > look like
> > > > > a usb problem.
> > > > > 
> > > > > This is the fi

Re: raid 5 creation fails on usb3 connected drives kernel 4.4.x, 4.5

2016-04-16 Thread Greg KH
On Sat, Apr 16, 2016 at 09:25:13AM -0700, James Bottomley wrote:
> On Sat, 2016-04-16 at 09:08 -0700, Greg KH wrote:
> > On Sat, Apr 09, 2016 at 08:18:26PM +1000, Brian Chadwick wrote:
> > > On 09/04/16 00:24, Greg KH wrote:
> > > > On Fri, Apr 08, 2016 at 07:37:12PM +1000, Brian Chadwick wrote:
> > > > > On 08/04/16 01:59, Greg KH wrote:
> > > > > > On Fri, Apr 08, 2016 at 01:34:51AM +1000, Brian Chadwick
> > > > > > wrote:
> > > > > > > On 08/04/16 00:44, Greg KH wrote:
> > > > > > > > On Thu, Apr 07, 2016 at 03:04:55PM +1000, Brian Chadwick
> > > > > > > > wrote:
> > > > > > > > > On 06/04/16 19:53, Greg KH wrote:
> > > > > > > > > > On Tue, Apr 05, 2016 at 06:42:51PM +1000, Brian
> > > > > > > > > > Chadwick wrote:
> > > > > > > > > > > SETUP:
> > > > > > > > > > > 
> > > > > > > > > > > i7 16GB Computer.
> > > > > > > > > > > 
> > > > > > > > > > > 1 x PCI-x USB3 adaptor card (i think uses xhci
> > > > > > > > > > > -hcd)04:00.0 USB controller:
> > > > > > > > > > > Renesas Technology Corp. uPD720202 USB 3.0 Host
> > > > > > > > > > > Controller (rev 02)
> > > > > > > > > > > Kernel driver in use: xhci_hcd
> > > > > > > > > > > 
> > > > > > > > > > > 2 x USB3 to dual SATA HDD docks. uses JMicron
> > > > > > > > > > > JMS56x Series controllers,
> > > > > > > > > > > uses uas.ko kernel module
> > > > > > > > > > > Bus 004 Device 002: ID 152d:9561 JMicron Technology
> > > > > > > > > > > Corp. / JMicron USA
> > > > > > > > > > > Technology Corp.
> > > > > > > > > > > Bus 004 Device 003: ID 152d:9561 JMicron Technology
> > > > > > > > > > > Corp. / JMicron USA
> > > > > > > > > > > Technology Corp.
> > > > > > > > > > > 
> > > > > > > > > > > 3 x Western Digital 320GB 3.5" SATA drives
> > > > > > > > > > > 
> > > > > > > > > > > USE:
> > > > > > > > > > > 
> > > > > > > > > > > RAID 5
> > > > > > > > > > > 
> > > > > > > > > > > KERNEL:
> > > > > > > > > > > 4.3.5
> > > > > > > > > > > 
> > > > > > > > > > > System works perfectly as expected.
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > Change to kernel 4.4.x or 4.5 and RAID 5 doesnt
> > > > > > > > > > > work. I am not sure whether
> > > > > > > > > > > its actually RAID 5 at fault or if its the USB
> > > > > > > > > > > Attached SCSI driver uas.ko,
> > > > > > > > > > > or maybe the host driver xhci-hcd.
> > > > > > > > > > Can you run 'git bisect' to try to track down the
> > > > > > > > > > offending commit?
> > 
> > 
> > 
> > > Hi, well to my surprise git bisect has come up with a commit about
> > > which I
> > > had no inkling. I may have to go to another mailing list it doesnt
> > > look like
> > > a usb problem.
> > > 
> > > This is the final output of 'git bisect view' :
> > > 
> > > 
> > > commit 64d513ac31bd02a3c9b69ef0f36c196f9a9d
> > > Author: Christoph Hellwig <h...@lst.de>
> > > Date:   Thu Oct 8 09:28:04 2015 +0100
> > > 
> > > scsi: use host wide tags by default
> > > 
> > > This patch changes the !blk-mq path to the same defaults as the
> > > blk-mq
> > > I/O path by always enabling block tagging, and always using
> > > host wide
> > > tags.  We've had blk-mq available for a few releases so bugs
> > > with
> > > this mode should have been ironed out, and this ensures we get
> > > better
> > > coverage of over tagging setup over different configs.
> > > 
> > > Signed-off-by: Christoph Hellwig <h...@lst.de>
> > > Acked-by: Jens Axboe <ax...@kernel.dk>
> > > Reviewed-by: Hannes Reinecke <h...@suse.de>
> > > Signed-off-by: James Bottomley <jbottom...@odin.com>
> > > 
> > > 
> > > 
> > > Thank you for your help in narrowing this down, and in educating me
> > > about
> > > git bisect (its a neat tool) ... I await your advice as how to
> > > proceed from
> > > here.
> > 
> > Sorry for the delay, nice work tracking down the problem.
> > 
> > Christoph, any ideas?  This patch is breaking this user's system as
> > described above.  Is there a more recent fix for this, or did
> > something
> > forget to get changed in the large patch you did here?
> 
> If this is UAS connected devices, then this is likely the fix:
> 
> http://marc.info/?l=linux-scsi=146045685829613

Ah, nice, it's planned to go to Linus later today...

Brian, can you test that patch out to see if it resolves your issue or
not?

thanks,

greg k-h
--
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: raid 5 creation fails on usb3 connected drives kernel 4.4.x, 4.5

2016-04-16 Thread Greg KH
On Sat, Apr 09, 2016 at 08:18:26PM +1000, Brian Chadwick wrote:
> On 09/04/16 00:24, Greg KH wrote:
> > On Fri, Apr 08, 2016 at 07:37:12PM +1000, Brian Chadwick wrote:
> > > On 08/04/16 01:59, Greg KH wrote:
> > > > On Fri, Apr 08, 2016 at 01:34:51AM +1000, Brian Chadwick wrote:
> > > > > On 08/04/16 00:44, Greg KH wrote:
> > > > > > On Thu, Apr 07, 2016 at 03:04:55PM +1000, Brian Chadwick wrote:
> > > > > > > On 06/04/16 19:53, Greg KH wrote:
> > > > > > > > On Tue, Apr 05, 2016 at 06:42:51PM +1000, Brian Chadwick wrote:
> > > > > > > > > SETUP:
> > > > > > > > > 
> > > > > > > > > i7 16GB Computer.
> > > > > > > > > 
> > > > > > > > > 1 x PCI-x USB3 adaptor card (i think uses xhci-hcd)04:00.0 
> > > > > > > > > USB controller:
> > > > > > > > > Renesas Technology Corp. uPD720202 USB 3.0 Host Controller 
> > > > > > > > > (rev 02)
> > > > > > > > > Kernel driver in use: xhci_hcd
> > > > > > > > > 
> > > > > > > > > 2 x USB3 to dual SATA HDD docks. uses JMicron JMS56x Series 
> > > > > > > > > controllers,
> > > > > > > > > uses uas.ko kernel module
> > > > > > > > > Bus 004 Device 002: ID 152d:9561 JMicron Technology Corp. / 
> > > > > > > > > JMicron USA
> > > > > > > > > Technology Corp.
> > > > > > > > > Bus 004 Device 003: ID 152d:9561 JMicron Technology Corp. / 
> > > > > > > > > JMicron USA
> > > > > > > > > Technology Corp.
> > > > > > > > > 
> > > > > > > > > 3 x Western Digital 320GB 3.5" SATA drives
> > > > > > > > > 
> > > > > > > > > USE:
> > > > > > > > > 
> > > > > > > > > RAID 5
> > > > > > > > > 
> > > > > > > > > KERNEL:
> > > > > > > > > 4.3.5
> > > > > > > > > 
> > > > > > > > > System works perfectly as expected.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Change to kernel 4.4.x or 4.5 and RAID 5 doesnt work. I am 
> > > > > > > > > not sure whether
> > > > > > > > > its actually RAID 5 at fault or if its the USB Attached SCSI 
> > > > > > > > > driver uas.ko,
> > > > > > > > > or maybe the host driver xhci-hcd.
> > > > > > > > Can you run 'git bisect' to try to track down the offending 
> > > > > > > > commit?



> Hi, well to my surprise git bisect has come up with a commit about which I
> had no inkling. I may have to go to another mailing list it doesnt look like
> a usb problem.
> 
> This is the final output of 'git bisect view' :
> 
> 
> commit 64d513ac31bd02a3c9b69ef0f36c196f9a9d
> Author: Christoph Hellwig <h...@lst.de>
> Date:   Thu Oct 8 09:28:04 2015 +0100
> 
> scsi: use host wide tags by default
> 
> This patch changes the !blk-mq path to the same defaults as the blk-mq
> I/O path by always enabling block tagging, and always using host wide
> tags.  We've had blk-mq available for a few releases so bugs with
> this mode should have been ironed out, and this ensures we get better
> coverage of over tagging setup over different configs.
> 
> Signed-off-by: Christoph Hellwig <h...@lst.de>
> Acked-by: Jens Axboe <ax...@kernel.dk>
> Reviewed-by: Hannes Reinecke <h...@suse.de>
> Signed-off-by: James Bottomley <jbottom...@odin.com>
> 
> 
> 
> Thank you for your help in narrowing this down, and in educating me about
> git bisect (its a neat tool) ... I await your advice as how to proceed from
> here.

Sorry for the delay, nice work tracking down the problem.

Christoph, any ideas?  This patch is breaking this user's system as
described above.  Is there a more recent fix for this, or did something
forget to get changed in the large patch you did here?

thanks,

greg k-h
--
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 0/4] Decouple X86_32 dependency from the ISA Kconfig option

2016-04-13 Thread Greg KH
On Wed, Apr 13, 2016 at 10:48:42AM -0400, William Breathitt Gray wrote:
> On Wed, Apr 13, 2016 at 04:38:38PM +0200, Ingo Molnar wrote:
> >Ah, ok, so it's for enabling real hardware, not just a cleanup, right? You 
> >might 
> >want to put that info into the boilerplate mail or so.
> >
> >I'm perfectly fine with all the patches that touch x86 code:
> >
> >  Acked-by: Ingo Molnar 
> >
> >I suppose you'd like to have these in the driver tree, all in one place?
> >
> >Thanks,
> >
> > Ingo
> 
> Ah yes, in retrospect I should have made it clear that this was for
> supporting hardware rather than simply code cleanup. That was an
> oversight on my part not to have made it more explicit.
> 
> Introducing everything to the driver tree would be most convenient, thus
> allowing me to quickly release my subsequent patches which will be
> rebased on top of these.

Ok, I can take these.

thanks,

greg k-h
--
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] csiostor: Fix backwards locking in the function __csio_unreg_rnode

2016-04-06 Thread Greg KH
On Wed, Apr 06, 2016 at 01:28:24PM -0400, James Bottomley wrote:
> On Wed, 2016-04-06 at 13:23 -0400, Bastien Philbert wrote:
> > 
> > On 2016-04-06 01:14 PM, James Bottomley wrote:
> > > On Wed, 2016-04-06 at 10:36 -0400, Bastien Philbert wrote:
> > > > 
> > > > On 2016-04-06 10:24 AM, James Bottomley wrote:
> > > > > On Wed, 2016-04-06 at 10:11 -0400, Bastien Philbert wrote:
> > > > > > 
> > > > > > On 2016-04-06 09:38 AM, James Bottomley wrote:
> > > > > > > On Wed, 2016-04-06 at 09:21 -0400, Bastien Philbert wrote:
> > > > > > > > 
> > > > > > > > On 2016-04-06 03:48 AM, Julian Calaby wrote:
> > > > > > > > > Hi Bastien,
> > > > > > > > > 
> > > > > > > > > On Wed, Apr 6, 2016 at 7:19 AM, Bastien Philbert
> > > > > > > > >  wrote:
> > > > > > > > > > This fixes backwards locking in the function
> > > > > > > > > > __csio_unreg_rnode
> > > > > > > > > > to
> > > > > > > > > > properly lock before the call to the function
> > > > > > > > > > csio_unreg_rnode
> > > > > > > > > > and
> > > > > > > > > > not unlock with spin_unlock_irq as this would not
> > > > > > > > > > allow
> > > > > > > > > > the
> > > > > > > > > > proper
> > > > > > > > > > protection for concurrent access on the shared
> > > > > > > > > > csio_hw
> > > > > > > > > > structure
> > > > > > > > > > pointer hw. In addition switch the locking after the
> > > > > > > > > > critical
> > > > > > > > > > region
> > > > > > > > > > function call to properly unlock instead with
> > > > > > > > > > spin_unlock_irq
> > > > > > > > > > on
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Bastien Philbert <
> > > > > > > > > > bastienphilb...@gmail.com>
> > > > > > > > > > ---
> > > > > > > > > >  drivers/scsi/csiostor/csio_rnode.c | 4 ++--
> > > > > > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/drivers/scsi/csiostor/csio_rnode.c
> > > > > > > > > > b/drivers/scsi/csiostor/csio_rnode.c
> > > > > > > > > > index e9c3b04..029a09e 100644
> > > > > > > > > > --- a/drivers/scsi/csiostor/csio_rnode.c
> > > > > > > > > > +++ b/drivers/scsi/csiostor/csio_rnode.c
> > > > > > > > > > @@ -580,9 +580,9 @@ __csio_unreg_rnode(struct
> > > > > > > > > > csio_rnode
> > > > > > > > > > *rn)
> > > > > > > > > > ln->last_scan_ntgts--;
> > > > > > > > > > }
> > > > > > > > > > 
> > > > > > > > > > -   spin_unlock_irq(>lock);
> > > > > > > > > > -   csio_unreg_rnode(rn);
> > > > > > > > > > spin_lock_irq(>lock);
> > > > > > > > > > +   csio_unreg_rnode(rn);
> > > > > > > > > > +   spin_unlock_irq(>lock);
> > > > > > > > > 
> > > > > > > > > Are you _certain_ this is correct? This construct
> > > > > > > > > usually
> > > > > > > > > appears
> > > > > > > > > when
> > > > > > > > > a function has a particular lock held, then needs to
> > > > > > > > > unlock
> > > > > > > > > it
> > > > > > > > > to
> > > > > > > > > call
> > > > > > > > > some other function. Are you _certain_ that this isn't
> > > > > > > > > the
> > > > > > > > > case?
> > > > > > > > > 
> > > > > > > > > Thanks,
> > > > > > > > > 
> > > > > > > > Yes I am pretty certain this is correct. I checked the
> > > > > > > > paths
> > > > > > > > that
> > > > > > > > called this function
> > > > > > > > and it was weired that none of them gradded the spinlock
> > > > > > > > before
> > > > > > > > hand.
> > > > > > > 
> > > > > > > That's not good enough.  If your theory is correct, lockdep
> > > > > > > should
> > > > > > > be
> > > > > > > dropping an already unlocked assertion in this codepath ...
> > > > > > > do
> > > > > > > you
> > > > > > > see
> > > > > > > this?
> > > > > > > 
> > > > > > > James
> > > > > > > 
> > > > > > > 
> > > > > > Yes I do.
> > > > > 
> > > > > You mean you don't see the lockdep assert, since you're asking
> > > > > to 
> > > > > drop the patch?
> > > > > 
> > > > > >  For now just drop the patch but I am still concerned that we
> > > > > > are
> > > > > > double unlocking here.
> > > > > 
> > > > > Really, no.  The pattern in the code indicates the lock is
> > > > > expected 
> > > > > to be held.  This can be wrong (sometimes code moves or people
> > > > > forget), but if it is wrong we'll get an assert about unlock of
> > > > > an 
> > > > > already unlocked lock.  If there's no assert, the lock is held
> > > > > on 
> > > > > entry and the code is correct.
> > > > > 
> > > > > You're proposing patches based on misunderstandings of the code
> > > > > which aren't backed up by actual issues and wasting everyone's
> > > > > time 
> > > > > to look at them.  Please begin with the hard evidence of a
> > > > > problem 
> > > > > first, so post the lockdep assert in the changelog so we know 
> > > > > there's a real problem.
> > > > > 
> > > > > James
> > > > > 
> > > > Certainly James. I think I just got carried away with the last
> > > > few 
> > > > patches :(.
> > > 
> > > Is this Nick Krause?  An email reply that Martin 

Re: [PATCH v7] scsi: ufs: add ioctl interface for query request

2016-03-10 Thread Greg KH
On Thu, Mar 10, 2016 at 06:48:54PM -, yga...@codeaurora.org wrote:
> > On Thu, Mar 10, 2016 at 04:29:55PM -, yga...@codeaurora.org wrote:
> >> > On Thu, Mar 10, 2016 at 03:52:54PM -, yga...@codeaurora.org wrote:
> >> >> > On Wed, Mar 09, 2016 at 08:52:59PM -, yga...@codeaurora.org
> >> wrote:
> >> >> >> > On Wed, Mar 09, 2016 at 07:09:49PM -, yga...@codeaurora.org
> >> >> wrote:
> >> >> >> >> > On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote:
> >> >> >> >> >> This patch exposes the ioctl interface for UFS driver via
> >> SCSI
> >> >> >> device
> >> >> >> >> >> ioctl interface. As of now UFS driver would provide the
> >> ioctl
> >> >> for
> >> >> >> >> query
> >> >> >> >> >> interface to connected UFS device.
> >> >> >> >> >>
> >> >> >> >> >> Reviewed-by: Subhash Jadavani 
> >> >> >> >> >> Signed-off-by: Dolev Raviv 
> >> >> >> >> >> Signed-off-by: Gilad Broner 
> >> >> >> >> >> Signed-off-by: Yaniv Gardi 
> >> >> >> >> >
> >> >> >> >> > What tool is going to use this ioctl?  Why does userspcae
> >> want
> >> >> to
> >> >> >> do
> >> >> >> >> > something "special" with UFS devices?  Shouldn't they just be
> >> >> >> treated
> >> >> >> >> > like any other normal block device?
> >> >> >> >> >
> >> >> >> >>
> >> >> >> >> Any userspace application can be a tool.
> >> >> >> >> We already implemented and used a user space application, that
> >> >> sent
> >> >> >> >> queries to the UFS devices in order to get information and
> >> >> >> descriptors.
> >> >> >> >
> >> >> >> > But do you want to do with that information?  Why does userspace
> >> >> care?
> >> >> >> >
> >> >> >>
> >> >> >> i don't really understand the subtext of your question -
> >> >> >> as ANY ioctl cb, we decided to implement the ioctl callback of
> >> this
> >> >> scsi
> >> >> >> device in order to get information like UNIT DESC, DEVICE DESC,
> >> >> FLAGs,
> >> >> >> ATTRIBUTES.
> >> >> >> When dealing with UFS devices, one should be able to read the
> >> >> >> characteristics of the device. why ? well, why not ?
> >> >> >
> >> >> > Why aren't those characteristics just exported as sysfs attributes
> >> >> under
> >> >> > control by the UFS controller driver?  Why do you need/want an
> >> ioctl
> >> >> for
> >> >> > this?
> >> >> >
> >> >>
> >> >> Hi greg k-h,
> >> >>
> >> >> in our code, we used the IOCTL during runtime, in order to determine
> >> >> some
> >> >> information about the RPMB well known lun.
> >> >> with the rpmb lun ID we could then go to /dev/sgX and issue
> >> >> UFS_IOCTL_QUERY to this lun and get the data -
> >> >> reading the QUERY_DESC_IDN_GEOMETRY descriptor and reading the
> >> >> QUERY_DESC_IDN_UNIT descriptor.
> >> >>
> >> >> this was crucial to the work we do in RPMB.
> >> >
> >> > What is RPMB?
> >> >
> >> > And again, why not just use sysfs attributes on your host controller
> >> > device?  Why does this have to be a custom ioctl?
> >> >
> >> > greg k-h
> >> >
> >>
> >> RPMB is spcial Logical Unit in the UFS device. you can read about it in
> >> the UFS spec. Greg, you are insisting on sysfs, but i can't implement it
> >> now, as i don't have the Hardware anymore, or the time.
> >
> > If you don't have the hardware, how are you testing this patch?
> 
> This patch is already tested and verified, and also was much helpful
> during development.
> Also, this patch is a few months old and was tested when previous version
> were uploaded.
> Currently HW is not available.
> >
> > And if you don't have the hardware I guess you don't need this change :)
> >
> >> This is a tested and verified code that was accepted and reviewed
> >> already,
> >> so i'm not sure what is wrong with this solution, not to say, it's
> >> already
> >> implemented, tested and verified.
> >> hope you are help us push it.
> >
> > That's a horrible reason to merge a patch that someone else is going to
> > have to support for 20+ years with an api that doesn't make much sense.
> >
> > If you don't have the hardware, then this isn't needed.  But if you do,
> 
> Even if currently HW is not available it doesn't mean this is not needed
> in the future again.

Great, if in the future, you need this again, please resubmit it.
Adding code to the kernel for no real user is a maintaince burden on
others.  Please don't do that.  Especially for an API that is now
required to be supported for forever.

> > then please look into using sysfs for this, as I think that should be
> > the correct interface here, again, not some random ioctl.
> >
> 
> Why sysfs makes more sense than this one ?

Why doesn't it?  Why would an ioctl make sense to get simple attributes
from a host controller?  Why not use the interface that makes it trivial
to use from userspace that other drivers already use in this type of
situation.

In other words, why do you have to have this ioctl?  What requires this
to be the way the API works?

But again, as you don't need 

Re: [PATCH v7] scsi: ufs: add ioctl interface for query request

2016-03-10 Thread Greg KH
On Thu, Mar 10, 2016 at 04:29:55PM -, yga...@codeaurora.org wrote:
> > On Thu, Mar 10, 2016 at 03:52:54PM -, yga...@codeaurora.org wrote:
> >> > On Wed, Mar 09, 2016 at 08:52:59PM -, yga...@codeaurora.org wrote:
> >> >> > On Wed, Mar 09, 2016 at 07:09:49PM -, yga...@codeaurora.org
> >> wrote:
> >> >> >> > On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote:
> >> >> >> >> This patch exposes the ioctl interface for UFS driver via SCSI
> >> >> device
> >> >> >> >> ioctl interface. As of now UFS driver would provide the ioctl
> >> for
> >> >> >> query
> >> >> >> >> interface to connected UFS device.
> >> >> >> >>
> >> >> >> >> Reviewed-by: Subhash Jadavani 
> >> >> >> >> Signed-off-by: Dolev Raviv 
> >> >> >> >> Signed-off-by: Gilad Broner 
> >> >> >> >> Signed-off-by: Yaniv Gardi 
> >> >> >> >
> >> >> >> > What tool is going to use this ioctl?  Why does userspcae want
> >> to
> >> >> do
> >> >> >> > something "special" with UFS devices?  Shouldn't they just be
> >> >> treated
> >> >> >> > like any other normal block device?
> >> >> >> >
> >> >> >>
> >> >> >> Any userspace application can be a tool.
> >> >> >> We already implemented and used a user space application, that
> >> sent
> >> >> >> queries to the UFS devices in order to get information and
> >> >> descriptors.
> >> >> >
> >> >> > But do you want to do with that information?  Why does userspace
> >> care?
> >> >> >
> >> >>
> >> >> i don't really understand the subtext of your question -
> >> >> as ANY ioctl cb, we decided to implement the ioctl callback of this
> >> scsi
> >> >> device in order to get information like UNIT DESC, DEVICE DESC,
> >> FLAGs,
> >> >> ATTRIBUTES.
> >> >> When dealing with UFS devices, one should be able to read the
> >> >> characteristics of the device. why ? well, why not ?
> >> >
> >> > Why aren't those characteristics just exported as sysfs attributes
> >> under
> >> > control by the UFS controller driver?  Why do you need/want an ioctl
> >> for
> >> > this?
> >> >
> >>
> >> Hi greg k-h,
> >>
> >> in our code, we used the IOCTL during runtime, in order to determine
> >> some
> >> information about the RPMB well known lun.
> >> with the rpmb lun ID we could then go to /dev/sgX and issue
> >> UFS_IOCTL_QUERY to this lun and get the data -
> >> reading the QUERY_DESC_IDN_GEOMETRY descriptor and reading the
> >> QUERY_DESC_IDN_UNIT descriptor.
> >>
> >> this was crucial to the work we do in RPMB.
> >
> > What is RPMB?
> >
> > And again, why not just use sysfs attributes on your host controller
> > device?  Why does this have to be a custom ioctl?
> >
> > greg k-h
> >
> 
> RPMB is spcial Logical Unit in the UFS device. you can read about it in
> the UFS spec. Greg, you are insisting on sysfs, but i can't implement it
> now, as i don't have the Hardware anymore, or the time.

If you don't have the hardware, how are you testing this patch?

And if you don't have the hardware I guess you don't need this change :)

> This is a tested and verified code that was accepted and reviewed already,
> so i'm not sure what is wrong with this solution, not to say, it's already
> implemented, tested and verified.
> hope you are help us push it.

That's a horrible reason to merge a patch that someone else is going to
have to support for 20+ years with an api that doesn't make much sense.

If you don't have the hardware, then this isn't needed.  But if you do,
then please look into using sysfs for this, as I think that should be
the correct interface here, again, not some random ioctl.

greg k-h
--
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 v7] scsi: ufs: add ioctl interface for query request

2016-03-10 Thread Greg KH
On Thu, Mar 10, 2016 at 03:52:54PM -, yga...@codeaurora.org wrote:
> > On Wed, Mar 09, 2016 at 08:52:59PM -, yga...@codeaurora.org wrote:
> >> > On Wed, Mar 09, 2016 at 07:09:49PM -, yga...@codeaurora.org wrote:
> >> >> > On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote:
> >> >> >> This patch exposes the ioctl interface for UFS driver via SCSI
> >> device
> >> >> >> ioctl interface. As of now UFS driver would provide the ioctl for
> >> >> query
> >> >> >> interface to connected UFS device.
> >> >> >>
> >> >> >> Reviewed-by: Subhash Jadavani 
> >> >> >> Signed-off-by: Dolev Raviv 
> >> >> >> Signed-off-by: Gilad Broner 
> >> >> >> Signed-off-by: Yaniv Gardi 
> >> >> >
> >> >> > What tool is going to use this ioctl?  Why does userspcae want to
> >> do
> >> >> > something "special" with UFS devices?  Shouldn't they just be
> >> treated
> >> >> > like any other normal block device?
> >> >> >
> >> >>
> >> >> Any userspace application can be a tool.
> >> >> We already implemented and used a user space application, that sent
> >> >> queries to the UFS devices in order to get information and
> >> descriptors.
> >> >
> >> > But do you want to do with that information?  Why does userspace care?
> >> >
> >>
> >> i don't really understand the subtext of your question -
> >> as ANY ioctl cb, we decided to implement the ioctl callback of this scsi
> >> device in order to get information like UNIT DESC, DEVICE DESC, FLAGs,
> >> ATTRIBUTES.
> >> When dealing with UFS devices, one should be able to read the
> >> characteristics of the device. why ? well, why not ?
> >
> > Why aren't those characteristics just exported as sysfs attributes under
> > control by the UFS controller driver?  Why do you need/want an ioctl for
> > this?
> >
> 
> Hi greg k-h,
> 
> in our code, we used the IOCTL during runtime, in order to determine some
> information about the RPMB well known lun.
> with the rpmb lun ID we could then go to /dev/sgX and issue
> UFS_IOCTL_QUERY to this lun and get the data -
> reading the QUERY_DESC_IDN_GEOMETRY descriptor and reading the
> QUERY_DESC_IDN_UNIT descriptor.
> 
> this was crucial to the work we do in RPMB.

What is RPMB?

And again, why not just use sysfs attributes on your host controller
device?  Why does this have to be a custom ioctl?

greg k-h
--
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 v7] scsi: ufs: add ioctl interface for query request

2016-03-09 Thread Greg KH
On Wed, Mar 09, 2016 at 08:52:59PM -, yga...@codeaurora.org wrote:
> > On Wed, Mar 09, 2016 at 07:09:49PM -, yga...@codeaurora.org wrote:
> >> > On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote:
> >> >> This patch exposes the ioctl interface for UFS driver via SCSI device
> >> >> ioctl interface. As of now UFS driver would provide the ioctl for
> >> query
> >> >> interface to connected UFS device.
> >> >>
> >> >> Reviewed-by: Subhash Jadavani 
> >> >> Signed-off-by: Dolev Raviv 
> >> >> Signed-off-by: Gilad Broner 
> >> >> Signed-off-by: Yaniv Gardi 
> >> >
> >> > What tool is going to use this ioctl?  Why does userspcae want to do
> >> > something "special" with UFS devices?  Shouldn't they just be treated
> >> > like any other normal block device?
> >> >
> >>
> >> Any userspace application can be a tool.
> >> We already implemented and used a user space application, that sent
> >> queries to the UFS devices in order to get information and descriptors.
> >
> > But do you want to do with that information?  Why does userspace care?
> >
> 
> i don't really understand the subtext of your question -
> as ANY ioctl cb, we decided to implement the ioctl callback of this scsi
> device in order to get information like UNIT DESC, DEVICE DESC, FLAGs,
> ATTRIBUTES.
> When dealing with UFS devices, one should be able to read the
> characteristics of the device. why ? well, why not ?

Why aren't those characteristics just exported as sysfs attributes under
control by the UFS controller driver?  Why do you need/want an ioctl for
this?

> during development of this driver, it was useful in many cases to be able
> to communicate with the device, by simple IOCTL command, rather than
> implementing ad-hock.

Do other storage busses have these types of "custom" ioctls for their
bus-type alone?  For simple attributes like this, shouldn't you be using
sysfs instead so that it is much easier for userspace tools to get
access to them?

thanks,

greg k-h
--
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 v7] scsi: ufs: add ioctl interface for query request

2016-03-09 Thread Greg KH
On Wed, Mar 09, 2016 at 07:09:49PM -, yga...@codeaurora.org wrote:
> > On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote:
> >> This patch exposes the ioctl interface for UFS driver via SCSI device
> >> ioctl interface. As of now UFS driver would provide the ioctl for query
> >> interface to connected UFS device.
> >>
> >> Reviewed-by: Subhash Jadavani 
> >> Signed-off-by: Dolev Raviv 
> >> Signed-off-by: Gilad Broner 
> >> Signed-off-by: Yaniv Gardi 
> >
> > What tool is going to use this ioctl?  Why does userspcae want to do
> > something "special" with UFS devices?  Shouldn't they just be treated
> > like any other normal block device?
> >
> 
> Any userspace application can be a tool.
> We already implemented and used a user space application, that sent
> queries to the UFS devices in order to get information and descriptors.

But do you want to do with that information?  Why does userspace care?

> Not only ioctl interface is a useful way to interact with the device,
> we used it, and found it very helpful in varies cases.

In what case was it helpful?  Why does userspace care about ufs
specifics, it should just treat it like any other block device and not
care at all.

thanks,

greg k-h
--
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 v7] scsi: ufs: add ioctl interface for query request

2016-03-09 Thread Greg KH
On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote:
> This patch exposes the ioctl interface for UFS driver via SCSI device
> ioctl interface. As of now UFS driver would provide the ioctl for query
> interface to connected UFS device.
> 
> Reviewed-by: Subhash Jadavani 
> Signed-off-by: Dolev Raviv 
> Signed-off-by: Gilad Broner 
> Signed-off-by: Yaniv Gardi 

What tool is going to use this ioctl?  Why does userspcae want to do
something "special" with UFS devices?  Shouldn't they just be treated
like any other normal block device?

thanks,

greg k-h
--
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 v2] megaraid_sas: add an i/o barrier

2016-02-01 Thread Greg KH
On Mon, Feb 01, 2016 at 03:12:04PM +0100, Tomas Henzl wrote:
> A barrier should be added to ensure proper
> ordering of memory mapped writes.
> 
> V2: - added the barrier also to megasas_fire_cmd_skinny,
> as suggested by Kashyap Desai
> 
> Signed-off-by: Tomas Henzl 
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c   | 1 +
>  drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 +
>  2 files changed, 2 insertions(+)



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.


--
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] megaraid_sas : Add locking to megasas_aen_polling

2015-10-30 Thread Greg KH
On Fri, Oct 30, 2015 at 01:47:50PM -0400, Ben Guthro wrote:
> From: Glenn Watkins 
> 
> Under conditions of offlining drives, and rescanning the scsi host,
> we can get into situations that the megasas_aen_polling kthread
> can crash(GPF) in the megasas_aen_polling work queue:
> 
> [ 1206.568641] general protection fault:  [#1] SMP
> [ 1206.569479] Modules linked in: xt_tcpudp nf_conntrack_ipv4 nf_defrag_ipv4 
> xt_state nf_conntrack iptable_filter ip_tables x_tables coretemp 
> crct10dif_pclmul crc32_pclmul aesni_intel ablk_helper cryptd psmouse lrw 
> vmwgfx gf128mul serio_raw glue_helper aes_x86_64 ppdev ttm microcode 
> vmw_balloon drm_kms_helper drm parport_pc parport fb_sys_fops sysimgblt 
> sysfillrect syscopyarea vmw_vmci binfmt_misc floppy mptspi mptscsih 
> vmw_pvscsi megaraid_sas pata_acpi mptbase vmxnet3
> [ 1206.576488] CPU: 0 PID: 1157 Comm: kworker/0:2 Not tainted 4.3.0-rc7-svt1 
> #1
> [ 1206.577520] Hardware name: VMware, Inc. VMware Virtual Platform/440BX 
> Desktop Reference Platform, BIOS 6.00 04/14/2014
> [ 1206.579101] Workqueue: events megasas_aen_polling [megaraid_sas]
> [ 1206.580007] task: 8818bb7b8000 ti: 8818ca28 task.ti: 
> 8818ca28
> [ 1206.581104] RIP: 0010:[]  [] 
> bdi_unregister+0x3d/0x1e0
> [ 1206.582339] RSP: 0018:8818ca283cb8  EFLAGS: 00010246
> [ 1206.583131] RAX: dead0200 RBX: 8818bb603f08 RCX: 
> 8818c6487800
> [ 1206.584184] RDX: 8818bb603f08 RSI: 7fff RDI: 
> 81f9aa68
> [ 1206.585243] RBP: 8818ca283d18 R08:  R09: 
> 
> [ 1206.586294] R10: 000fffe0 R11: dead0200 R12: 
> 8818bb6042f0
> [ 1206.587346] R13: 8818bb604530 R14: 00ae R15: 
> 0080
> [ 1206.588388] FS:  () GS:88193fc0() 
> knlGS:
> [ 1206.589598] CS:  0010 DS:  ES:  CR0: 8005003b
> [ 1206.590457] CR2: 01a89000 CR3: 0018c07f2000 CR4: 
> 000406f0
> [ 1206.591545] Stack:
> [ 1206.591870]  8818bb6042f0 8818bb603d78 00ae 
> 0080
> [ 1206.593098]  8818ca283ce8 8108f683 8818ca283d18 
> 813332b0
> [ 1206.594308]  8818ca283d18 8818bb603d78 8818bb6042f0 
> 8818bb604530
> [ 1206.595532] Call Trace:
> [ 1206.595922]  [] ? cancel_delayed_work_sync+0x13/0x20
> [ 1206.596903]  [] ? blk_sync_queue+0x80/0x90
> [ 1206.597753]  [] blk_cleanup_queue+0x114/0x150
> [ 1206.598645]  [] __scsi_remove_device+0x54/0xd0
> [ 1206.599556]  [] scsi_remove_device+0x2f/0x50
> [ 1206.600441]  [] megasas_aen_polling+0x34d/0x670 
> [megaraid_sas]
> [ 1206.601561]  [] process_one_work+0x14c/0x400
> [ 1206.602449]  [] worker_thread+0x117/0x480
> [ 1206.603295]  [] ? create_worker+0x1c0/0x1c0
> [ 1206.604160]  [] kthread+0xc9/0xe0
> [ 1206.604898]  [] ? flush_kthread_worker+0x90/0x90
> [ 1206.605831]  [] ret_from_fork+0x3f/0x70
> [ 1206.606659]  [] ? flush_kthread_worker+0x90/0x90
> [ 1206.607585] Code: c7 c7 68 aa f9 81 48 83 ec 48 e8 bf 76 59 00 48 8b 43 08 
> 48 8b 13 49 bb 00 02 00 00 00 00 ad de 48 c7 c7 68 aa f9 81 48 89 42 08 <48> 
> 89 10 4c 89 5b 08 e8 27 76 59 00 e8 32 92 f4 ff 48 8d 7b 50
> [ 1206.611938] RIP  [] bdi_unregister+0x3d/0x1e0
> [ 1206.612856]  RSP 
> 
> This can be readily reproduced by a pair of shell scripts - one of which 
> loops on
> onlining / offlining drives via MegaCli (or storcli, if you prefer)
> 
> #!/bin/bash
> 
> while [ 1 ]; do
> /opt/MegaRAID/MegaCli/MegaCli64 pdoffline physdrv[32:0] a0 &>2
> /opt/MegaRAID/MegaCli/MegaCli64 pdoffline physdrv[32:11] a0 &>2
> 
> /opt/MegaRAID/MegaCli/MegaCli64 pdonline physdrv[32:0] a0 &>2
> /opt/MegaRAID/MegaCli/MegaCli64 pdonline physdrv[32:11] a0 &>2
> done
> 
> Meanwhile, the second script is looping on rescanning the scsi hosts:
> 
> #!/bin/bash
> while [ 1 ]; do
> for (( l=0; l<4; l++ )); do
> echo - - - > /sys/class/scsi_host/host$l/scan
> done
> done
> 
> This was originally introduced in the following commit:
> 
> commit 7e8a75f4dfbff173977b2f58799c3eceb7b09afd
> Author: Yang, Bo 
> Date:   Tue Oct 6 14:50:17 2009 -0600
> 
> [SCSI] megaraid_sas: Add the support for updating the OS after 
> adding/removing the devices from FW
> 
> The fix for this is to add some locking around the AEN polling.
> Since this affects all kernels since 2.6.33, I have also CC'ed the stable 
> list.
> 
> Signed-off-by: Glenn Watkins 
> Signed-off-by: Ben Guthro 
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c |2 ++
>  1 file changed, 2 insertions(+)
> 



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" 

Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management

2015-10-16 Thread Greg KH
On Fri, Oct 16, 2015 at 01:03:42PM -0700, Lee Duncan wrote:
> Adding linux-usb and linux-hotplug to cc list, in case they wish to comment.
> 
> Summary: I want to change SCSI host number so that it gets re-used, like
> disk index numbers, instead of always increasing.
> 
> Please see below.
> 
> On 10/14/2015 11:53 AM, James Bottomley wrote:
> > On Wed, 2015-10-14 at 11:34 -0700, Lee Duncan wrote:
> >> On 10/14/2015 06:55 AM, James Bottomley wrote:
> >>> On Wed, 2015-10-07 at 16:51 -0700, Lee Duncan wrote:
>  Update the SCSI hosts module to use the ida_simple*() routines
>  to manage its host_no index instead of an ATOMIC integer. This
>  means that the SCSI host number will now be reclaimable.
> >>>
> >>> OK, but why would we want to do this?  We do it for sd because our minor
> >>> space for the device nodes is very constrained, so packing is essential.
> >>> For HBAs, there's no device space density to worry about, they're
> >>> largely statically allocated at boot time and not reusing the numbers
> >>> allows easy extraction of hotplug items for the logs (quite useful for
> >>> USB) because each separate hotplug has a separate and monotonically
> >>> increasing host number.
> >>>
> >>> James
> >>>
> >>
> >> Good question, James. Apologies for not making the need clear.
> >>
> >> The iSCSI subsystem uses a host structure for discovery, then throws it
> >> away. So each time it does discovery it gets a new host structure. With
> >> the current approach, that number is ever increasing. It's only a matter
> >> of time until some user with a hundreds of disks and perhaps thousands
> >> of LUNs, that likes to do periodic discovery (think super-computers)
> >> will run out of host numbers. Or, worse yet, get a negative number
> >> number (because the value is signed right now).
> >>
> >> And this use case is a real one right now, by the way.
> > 
> > Um, so even if you do discovery continuously, say one every second, it
> > still will take 68 years before we wrap the sign.
> > 
> >> As you can see from the patch, it's a small amount of code to ensure
> >> that the host number management is handled more cleanly.
> > 
> > Well, I'm a bit worried about the loss of a monotonically increasing
> > host number from the debugging perspective.  Right now, if you look at
> > any log, hostX always refers to one and only one incarnation throughout
> > the system lifetime for any given value of X.  With your patch, the
> > lowest host number gets continually reused ... probably for every hot
> > plug event.  If the USB and other hotplug system people don't mind this,
> > I suppose I can live with it, but I'd like to hear their view before
> > making this change.

USB "people" don't care about this, why would we?  You can plug and
unplug and plug devices in lots of times and they get the "old" names
all the time, this is something that tools have had to deal with for
well over a decade.

thanks,

greg k-h
--
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 v2] hpsa: convert DEVICE_ATTR to RO|WO|RW and show methods must not use snprintf

2015-06-30 Thread Greg KH
On Wed, Jul 01, 2015 at 01:56:03AM +, Seymour, Shane M wrote:
 
 Changed DEVICE_ATTR macro usage to DEVICE_ATTR_RO|WO|RW.
 This also forced some show/store function names to change.
 
 Changed all show method snprintf() usage to scnprintf() per
 Documentation/filesystems/sysfs.txt.

Forgot to mention this before, but you are doing two different things,
so this really should be 2 different patches.

But I'm not the scsi maintainer, maybe they have more lax acceptance
rules here :)

thanks,

greg k-h
--
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/3] hpsa: Convert DEVICE_ATTR macro usage to DEVICE_ATTR_RO|WR|WO

2015-06-30 Thread Greg KH g...@kroah.com (g...@kroah.com)
On Wed, Jul 01, 2015 at 03:47:10AM +, Seymour, Shane M wrote:
 
 Convert DEVICE_ATTR macro usage to DEVICE_ATTR_RO|WR|WO
 Changes forced some function names to change.
 
 Signed-off-by: Shane Seymour shane.seym...@hp.com

Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org
--
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] hpsa: convert DEVICE_ATTR to RO|WO|RW and show methods must use scnprintf

2015-06-30 Thread Greg KH
On Tue, Jun 30, 2015 at 05:22:20AM +, Seymour, Shane M wrote:
 
 Changed DEVICE_ATTR macro usage to DEVICE_ATTR_RO|WO|RW.
 This also forced some show/store function names to change.
 
 Changed all show method sprint/snprintf usage to scnprintf per
 Documentation/filesystems/sysfs.txt.

There's no need to change sprintf() to scnprintf() at all, that's just
useless churn.

 
 Signed-off-by: Shane Seymour shane.seym...@hp.com
 ---
 --- a/drivers/scsi/hpsa.c 2015-06-25 15:52:15.633031319 -0500
 +++ b/drivers/scsi/hpsa.c 2015-06-29 17:28:24.628475369 -0500
 @@ -376,7 +376,7 @@ static int check_for_busy(struct ctlr_in
  }
  
  static u32 lockup_detected(struct ctlr_info *h);
 -static ssize_t host_show_lockup_detected(struct device *dev,
 +static ssize_t lockup_detected_show(struct device *dev,
   struct device_attribute *attr, char *buf)
  {
   int ld;
 @@ -386,10 +386,10 @@ static ssize_t host_show_lockup_detected
   h = shost_to_hba(shost);
   ld = lockup_detected(h);
  
 - return sprintf(buf, ld=%d\n, ld);
 + return scnprintf(buf, PAGE_SIZE, ld=%d\n, ld);

Like here, it's obvious that the original will never overflow, so don't
even worry about checking, it's pointless.

So please don't change this.

greg k-h
--
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: Backport of a fix for HPSA (Disabling a disabled device problem during kdump) driver

2015-06-29 Thread Greg KH
On Fri, May 22, 2015 at 12:19:16PM -0700, Vinson Lee wrote:
 On Sat, May 2, 2015 at 10:56 AM, Greg KH g...@kroah.com wrote:
  On Mon, Apr 27, 2015 at 03:10:35PM +0200, Tomas Henzl wrote:
  On 04/27/2015 02:55 PM, Greg KH wrote:
   On Mon, Apr 27, 2015 at 02:17:32PM +0200, Tomas Henzl wrote:
   On 04/26/2015 11:27 AM, Greg KH wrote:
   On Mon, Apr 13, 2015 at 03:18:44PM +0200, Tomas Henzl wrote:
   On 04/11/2015 12:45 AM, Vinson Lee wrote:
   On Tue, Jan 27, 2015 at 4:18 PM, Greg KH g...@kroah.com wrote:
   On Tue, Jan 06, 2015 at 05:15:19PM +0100, Tomas Henzl wrote:
   On 01/05/2015 07:41 PM, Masoud Sharbiani wrote:
   Dear stable maintainers,
   Can you please backport commitid 
   132aa220b45d60e9b20def1e9d8be9422eed9616
(hpsa: refine the pci enable/disable handling) to 3.10 stable 
   (and
   earlier, if applicable)?
   Please do not apply this patch isolated from his friend, the
   859c75aba20264d87dd026bab0d0ca3bff385955 hpsa: add missing 
   pci_set_master in kdump path
   needs to be applied together with the 
   132aa220b45d60e9b20def1e9d8be9422eed9616 .
  
   In addition to that, after the original issue goes away you may 
   notice sometimes
   an unhandled irq 16 message, to fix this a patch is posted
   here http://www.spinics.net/lists/linux-scsi/msg80316.html
   This patch still awaits a maintainers review though.
  
   Probably the best idea is to wait until the issue is solved 
   completely.
   I'll wait, when it all gets worked out, please let stable@ know what
   patches to apply where.
  
   thanks,
  
   greg k-h
   --
   To unsubscribe from this list: send the line unsubscribe stable in
   the body of a message to majord...@vger.kernel.org
   More majordomo info at  http://vger.kernel.org/majordomo-info.html
   Hi, Thomas.
  
   The unhandled irq 16 issue seems to be fixed by 4.0-rc1 commit hpsa:
   turn off interrupts when kdump starts.
  
   Are the following patches suitable for stable now?
   Yes, I believe they are, just note that
   03741d9 hpsa: fix memory leak in kdump hard reset
   is not a part of that group we discussed before, but it may be added
   to stable too.
  
   Cheers,
   Tomas
  
   3b74729 hpsa: turn off interrupts when kdump starts
   03741d9 hpsa: fix memory leak in kdump hard reset
   859c75a hpsa: add missing pci_set_master in kdump path
   132aa22 hpsa: refine the pci enable/disable handling
   So what order, and to what stable tree(s) do you want these applied to?
   They seem to span a number of kernel versions, so I'm not quite sure
   what you are wanting me to do.
   Please add it in this order
   1. 132aa220b4 hpsa: refine the pci enable/disable handling
   2. 859c75aba2 hpsa: add missing pci_set_master in kdump path
  The first two patches create the fix Masoud has asked for originally 
   to port to stable.
   3. 3b74729878 hpsa: turn off interrupts when kdump starts
  The third should be added too, it fixes an issue made visible by the 
   first patch
  
   The fourth patch
   03741d956e hpsa: fix memory leak in kdump hard reset
   is completely unrelated to the first group, so it may be added after or 
   before
   or in the same sequence as it is in the mainline - that is before the 
   third patch.
   This patch is tiny error path fix - not sure if important enough for 
   stable.
   Which stable tree(s) do you want these applied to?  As these span
   different kernel versions, I have no idea of what to do here.
 
  Masoud has asked for the inclusion, I think that what he wrote was - 3.10 
  stable (and earlier, if applicable),
  I don't know much about stable so I can't decide to which tree(s) the 
  series should be ported.
 
  Ok, I'll wait for someone to clarify before doing anything here.
 
  thanks,
 
  greg k-h
 
 
 The first and second patches are from upstream 3.18. The third patch
 is from upstream 4.0.
 
 Please use the following list of kernels and patches for backporting.
 
 3.18, 3.19
 3b74729878 hpsa: turn off interrupts when kdump starts
 
 = 3.17
 132aa220b4 hpsa: refine the pci enable/disable handling
 859c75aba2 hpsa: add missing pci_set_master in kdump path

These two worked for 3.10 and 3.14-stable.

 3b74729878 hpsa: turn off interrupts when kdump starts

This one did not apply, so I can't add it.

thanks,

greg k-h
--
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] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

2015-06-24 Thread Greg KH gre...@linuxfoundation.org (gre...@linuxfoundation.org)
On Wed, Jun 24, 2015 at 03:25:57PM +0900, Sergey Senozhatsky wrote:
 On (06/24/15 06:10), Seymour, Shane M wrote:
 [..]
   
   /* The sysfs driver interface. Read-only at the moment */
  -static ssize_t st_try_direct_io_show(struct device_driver *ddp, char *buf)
  +static ssize_t try_direct_io_show(struct device_driver *ddp, char *buf)
   {
  -   return snprintf(buf, PAGE_SIZE, %d\n, try_direct_io);
  +   return sprintf(buf, %d\n, try_direct_io);
   }
 
 a nitpick,
 
 per Documentation/filesystems/sysfs.txt
 
 :
 : - show() should always use scnprintf().
 :

Don't believe everything you read, this change is just fine.

But, you are doing something here that you did not say you were doing in
the changelog, so for that reason, the patch should be redone.

thanks,

greg k-h
--
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] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

2015-06-24 Thread Greg KH gre...@linuxfoundation.org (gre...@linuxfoundation.org)
On Wed, Jun 24, 2015 at 03:25:57PM +0900, Sergey Senozhatsky wrote:
 On (06/24/15 06:10), Seymour, Shane M wrote:
 [..]
   
   /* The sysfs driver interface. Read-only at the moment */
  -static ssize_t st_try_direct_io_show(struct device_driver *ddp, char *buf)
  +static ssize_t try_direct_io_show(struct device_driver *ddp, char *buf)
   {
  -   return snprintf(buf, PAGE_SIZE, %d\n, try_direct_io);
  +   return sprintf(buf, %d\n, try_direct_io);
   }
 
 a nitpick,
 
 per Documentation/filesystems/sysfs.txt
 
 :
 : - show() should always use scnprintf().
 :

That should be rewritten to say, don't use snprintf(), but scnprintf(),
if you want to.  Otherwise sprintf() should be fine as you obviously are
only returning a single value to userspace

Or something like that.

thanks,

greg k-h
--
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 v2] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

2015-06-24 Thread Greg KH gre...@linuxfoundation.org (gre...@linuxfoundation.org)
On Wed, Jun 24, 2015 at 06:54:35AM +, Seymour, Shane M wrote:
 
 Convert DRIVER_ATTR macros to DRIVER_ATTR_RO requested by
 Greg KH. Also switched to using scnprintf instead of snprintf
 per Documentation/filesystems/sysfs.txt.
 
 Suggested-by: Greg Kroah-Hartman gre...@linuxfoundation.org
 Signed-off-by: Shane Seymour shane.seym...@hp.com
 ---
 This patch was implemented on top of the previous patch to
 convert to using driver attr groups.
 
 Changes from v1:
 - switched to scnprintf from sprintf after feedback from Sergey
 Senozhatsky.

Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org
--
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] st: convert to using driver attr groups for sysfs

2015-06-23 Thread Greg KH
On Tue, Jun 23, 2015 at 08:11:00AM +, Seymour, Shane M wrote:
 This patch changes the st driver to use attribute groups so
 driver sysfs files are created automatically. See the
 following for reference:
 
 http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
 
 Signed-off-by: Shane Seymour shane.seym...@hp.com

Very nice, thanks for doing this.

Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org

 ---
 --- a/drivers/scsi/st.c   2015-06-22 14:20:40.829612661 -0500
 +++ b/drivers/scsi/st.c   2015-06-22 15:49:49.357248393 -0500
 @@ -85,6 +85,7 @@ static int debug_flag;
  
  static struct class st_sysfs_class;
  static const struct attribute_group *st_dev_groups[];
 +static const struct attribute_group *st_drv_groups[];
  
  MODULE_AUTHOR(Kai Makisara);
  MODULE_DESCRIPTION(SCSI tape (st) driver);
 @@ -198,15 +199,13 @@ static int sgl_unmap_user_pages(struct s
  static int st_probe(struct device *);
  static int st_remove(struct device *);
  
 -static int do_create_sysfs_files(void);
 -static void do_remove_sysfs_files(void);
 -
  static struct scsi_driver st_template = {
   .gendrv = {
   .name   = st,
   .owner  = THIS_MODULE,
   .probe  = st_probe,
   .remove = st_remove,
 + .groups = st_drv_groups,
   },
  };
  
 @@ -4404,14 +4403,8 @@ static int __init init_st(void)
   if (err)
   goto err_chrdev;
  
 - err = do_create_sysfs_files();
 - if (err)
 - goto err_scsidrv;
 -
   return 0;
  
 -err_scsidrv:
 - scsi_unregister_driver(st_template.gendrv);
  err_chrdev:
   unregister_chrdev_region(MKDEV(SCSI_TAPE_MAJOR, 0),
ST_MAX_TAPE_ENTRIES);
 @@ -4422,7 +4415,6 @@ err_class:
  
  static void __exit exit_st(void)
  {
 - do_remove_sysfs_files();
   scsi_unregister_driver(st_template.gendrv);
   unregister_chrdev_region(MKDEV(SCSI_TAPE_MAJOR, 0),
ST_MAX_TAPE_ENTRIES);
 @@ -4459,44 +4451,14 @@ static ssize_t st_version_show(struct de
  }
  static DRIVER_ATTR(version, S_IRUGO, st_version_show, NULL);

For a future patch, you might want to convert these type of declarations
to use DRIVER_ATTR_RO() and friends.

thanks,

greg k-h
--
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: Backport of a fix for HPSA (Disabling a disabled device problem during kdump) driver

2015-04-27 Thread Greg KH
On Mon, Apr 27, 2015 at 02:17:32PM +0200, Tomas Henzl wrote:
 On 04/26/2015 11:27 AM, Greg KH wrote:
  On Mon, Apr 13, 2015 at 03:18:44PM +0200, Tomas Henzl wrote:
  On 04/11/2015 12:45 AM, Vinson Lee wrote:
  On Tue, Jan 27, 2015 at 4:18 PM, Greg KH g...@kroah.com wrote:
  On Tue, Jan 06, 2015 at 05:15:19PM +0100, Tomas Henzl wrote:
  On 01/05/2015 07:41 PM, Masoud Sharbiani wrote:
  Dear stable maintainers,
  Can you please backport commitid 
  132aa220b45d60e9b20def1e9d8be9422eed9616
   (hpsa: refine the pci enable/disable handling) to 3.10 stable (and
  earlier, if applicable)?
  Please do not apply this patch isolated from his friend, the
  859c75aba20264d87dd026bab0d0ca3bff385955 hpsa: add missing 
  pci_set_master in kdump path
  needs to be applied together with the 
  132aa220b45d60e9b20def1e9d8be9422eed9616 .
 
  In addition to that, after the original issue goes away you may notice 
  sometimes
  an unhandled irq 16 message, to fix this a patch is posted
  here http://www.spinics.net/lists/linux-scsi/msg80316.html
  This patch still awaits a maintainers review though.
 
  Probably the best idea is to wait until the issue is solved completely.
  I'll wait, when it all gets worked out, please let stable@ know what
  patches to apply where.
 
  thanks,
 
  greg k-h
  --
  To unsubscribe from this list: send the line unsubscribe stable in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html
  Hi, Thomas.
 
  The unhandled irq 16 issue seems to be fixed by 4.0-rc1 commit hpsa:
  turn off interrupts when kdump starts.
 
  Are the following patches suitable for stable now?
  Yes, I believe they are, just note that 
  03741d9 hpsa: fix memory leak in kdump hard reset
  is not a part of that group we discussed before, but it may be added
  to stable too.
 
  Cheers,
  Tomas
 
  3b74729 hpsa: turn off interrupts when kdump starts
  03741d9 hpsa: fix memory leak in kdump hard reset
  859c75a hpsa: add missing pci_set_master in kdump path
  132aa22 hpsa: refine the pci enable/disable handling
  So what order, and to what stable tree(s) do you want these applied to?
  They seem to span a number of kernel versions, so I'm not quite sure
  what you are wanting me to do.
 
 Please add it in this order
 1. 132aa220b4 hpsa: refine the pci enable/disable handling
 2. 859c75aba2 hpsa: add missing pci_set_master in kdump path
The first two patches create the fix Masoud has asked for originally to 
 port to stable.
 3. 3b74729878 hpsa: turn off interrupts when kdump starts
The third should be added too, it fixes an issue made visible by the first 
 patch
 
 The fourth patch
 03741d956e hpsa: fix memory leak in kdump hard reset
 is completely unrelated to the first group, so it may be added after or before
 or in the same sequence as it is in the mainline - that is before the third 
 patch.
 This patch is tiny error path fix - not sure if important enough for stable.

Which stable tree(s) do you want these applied to?  As these span
different kernel versions, I have no idea of what to do here.

Please be specific.

thanks,

greg k-h
--
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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-10 Thread Greg KH
On Tue, Feb 10, 2015 at 02:27:20PM +, Bryn M. Reeves wrote:
 On Sat, Feb 07, 2015 at 12:07:43PM +0800, Greg KH wrote:
 
 $ cat /sys/fs/selinux/avc/cache_stats 
 lookups hits misses allocations reclaims frees
 18938916 18921707 17209 17209 17328 22215
 38164283 38146514 17769 17769 16800 19049
 18078108 18056991 21117 21117 21344 19305
 15168204 15150079 18125 18125 17776 13149
 0 0 0 0 0 0
 0 0 0 0 0 0
 0 0 0 0 0 0
 0 0 0 0 0 0
 
 $ cat /sys/fs/selinux/avc/hash_stats
 entries: 506
 buckets used: 290/512
 longest chain: 5

Ugh, those look like they should be debugfs interfaces.  Thanks, I'll
add them to my list of things to nag people about...

greg k-h
--
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] st implement tape statistics v3 (updated patch)

2015-02-09 Thread Greg KH
On Mon, Feb 09, 2015 at 11:56:25AM +, Seymour, Shane M wrote:
 Hi Greg,
 
 Thank you for pushing me to go that little further. The statistics directory 
 is back. Any feedback from anyone would be appreciated.
 
 Thanks
 Shane
 --
 
 Signed-off-by: shane.seym...@hp.com
 Tested-by: shane.seym...@hp.com

Please provide a real changelog so that people can figure out what is
going on here.

And you need '' and '' for your signed-off-by lines.


 ---
 --- a/drivers/scsi/st.c   2015-01-11 14:46:00.243814755 -0600
 +++ b/drivers/scsi/st.c   2015-02-09 00:05:46.838967740 -0600
 @@ -476,10 +476,25 @@ static void st_scsi_execute_end(struct r
   struct st_request *SRpnt = req-end_io_data;
   struct scsi_tape *STp = SRpnt-stp;
   struct bio *tmp;
 + u64 ticks;
  
   STp-buffer-cmdstat.midlevel_result = SRpnt-result = req-errors;
   STp-buffer-cmdstat.residual = req-resid_len;
  
 +/* Note that irrespective of the I/O succeeding or not we count it. We
 +   don't have bufflen any more so a read at end of file will over count
 +   the blocks by one. */
 + if (STp-stats != NULL) {
 + ticks = get_jiffies_64();
 + STp-stats-in_flight--;
 + ticks -= STp-stats-stamp;
 + STp-stats-io_ticks += ticks;
 + if (req-cmd[0] == WRITE_6)
 + STp-stats-write_ticks += ticks;
 + else if (req-cmd[0] == READ_6)
 + STp-stats-read_ticks += ticks;
 + }
 +
   tmp = SRpnt-bio;
   if (SRpnt-waiting)
   complete(SRpnt-waiting);
 @@ -496,6 +511,7 @@ static int st_scsi_execute(struct st_req
   struct rq_map_data *mdata = SRpnt-stp-buffer-map_data;
   int err = 0;
   int write = (data_direction == DMA_TO_DEVICE);
 + struct scsi_tape *STp = SRpnt-stp;
  
   req = blk_get_request(SRpnt-stp-device-request_queue, write,
 GFP_KERNEL);
 @@ -516,6 +532,20 @@ static int st_scsi_execute(struct st_req
   }
   }
  
 + if (STp-stats != NULL) {
 + if (cmd[0] == WRITE_6) {
 + STp-stats-write_cnt++;
 + STp-stats-write_byte_cnt += bufflen;
 + } else if (cmd[0] == READ_6) {
 + STp-stats-read_cnt++;
 + STp-stats-read_byte_cnt += bufflen;
 + } else {
 + STp-stats-other_cnt++;
 + }
 + STp-stats-stamp = get_jiffies_64();
 + STp-stats-in_flight++;
 + }
 +
   SRpnt-bio = req-bio;
   req-cmd_len = COMMAND_SIZE(cmd[0]);
   memset(req-cmd, 0, BLK_MAX_CDB);
 @@ -4064,6 +4094,7 @@ out:
  static int create_cdevs(struct scsi_tape *tape)
  {
   int mode, error;
 +

Not related to this patch.

   for (mode = 0; mode  ST_NBR_MODES; ++mode) {
   error = create_one_cdev(tape, mode, 0);
   if (error)
 @@ -4222,6 +4253,8 @@ static int st_probe(struct device *dev)
   }
   tpnt-index = error;
   sprintf(disk-disk_name, st%d, tpnt-index);
 +/* Allocate stats structure */

Odd indentation.

 + tpnt-stats = kzalloc(sizeof(struct scsi_tape_stats), GFP_ATOMIC);

No checking to see if it succeeded?  Why not?

And why ATOMIC?  You are not in interrupt contect in your probe
function.

  
   dev_set_drvdata(dev, tpnt);
  
 @@ -4248,6 +4281,8 @@ out_put_queue:
   blk_put_queue(disk-queue);
  out_put_disk:
   put_disk(disk);
 + if (tpnt-stats != NULL)
 + kfree(tpnt-stats);

The if check is not needed.

   kfree(tpnt);
  out_buffer_free:
   kfree(buffer);
 @@ -4298,6 +4333,10 @@ static void scsi_tape_release(struct kre
  
   disk-private_data = NULL;
   put_disk(disk);
 + if (tpnt-stats != NULL) {
 + kfree(tpnt-stats);
 + tpnt-stats = NULL;
 + }
   kfree(tpnt);
   return;
  }
 @@ -4513,6 +4552,223 @@ options_show(struct device *dev, struct
  }
  static DEVICE_ATTR_RO(options);
  
 +/* Support for tape stats */
 +
 +/**
 + * read_cnt_how - return read count - count of reads made from tape drive
 + * @dev: struct device
 + * @attr: attribute structure
 + * @buf: buffer to return formatted data in
 + */
 +static ssize_t read_cnt_show(struct device *dev,
 + struct device_attribute *attr, char *buf)
 +{
 + struct st_modedef *STm = dev_get_drvdata(dev);
 + struct scsi_tape *STp = STm-tape;
 + struct scsi_tape_stats *STt;
 +
 + if ((STp == 0) || (STp-stats == 0))
 + return -EINVAL;

How can STp or STP-stats ever be NULL?  (hint, check against NULL for
typesafe)

 + STt = STp-stats;
 + if (STt == 0)
 + return -EINVAL;

You already made this check above?  Please fix all of this up in all of
these callbacks, the checking should not be needed.

 + return snprintf(buf, PAGE_SIZE, %llu, STt-read_cnt);
 +}
 +static DEVICE_ATTR_RO(read_cnt);
 +
 +/**
 + * read_byte_cnt_show - return read byte count - tape 

Re: [PATCH] st implement tape statistics v3

2015-02-09 Thread Greg KH
On Mon, Feb 09, 2015 at 06:01:39AM +, Seymour, Shane M wrote:
 Hi All,
 
 Please find attached v3 of the patch. It implements the changes requested by 
 Greg. The statistics files aren't in a separate directory any more they're 
 implemented directly as device attributes unless someone has objections to 
 them being in a place like /sys/class/scsi_tape/*/.

Why not keep them in a subdir, like before?  You can still do that with
a sysfs attribute group, just make a new one and add it to the existing
list of groups for the tape device.  Makes things a bit more simple
for userspace, they can just iterate over all of the files in the
directory, instead of having to only open some of them.

thanks,

greg k-h
--
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 RESEND] st implement tape statistics v3

2015-02-09 Thread Greg KH
Comment about your subject: the v3 needs to go inside the []

And you need to put below the --- line what has changed from the last
version, I don't see any of my comments address here :(

On Mon, Feb 09, 2015 at 10:41:38PM +, Seymour, Shane M wrote:
 The following patch exposes statistics for the st driver via sysfs. There is 
 a need for companies with large numbers of tape drives numbering in the tens 
 of thousands to track the performance of those tape drives (e.g. when a 
 backup exceeds its window). The statistics provided should allow the 
 calculation of throughput, average block sizes for read and write, and time 
 spent waiting for I/O to complete or doing tape movement.

Your keyboard has an Enter key, please use it around the 72nd column :)

thanks,

greg k-h
--
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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-07 Thread Greg KH
On Fri, Feb 06, 2015 at 03:41:58PM +, Bryn M. Reeves wrote:
 On Fri, Feb 06, 2015 at 04:59:16AM -0800, Greg KH wrote:
  On Fri, Feb 06, 2015 at 12:20:53AM +, Seymour, Shane M wrote:
   The current patch that implements tape statistics is here:
   
   http://marc.info/?l=linux-scsim=142112067313723w=2
  
  Aside from the do we want to do this all in a single file issue that I
  will say more on below, this patch has issues.  Please don't use a
  kobject for _ANYTHING_ in sysfs that has a struct device as a parent.
  If you do that, it can't be seen by userspace tools very well, if at
  all.
 
 I can't speak for Shane but wouldn't spend too much time looking at the
 current v2 patch: it's the result of a pretty ugly compromise suggested
 on linux-scsi.

Fair enough, but please feel free to cc: me on the patch that you do
feel is correct to get a sysfs-related review.

   Recently there was was another discussion here about one file vs a 
   collection of files for tape statistics:
   
   http://marc.info/?l=linux-scsim=142316255501550w=2
   
   The result was that I should ask here what method I should use. I
   would like to get feedback in relation to tape statistics and one file
   vs multi-file in sysfs. I'm happy to keep the existing code or change
   to a single file approach.
  
  One of the primary reasons we created sysfs and the one value per file
  rule is that multi-value files just do not work well.  Yes, you get an
  atomic snapshot, and you save some open/read/close syscall roundtrips,
  but you do so at the expense of forcing userspace to know what the
  format of the file is.  And once you create it, you can NEVER CHANGE IT
  AGAIN.
 
 I am not convinced this is a concern for tape statistics: they are pretty
 much a solved problem. The commercial *nixes have had this for decades.
 
 Likewise for disk stats: although fluff like maj:min/name etc. has been
 shuffled a few times the basic fields have remained unchanged for a very
 long time and sysfs already removes the need to include an identity
 field.

We already handle i/o stats just fine, why create a special sysfs
interface for just a tape device interface?  What makes them so special?

  Yes, that's right, if you come up with some new statistic in the future,
  or realize that one of the ones you have now is wrong, you can't change
  it, you have to make a whole new file, otherwise you could break
  userspace tools.
 
 I understand the fact that you can't change them; I just don't think it's
 a big problem in this specific case (and much less than some of the
 more imaginative sysfs content - 2d int arrays with column headers
 anyone?).

What sysfs file is a 2d int array?  I'll be glad to fix it.

Also, everyone doesn't think their solution will ever need to be
changed.  Until later when it does :)

  And yes, open/read/close does take take a few extra cycles, but you
  can't really measure it for a virtual filesystem like this on any modern
  system.
 
 I'll try to get some numbers when I get back home next week - Shane is
 talking about use cases involving tens of thousands of tape devices. I
 am not certain that the overhead would be unmeasurable in that case: the
 additional context switching  TLB flushes alone seem like they would
 add up.

If you want to measure tens of thousands of tape devices then you
shouldn't be using sysfs in the first place as it is not designed for
speed at all.  Use the existing i/o rate interfaces instead, don't try
to cram something into sysfs that doesn't belong there.

thanks,

greg k-h
--
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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-07 Thread Greg KH
On Sat, Feb 07, 2015 at 09:27:05PM -0500, Laurence Oberman wrote:
 Hello
 Its not going to be tens of thousands of devices. That count was an
 aggregate based on 1000's of servers.
 In reality its unlikely to ever be more than 100 tapes drives per
 individual Linux kernel instance.
 Therefore sysfs will be the valid way to do this and make the data
 available to user space.

Even if it is only 2 tape drives, again, what's wrong with using the
existing i/o statistic interfaces that all block devices have?  Don't go
making special one-off interfaces for one type of device if at all
possible.

thanks,

greg k-h
--
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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-06 Thread Greg KH
On Fri, Feb 06, 2015 at 09:13:55AM +, Bryn M. Reeves wrote:
  The sysfs documentation says that files should contain one item per
  file (with some small exceptions):
  
   Attributes should be ASCII text files, preferably with only one value
   per file. It is noted that it may not be efficient to contain only one
   value per file, so it is socially acceptable to express an array of
   values of the same type.
 
 Right: I think there's good precedent for the array file style when
 dealing with counter sets.

See my previous reply for why I strongly feel this is incorrect, sorry.

greg k-h
--
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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-06 Thread Greg KH
On Fri, Feb 06, 2015 at 12:20:53AM +, Seymour, Shane M wrote:
 Hello linux-api'ers
 
 There has been some ongoing discussion about the best way to implement tape 
 statistics. The original method suggested a long time ago used a single file 
 in sysfs similar to block statistics in sysfs. That lead to an impass about 
 the code on the linux-scsi mailing list.
 
 The sysfs documentation says that files should contain one item per file 
 (with some small exceptions):
 
  Attributes should be ASCII text files, preferably with only one value
  per file. It is noted that it may not be efficient to contain only one
  value per file, so it is socially acceptable to express an array of
  values of the same type.
 
 The current patch that implements tape statistics is here:
 
 http://marc.info/?l=linux-scsim=142112067313723w=2

Aside from the do we want to do this all in a single file issue that I
will say more on below, this patch has issues.  Please don't use a
kobject for _ANYTHING_ in sysfs that has a struct device as a parent.
If you do that, it can't be seen by userspace tools very well, if at
all.

Instead, if you want to create a directory, just use an attribute group,
which will be created at the proper time (before the device is announced
to userspace), and then userspace can see it with the tools it is used
to using (i.e. libudev and friends.)

That should simplify your code a lot, please make that change no matter
what happens here with the content of the files.

 Recently there was was another discussion here about one file vs a collection 
 of files for tape statistics:
 
 http://marc.info/?l=linux-scsim=142316255501550w=2
 
 The result was that I should ask here what method I should use. I
 would like to get feedback in relation to tape statistics and one file
 vs multi-file in sysfs. I'm happy to keep the existing code or change
 to a single file approach.

One of the primary reasons we created sysfs and the one value per file
rule is that multi-value files just do not work well.  Yes, you get an
atomic snapshot, and you save some open/read/close syscall roundtrips,
but you do so at the expense of forcing userspace to know what the
format of the file is.  And once you create it, you can NEVER CHANGE IT
AGAIN.

Yes, that's right, if you come up with some new statistic in the future,
or realize that one of the ones you have now is wrong, you can't change
it, you have to make a whole new file, otherwise you could break
userspace tools.

Instead, a one-value-per-file rule forces userspace to know that if the
file is present, it can be opened and read from for a single value.  If
it isn't there, it should fail properly and move on to the next
statistic.  If you want to add a new statistic, great, just add a new
file and you are set.

You aren't dealing with performance-sensitive numbers here that have
to have an atomic snapshot of the state at a specific point in time in
order to work properly, so just have a bunch of files, all one value per
file, then all userspace tools will just work (i.e. libudev), and
everyone is happy.

And yes, open/read/close does take take a few extra cycles, but you
can't really measure it for a virtual filesystem like this on any modern
system.

Hope this helps explain why we have the sysfs rule, and why you should
continue to follow it as well.

Yes, it's not always followed, but that's usually because people forgot
why we had this rule, and no one noticed or pointed it out to me that it
was wrong.

thanks,

greg k-h
--
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


  1   2   3   >