Re: [PATCH v2] zfcp: fix reaction on bit error theshold notification with adapter close
On Tue, Oct 01, 2019 at 05:07:50PM +0200, Steffen Maier wrote: > On 10/1/19 4:14 PM, Greg KH wrote: > > On Tue, Oct 01, 2019 at 12:49:49PM +0200, Steffen Maier wrote: > > > On excessive bit errors for the FCP channel ingress fibre path, the > > > channel > > > notifies us. Previously, we only emitted a kernel message and a trace > > > record. > > > Since performance can become suboptimal with I/O timeouts due to > > > bit errors, we now stop using an FCP device by default on channel > > > notification so multipath on top can timely failover to other paths. > > > A new module parameter zfcp.ber_stop can be used to get zfcp old behavior. > > > > Ugh, module parameters? This isn't the 1990's anymore :( > > > > Why not just make this a dynamic sysfs variable, that way you properly > > can set this on whatever device you want, not just "all or nothing"? > > Since we can see many more (virtual) FCP devices than we want to actually > use, we defer probing. It means, we only start allocating structures and > sysfs entries on setting an FCP "online" for the first time. Setting online > works through another sysfs attribute owned by our ccw bus code component > called "cio". IIRC, setting online does not emit a uevent. On setting > online, the (add) uevent of hot-/coldplug of an FCP device had already > happened, so we could not easily have end users craft udev rules to > automatically/persistently configure a new sysfs attribute (which is > FCP-device-specific and appears late) to disable the new code behavior. > > Not sure if that could ever become a problem for end users: Even if we were > to write into a new sysfs attribute, the attribute only appears during > setting online so this might race with starting to actually use the FCP > device with the new default behavior and could potentially disable I/O paths > before the sysfs attribute write could become effective to disable the new > behavor. Ok, then why make this a module option that you will have to support for the next 20+ years anyway if you feel this fix is the correct way that it should be done instead? module options are tough to manage and support, only add them as a very last thing, when all other options have been ruled out. thanks, greg k-h
Re: [PATCH v2] zfcp: fix reaction on bit error theshold notification with adapter close
On Tue, Oct 01, 2019 at 12:49:49PM +0200, Steffen Maier wrote: > On excessive bit errors for the FCP channel ingress fibre path, the channel > notifies us. Previously, we only emitted a kernel message and a trace record. > Since performance can become suboptimal with I/O timeouts due to > bit errors, we now stop using an FCP device by default on channel > notification so multipath on top can timely failover to other paths. > A new module parameter zfcp.ber_stop can be used to get zfcp old behavior. Ugh, module parameters? This isn't the 1990's anymore :( Why not just make this a dynamic sysfs variable, that way you properly can set this on whatever device you want, not just "all or nothing"? thanks, greg k-h
Re: Slow I/O on USB media after commit f664a3cc17b7d0a2bc3b3ab96181e1029b0ec0e6
On Fri, Sep 20, 2019 at 09:25:17AM +0200, Andrea Vai wrote: > Il giorno gio, 19/09/2019 alle 13.54 -0400, Alan Stern ha scritto: > > > > In general, USB flash drives should not be expected to work as well > > as > > an actual disk drive connected over USB. > > Ok, so I think I'll buy some different hardware. Would an SSD drive > (connected over USB) behave like a flash drive or like an "actual disk > drive" from this point of view? It all depends on the drive. Some are a lot better than others, and it's almost impossible to tell until you buy the thing and try it out :(
Re: [PATCH v3 07/20] sg: move header to uapi section
On Mon, Aug 12, 2019 at 07:24:51AM -0700, Christoph Hellwig wrote: > > diff --git a/include/uapi/scsi/sg.h b/include/uapi/scsi/sg.h > > new file mode 100644 > > index ..072b45bd732f > > --- /dev/null > > +++ b/include/uapi/scsi/sg.h > > @@ -0,0 +1,329 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > This needs the syscall noticed for uapi headers. FYI, what is our > stance of just adding that notice to headers newly moved to UAPI? Just add them. > Do we need agreement from everyone who touched the file? Or just > after we started the split and SPDX annotations, as in this case > this header used to be available to user programs? The license of the entire kernel source is pretty obvious that anything UABI related needs the dual license, so adding it is fine. If not, the scripts will notice and we will just end up generating a patch to fix it anyway :) thanks, greg k-h
Re: [PATCH] mpt3sas: Use 63-bit DMA addressing on SAS35 HBA
On Fri, Jul 26, 2019 at 06:00:57AM -0400, Suganath Prabu wrote: > Although SAS3 & SAS3.5 IT HBA controllers support > 64-bit DMA addressing, as per hardware design, > DMA address with all 64-bits set (0x-) > results in a firmware fault. > > Fix: > Driver will set 63-bit DMA mask to ensure the above address > will not be used. > > Signed-off-by: Suganath Prabu > --- > drivers/scsi/mpt3sas/mpt3sas_base.c | 12 +++- > 1 file changed, 7 insertions(+), 5 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 4.14] scsi:be2iscsi: Fix a kernel address leakage in be_main.c
On Tue, Apr 16, 2019 at 03:06:34PM +0800, Fuqian Huang wrote: > Outputting kernel addresses will reveal the locations of kernel code > and data. And there is no need to print the address of a global object > beiscsi_iscsi_transport in beiscsi_module_init. > This case is similar to CVE-2018-7273[1]. > Just remove the print statement. > > [1] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-7273 > > Signed-off-by: Fuqian Huang 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 RESEND] aacraid: Fix performance issue (QD) on logical drives
On Wed, Mar 13, 2019 at 04:21:24PM -0700, Sagar Biradar wrote: > Fix performance issue where the queue depth for SmartIOC logical > volumes is set to 1, and allow the usual logical volume code > to be executed > > Fixes: a052865fe2871a3888 (aacraid: Set correct Queue Depth for HBA1000 > RAW disks) > > Signed-off-by: Sagar Biradar > Reviewed-by: Dave Carroll > --- > drivers/scsi/aacraid/linit.c | 13 - > 1 file changed, 8 insertions(+), 5 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] usb: uas: fix usb subsystem hang after power off hub port
On Mon, Mar 04, 2019 at 02:08:54AM +, jacky@sony.com wrote: > > > This email is confidential and intended only for the use of the individual or > entity named above and may contain information that is privileged. If you are > not the intended recipient, you are notified that any dissemination, > distribution or copying of this email is strictly prohibited. If you have > received this email in error, please notify us immediately by return email or > telephone and destroy the original message. - This mail is sent via Sony Asia > Pacific Mail Gateway.. This footer requires me to delete this email as it is not compatible with doing Linux kernel development. greg k-h
Re: [PATCH] mpt3sas: Use driver scsi lookup to track outstanding IOs
On Tue, Feb 19, 2019 at 07:02:46AM -0500, Sreekanth Reddy wrote: > Use driver's scsiio lookup table to track outstanding IOs at driver level, And why is this a stable kernel patch? You do not have a "Fixes:" tag in here. It really looks like a new feature :( greg k-h
Re: Request for opinion: ufsutils
On Wed, Feb 13, 2019 at 05:00:51PM +, Pedro Sousa wrote: > Hi, > > On 2/13/2019 9:30 AM, Avri Altman wrote: > >> > >> I can see the issue, there is no straightforward way of having "ufs-bsg" > >> without > >> a device attached? > > Right. > > > >> > >> This or any other method of talking with UniPro and MPhy layers when there > >> is no > >> device are necessary for us. Do you see some good way of doing this? > > I don't really know. > > If you have access to the bootloader, then implement it there, > > And call this functionality via fastboot getvar or fastboot oem. > For us would be better if we manage to use it through the driver. > > Until now, from this thread I think there are two scenarios: > > When there's a device, through ufs-bsg: > - ufs-utils: using the current ufs_bsg interface will have the capability to > cover most of the scenarios that I can think of and will be the standard way > to > interact with the ufs driver. > > During bring up/debug without link to device: > > - My proposal is to create a debugFS interface: >uic-cmd/ >├── dme-get >├── dme-set >├── dme-enable >├── dme-reset >└── ... >test-feature/ >├── PA_tf >└── ... >error-dump/ >├── UECPA >├── UECDL >└── ... >... > > I kindly request the ecosystem feedback. Don't ever put anything in debugfs that you need for a "real" system that you rely on, as it should not normally be present or mounted at all. It's for debugging stuff only, please be aware of that. greg k-h
Re: PROBLEM: syzkaller found / pool corruption-overwrite / page in user-area or NULL
On Thu, Jan 10, 2019 at 07:05:26PM +, esploit wrote: > Hi, I've been getting more into Kernel stuff lately and forged ahead > with some syzkaller bug finding. for syzkaller stuff, no need to cc: the security mailing list. Just work with the respective subsystem maintainers and developers (like you properly cc:ed) and all should be fine. thanks! greg k-h
Re: [PATCH 2/2] scsi: ufs: add inline crypto support to UFS HCD
On Tue, Dec 11, 2018 at 09:50:27AM +, Parshuram Thombare wrote:
> Add real time crypto support to UFS HCD using new device
> mapper 'crypto-ufs'. dmsetup tool can be used to enable
> real time / inline crypto support using device mapper
> 'crypt-ufs'.
>
> Signed-off-by: Parshuram Thombare
As you cc:ed me, I'll provide some minor review comments:
> +config BLK_DEV_HW_RT_ENCRYPTION
> + bool
> + depends on SCSI_UFSHCD_RT_ENCRYPTION
> + default n
n is always the default, you never need to list that.
> +
> source block/Kconfig.iosched
> diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
> index 2ddbb26..09a3ec0 100644
> --- a/drivers/scsi/ufs/Kconfig
> +++ b/drivers/scsi/ufs/Kconfig
> @@ -136,3 +136,15 @@ config SCSI_UFS_BSG
>
> Select this if you need a bsg device node for your UFS controller.
> If unsure, say N.
> +
> +config SCSI_UFSHCD_RT_ENCRYPTION
> + bool "Universal Flash Storage Controller RT encryption support"
> + depends on SCSI_UFSHCD
> + default n
Same here.
> + select BLK_DEV_HW_RT_ENCRYPTION if SCSI_UFSHCD_RT_ENCRYPTION
> + select BLK_DEV_DM if SCSI_UFSHCD_RT_ENCRYPTION
> + help
> + Universal Flash Storage Controller RT encryption support
> +
> + Select this if you want to enable real time encryption on UFS controller
> + If unsure, say N.
Don't you need to indent the help lines?
> +int ufshcd_crypto_init(struct ufs_hba *hba);
> +void ufshcd_crypto_remove(struct ufs_hba *hba);
> +void ufshcd_prepare_for_crypto(struct ufs_hba *hba, struct ufshcd_lrb *lrbp);
> +#endif
You need to provide inline functions for when your config option is not
enabled here.
That way you don't have to do this mess:
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 86fe114..a96b038 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -47,6 +47,9 @@
> #include "unipro.h"
> #include "ufs-sysfs.h"
> #include "ufs_bsg.h"
> +#ifdef CONFIG_SCSI_UFSHCD_RT_ENCRYPTION
> +#include "ufshcd-crypto.h"
> +#endif
No need for #ifdefs in .c files at all. Always include the .h file, and
then since your functions are all inline void functions, the code just
compiles away into nothing.
>
> #define CREATE_TRACE_POINTS
> #include
> @@ -2198,6 +2201,14 @@ static void ufshcd_prepare_req_desc_hdr(struct
> ufshcd_lrb *lrbp,
>
> dword_0 = data_direction | (lrbp->command_type
> << UPIU_COMMAND_TYPE_OFFSET);
> +#ifdef CONFIG_SCSI_UFSHCD_RT_ENCRYPTION
> + if (lrbp->cci >= 0) {
> + dword_0 |= (1 << CRYPTO_UTP_REQ_DESC_DWORD0_CE_SHIFT);
> + dword_0 |= ((lrbp->cci << CRYPTO_UTP_REQ_DESC_DWORD0_CCI_SHIFT)
> + & CRYPTO_UTP_REQ_DESC_DWORD0_CCI_MASK);
> + } else
> + dword_0 &= ~CRYPTO_UTP_REQ_DESC_DWORD0_CE_MASK;
> +#endif
Some comments on what this is trying to do here?
> if (lrbp->intr_cmd)
> dword_0 |= UTP_REQ_DESC_INT_CMD;
>
> @@ -2462,6 +2473,12 @@ static int ufshcd_queuecommand(struct Scsi_Host *host,
> struct scsi_cmnd *cmd)
> lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba) ? true : false;
> lrbp->req_abort_skip = false;
>
> +#ifdef CONFIG_SCSI_UFSHCD_RT_ENCRYPTION
> + lrbp->cci = -1;
What does -1 mean? You should have a type for it if it is something
"special".
> + /* prepare block for crypto */
> + if (hba->capabilities & MASK_CRYPTO_SUPPORT)
> + ufshcd_prepare_for_crypto(hba, lrbp);
> +#endif
Again, no #ifdefs needed please.
> ufshcd_comp_scsi_upiu(hba, lrbp);
>
> err = ufshcd_map_sg(hba, lrbp);
> @@ -8119,6 +8136,11 @@ void ufshcd_remove(struct ufs_hba *hba)
> ufs_bsg_remove(hba);
> ufs_sysfs_remove_nodes(hba->dev);
> scsi_remove_host(hba->host);
> +#ifdef CONFIG_SCSI_UFSHCD_RT_ENCRYPTION
> + if (hba->capabilities & MASK_CRYPTO_SUPPORT)
> + ufshcd_crypto_remove(hba);
> +#endif
Same ifdef here, and everywhere else in this file.
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -193,6 +193,9 @@ struct ufshcd_lrb {
> ktime_t compl_time_stamp;
>
> bool req_abort_skip;
> +#ifdef CONFIG_SCSI_UFSHCD_RT_ENCRYPTION
> + int cci;
> +#endif
No need for a #ifdef here. But comment on exactly what "cci" means,
that seems to not make any sense to me.
> };
>
> /**
> @@ -706,6 +709,9 @@ struct ufs_hba {
>
> struct device bsg_dev;
> struct request_queue*bsg_queue;
> +#ifdef CONFIG_SCSI_UFSHCD_RT_ENCRYPTION
> + struct ufshcd_crypto_ctx *cctx;
> +#endif
or here.
> };
>
> /* Returns true if clocks can be gated. Otherwise false */
> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> index 6fa889d..fe5a92d 100644
> --- a/drivers/scsi/ufs/ufshci.h
> +++ b/drivers/scsi/ufs/ufshci.h
> @@ -90,6 +90,7 @@ enum {
> MASK_64_ADDRESSING_SUPPORT = 0x0100,
> MAS
Re: [PATCH] scsi: libsas: Add missing license and update to SPDX license identifier
On Thu, Nov 29, 2018 at 01:11:10PM +0100, Greg KH wrote:
> On Thu, Nov 29, 2018 at 11:52:39AM +, John Garry wrote:
> > On 27/11/2018 15:23, John Garry wrote:
> > > On 27/11/2018 14:43, Greg KH wrote:
> > >
> > > Hi Greg,
> > >
> > > > On Tue, Nov 27, 2018 at 10:15:32PM +0800, John Garry wrote:
> > > > > Currently sas_task.c has no license specifier, so add SPDX license
> > > > > identifier for GPL-2.0+.
> > > > >
> > > > > As mentioned in commit b24413180f56 ("License cleanup: add SPDX
> > > > > GPL-2.0
> > > > > license identifier to files with no license"), files with no license
> > > > > in
> > > > > the kernel are under default kernel license.
> > > >
> > > > The default is GPLv2, not v2+.
> > >
> > > So sas_task.c should be v2.
> >
> > Hi Greg,
> >
> > I also note that currently we have an inconsistency in license of
> > sas_init.c:
> >
> > /*
> > * Serial Attached SCSI (SAS) Transport Layer initialization
> > *
> > * Copyright (C) 2005 Adaptec, Inc. All rights reserved.
> > * Copyright (C) 2005 Luben Tuikov
> > *
> > * This file is licensed under GPLv2.
> > *
> > * This program is free software; you can redistribute it and/or
> > * modify it under the terms of the GNU General Public License as
> > * published by the Free Software Foundation; either version 2 of the
> > * License, or (at your option) any later version.
> > *
> > * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> > * USA
> > *
> > */
> >
> > ...
> >
> > MODULE_AUTHOR("Luben Tuikov ");
> > MODULE_DESCRIPTION("SAS Transport Layer");
> > MODULE_LICENSE("GPL v2");
> >
> > So the license specifies v2+ but module license states v2.
> >
> > I could not find a docment for guidance on this. I also note that making
> > sas_task.c v2 would mean mixing v2 and v2+ into the module.
>
> This is not the only file in the kernel with this problem.
>
> For now, we have been trusting the "written text" lines over the
> MODULE_LICENSE() lines, as that seems to be the proper way forward.
>
> > I did find an example of someone changing the license:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/i2c/busses/i2c-designware-slave.c?h=v4.20-rc4&id=15c566fcff9cc7b8fd64461d6ee6fd1bc665b444
> >
>
> Yup, not good, that should be fixed.
I take it back, the changelog for the patch explains what is happening
here, the people involved were paying attention.
greg k-h
Re: [PATCH] scsi: libsas: Add missing license and update to SPDX license identifier
On Thu, Nov 29, 2018 at 11:52:39AM +, John Garry wrote:
> On 27/11/2018 15:23, John Garry wrote:
> > On 27/11/2018 14:43, Greg KH wrote:
> >
> > Hi Greg,
> >
> > > On Tue, Nov 27, 2018 at 10:15:32PM +0800, John Garry wrote:
> > > > Currently sas_task.c has no license specifier, so add SPDX license
> > > > identifier for GPL-2.0+.
> > > >
> > > > As mentioned in commit b24413180f56 ("License cleanup: add SPDX GPL-2.0
> > > > license identifier to files with no license"), files with no license in
> > > > the kernel are under default kernel license.
> > >
> > > The default is GPLv2, not v2+.
> >
> > So sas_task.c should be v2.
>
> Hi Greg,
>
> I also note that currently we have an inconsistency in license of
> sas_init.c:
>
> /*
> * Serial Attached SCSI (SAS) Transport Layer initialization
> *
> * Copyright (C) 2005 Adaptec, Inc. All rights reserved.
> * Copyright (C) 2005 Luben Tuikov
> *
> * This file is licensed under GPLv2.
> *
> * This program is free software; you can redistribute it and/or
> * modify it under the terms of the GNU General Public License as
> * published by the Free Software Foundation; either version 2 of the
> * License, or (at your option) any later version.
> *
> * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> * USA
> *
> */
>
> ...
>
> MODULE_AUTHOR("Luben Tuikov ");
> MODULE_DESCRIPTION("SAS Transport Layer");
> MODULE_LICENSE("GPL v2");
>
> So the license specifies v2+ but module license states v2.
>
> I could not find a docment for guidance on this. I also note that making
> sas_task.c v2 would mean mixing v2 and v2+ into the module.
This is not the only file in the kernel with this problem.
For now, we have been trusting the "written text" lines over the
MODULE_LICENSE() lines, as that seems to be the proper way forward.
> I did find an example of someone changing the license:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/i2c/busses/i2c-designware-slave.c?h=v4.20-rc4&id=15c566fcff9cc7b8fd64461d6ee6fd1bc665b444
>
Yup, not good, that should be fixed.
> Then someone changes the module license (but same company):
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/sound/soc/sh/rcar/core.c?h=v4.20-rc4&id=1e0edd4deadbbacd3b35179c233efa26624ab2af
That should be fine, the text says the correct one.
> At this point I'm reluctant to touch this in case I mess up, but there is
> still the missing license in sas_task.c .
Don't touch things like this unless you know _EXACTLY_ what you are
doing...
good luck!
greg k-h
Re: [PATCH] scsi: libsas: Add missing license and update to SPDX license identifier
On Tue, Nov 27, 2018 at 10:15:32PM +0800, John Garry wrote:
> Currently sas_task.c has no license specifier, so add SPDX license
> identifier for GPL-2.0+.
>
> As mentioned in commit b24413180f56 ("License cleanup: add SPDX GPL-2.0
> license identifier to files with no license"), files with no license in
> the kernel are under default kernel license.
The default is GPLv2, not v2+.
>
> While I'm at it, all other libsas source code files are updated to use
> SPDX license identifier for GPL-2.0+.
>
> Signed-off-by: John Garry
>
> diff --git a/drivers/scsi/libsas/Kconfig b/drivers/scsi/libsas/Kconfig
> index 13739bfa..bdc6bce 100644
> --- a/drivers/scsi/libsas/Kconfig
> +++ b/drivers/scsi/libsas/Kconfig
> @@ -4,22 +4,7 @@
> # Copyright (C) 2005 Adaptec, Inc. All rights reserved.
> # Copyright (C) 2005 Luben Tuikov
> #
> -# This file is licensed under GPLv2.
> -#
> -# 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 of the
> -# License.
> -#
> -# 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> -# USA
> +# SPDX-License-Identifier: GPL-2.0+
No, the above license is GPLv2 only, do NOT change the license of a file
unless you have permission to do so.
Also, the spdx line goes at the first line of the file.
> #
>
> config SCSI_SAS_LIBSAS
> diff --git a/drivers/scsi/libsas/Makefile b/drivers/scsi/libsas/Makefile
> index 5d51520..75998b7 100644
> --- a/drivers/scsi/libsas/Makefile
> +++ b/drivers/scsi/libsas/Makefile
> @@ -4,22 +4,7 @@
> # Copyright (C) 2005 Adaptec, Inc. All rights reserved.
> # Copyright (C) 2005 Luben Tuikov
> #
> -# This file is licensed under GPLv2.
> -#
> -# 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 of the
> -# License.
> -#
> -# 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> -# USA
> +# SPDX-License-Identifier: GPL-2.0+
Again you changed the license, not good :(
Please do not make these types of changes unless you really know what
you are doing, it is not ok to change the license of files.
greg k-h
Re: [PATCH v2] codafs: Fix build using bare-metal toolchain
On Mon, Oct 29, 2018 at 10:05:49PM +0200, Sam Protsenko wrote: > Hi Greg, > > On Mon, Oct 29, 2018 at 10:03 PM, Sam Protsenko > wrote: > > The kernel is self-contained project and can be built with bare-metal > > toolchain. But bare-metal toolchain doesn't define __linux__. Because of > > this u_quad_t type is not defined when using bare-metal toolchain and > > codafs build fails. This patch fixes it by defining u_quad_t type > > unconditionally. > > > > Signed-off-by: Sam Protsenko > > --- > > Can you please pull this one, if this applicable? I sent it a while > ago, but I guess it got lost in mailing list. It might be also > applicable to stable branch (as it fixes allmodconfig build for ARM > with bare-metal toolchain). Why are you asking me? I'm not the maintainer of this file :( confused, greg k-h
Re: [PATCH v2 3/7] drivers/base: Probe devices concurrently if requested by the driver
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
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] driver core: device: add BUS_ATTR_WO macro
On Tue, Oct 02, 2018 at 09:43:35AM +, Ioana Ciornei wrote: > > > Add BUS_ATTR_WO macro to make it easier to add attributes without > > > auditing the mode settings. Also, use the newly added macro where > > > appropriate. > > > > > > Signed-off-by: Ioana Ciornei > > > --- > > > arch/powerpc/platforms/pseries/ibmebus.c | 12 > > > drivers/block/rbd.c | 48 > > > > > > drivers/scsi/fcoe/fcoe_sysfs.c | 4 +-- > > > drivers/scsi/fcoe/fcoe_transport.c | 10 +++ > > > include/linux/device.h | 2 ++ > > > include/scsi/libfcoe.h | 8 +++--- > > > 6 files changed, 43 insertions(+), 41 deletions(-) > > > > Nice! This duplicates a lot of the work I did back in July but have not > > pushed out > > very far due to the other things that ended up happening around that time: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/log/?h=bus_cleanup > > > > > > As the patch series seen at that link shows, you can do this in more places > > than > > just what you did here. > > > > Either way, you should break this up into the individual patches, like I > > did or you > > can take my patches if you want. Getting the BUS_ATTR_WO() macro added is > > good to do now, and then you can go and hit up all of the different > > subsystems > > that should be converted over to it. > > I can of course split my patch into individual ones and resubmit them, but as > you already have the entire patch set ready, I feel like we can just push > those. I looked through your changes and it seems like you covered all the > subsystems. Please let me know if there is something else I should do. > > My intention here was to first add the _WO attribute so that afterwards I can > add a new bus attribute in the fsl-mc bus. Ok, I've queued up the patch that adds the _WO attribute now, so that you should be able to use this after 4.20-rc1. I'll work on getting my other patches merged in that series at that time as well. thanks, greg k-h
Re: Recent removal of bsg read/write support
On Thu, Oct 04, 2018 at 09:58:37AM +0300, Dror Levin wrote: > CC'ing Greg. > > On Mon, Sep 3, 2018 at 11:34 AM Dror Levin wrote: > > > > On Sun, Sep 2, 2018 at 8:55 PM Linus Torvalds > > wrote: > > > > > > On Sun, Sep 2, 2018 at 4:44 AM Richard Weinberger > > > wrote: > > > > > > > > CC'ing relevant people. Otherwise your mail might get lost. > > > > > > Indeed. > > > > Sorry for that. > > > > > > On Sun, Sep 2, 2018 at 1:37 PM Dror Levin wrote: > > > > > > > > > > We have an internal tool that uses the bsg read/write interface to > > > > > issue SCSI commands as part of a test suite for a storage device. > > > > > > > > > > After recently reading on LWN that this interface is to be removed we > > > > > tried porting our code to use sg instead. However, that raises new > > > > > issues - mainly getting ENOMEM over iSCSI for unknown reasons. > > > > > > Is there any chance that you can make more data available? > > > > Sure, I can try. > > > > We use writev() to send up to SG_MAX_QUEUE tasks at a time. Occasionally not > > all tasks are written at which point we wait for tasks to return before > > sending more, but then writev() fails with ENOMEM and we see this in the > > syslog: > > > > Sep 1 20:58:14 gdc-qa-io-017 kernel: sd 441:0:0:5: [sg73] > > sg_common_write: start_req err=-12 > > > > Failing tasks are reads of 128KiB. > > > > > I'd rather fix the sg interface (which while also broken garbage, we > > > can't get rid of) than re-surrect the bsg interface. > > Discussion seems to have died down but release of 4.19 is drawing near. > > Is there still any chance removal of bsg can be reconsidered? Maybe > postponed to the > next version to allow more time to adjust? > > I'm especially concerned about the possibility of this being > backported to stable kernels > which might leave us very little time to fix our code. What is being backported to what stable kernels and why? Is there sg patches? totally confused, greg k-h
Re: [PATCH] driver core: device: add BUS_ATTR_WO macro
On Mon, Oct 01, 2018 at 06:32:52PM +0300, Ioana Ciornei wrote: > Add BUS_ATTR_WO macro to make it easier to add attributes without > auditing the mode settings. Also, use the newly added macro where > appropriate. > > Signed-off-by: Ioana Ciornei > --- > arch/powerpc/platforms/pseries/ibmebus.c | 12 > drivers/block/rbd.c | 48 > > drivers/scsi/fcoe/fcoe_sysfs.c | 4 +-- > drivers/scsi/fcoe/fcoe_transport.c | 10 +++ > include/linux/device.h | 2 ++ > include/scsi/libfcoe.h | 8 +++--- > 6 files changed, 43 insertions(+), 41 deletions(-) Nice! This duplicates a lot of the work I did back in July but have not pushed out very far due to the other things that ended up happening around that time: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/log/?h=bus_cleanup As the patch series seen at that link shows, you can do this in more places than just what you did here. Either way, you should break this up into the individual patches, like I did or you can take my patches if you want. Getting the BUS_ATTR_WO() macro added is good to do now, and then you can go and hit up all of the different subsystems that should be converted over to it. thanks, greg k-h
Re: [PATCH 2/2] ata: Fix ZBC_OUT all bit handling
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
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)
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.
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()
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.
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 > > Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org; > > jaeg...@kernel.org; Alex Lemberg > > 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 > > > 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 > > 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.
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
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, &value)) \
> + 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
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
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.
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, &value))
> + 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 + curr_len), (PAGE_SIZE -
Re: [PATCH 10/18] qla2xxx: prevent bounds-check bypass via speculative execution
On Thu, Jan 11, 2018 at 02:15:12PM -0800, Dan Williams wrote: > On Sat, Jan 6, 2018 at 1:03 AM, 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... > > 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
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
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
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 > > Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org; Alex Lemberg > > > > 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
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
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
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 only. > + > +What: > /sys/bus/platform/drivers/ufshcd/*/attributes/reference_clock_frequency > +Date:August 2017 > +Contact: Stanislav Nijnikov > +Description: This file provides the reference clock frequency UFS device > + attribute.
Re: [PATCH v2 8/9] ufs: sysfs: flags
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 struct attribute_group
> ufs_sysfs_string_descriptors_group = {
> .attrs = ufs_sysfs_string_descriptors,
> };
>
> +#define ufs_sysfs_flag_show(_name, _uname)
> \
> +static ssize_t _name##_show(struct device *dev,
Re: [PATCH v2 7/9] ufs: sysfs: unit descriptor
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
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
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
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
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
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
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(&hba->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
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
On Tue, Dec 19, 2017 at 12:02:53PM -0800, Jaegeuk Kim wrote:
> From: Jaegeuk Kim
>
> This patch introduces attribute group to show existing sysfs entries.
>
> Cc: Greg KH
> Signed-off-by: Jaegeuk Kim
> ---
> 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(&hba->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, &hba->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(&hba->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, &hba->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[] = {
> + &dev_attr_rpm_lvl.attr,
> + &dev_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(&hba->dev->kobj, &ufshcd_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
Re: [PATCH] driver core: Make it safe to use get_device() if the reference count is zero
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
> > > > > 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(&dev->kobj)) : NULL;
> > > > > + return dev && kobject_get_unless_zero(&dev->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
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
> > > 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(&dev->kobj)) : NULL;
> > > + return dev && kobject_get_unless_zero(&dev->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
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(&dev->kobj)) : NULL;
> + return dev && kobject_get_unless_zero(&dev->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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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" > > > > > > 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 > 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
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
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.
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.
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)
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
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:
> (&shost->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 (&shost->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(&shost->scan_mutex);
>lock(s_active#336);
> lock(&shost->scan_mutex);
>
> *** DEADLOCK ***
> 3 locks held by test_02_sdev_de/12586:
> #0: (sb_writers#4){.+.+.+}, at: [] vfs_write+0x178/0x190
> #1: (&of->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(&kernfs_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
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
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
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
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
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
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
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
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
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
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
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
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 > >> > > >> >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
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 i
Re: raid 5 creation fails on usb3 connected drives kernel 4.4.x, 4.5
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 > > > 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 > > > Acked-by: Jens Axboe > > > Reviewed-by: Hannes Reinecke > > > Signed-off-by: James Bottomley > > > > > > > > > > > > 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&m=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
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 > 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 > Acked-by: Jens Axboe > Reviewed-by: Hannes Reinecke > Signed-off-by: James Bottomley > > > > 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
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
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(&hw->lock); > > > > > > > > > > - csio_unreg_rnode(rn); > > > > > > > > > > spin_lock_irq(&hw->lock); > > > > > > > > > > + csio_unreg_rnode(rn); > > > > > > > > > > + spin_unlock_irq(&hw->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 forwarded but the >
Re: [PATCH v7] scsi: ufs: add ioctl interface for query request
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 this code, let's just drop it. Feel free to revisit it sometime in the future if you ever get ha
Re: [PATCH v7] scsi: ufs: add ioctl interface for query request
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
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
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
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
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
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
