[PATCH 4/4] sg: use standard lists for sg_requests
'Sg_request' is using a private list implementation; convert it
to standard lists.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/sg.c | 107 ++
1 file changed, 44 insertions(+), 63 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 3599551..9b0429d 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -122,7 +122,7 @@
struct sg_fd;
typedef struct sg_request {/* SG_MAX_QUEUE requests outstanding per file */
- struct sg_request *nextrp; /* NULL -> tail request (slist) */
+ struct list_head nextrp;/* list entry */
struct sg_fd *parentfp; /* NULL -> not in use */
Sg_scatter_hold data; /* hold buffer, perhaps scatter list */
sg_io_hdr_t header; /* scsi command+info, see */
@@ -146,7 +146,7 @@
int timeout_user; /* defaults to SG_DEFAULT_TIMEOUT_USER */
Sg_scatter_hold reserve;/* buffer held for this file descriptor
*/
unsigned save_scat_len; /* original length of trunc. scat. element */
- Sg_request *headrp; /* head of request slist, NULL->empty */
+ struct list_head rq_list; /* head of request list */
struct fasync_struct *async_qp; /* used by asynchronous notification */
Sg_request req_arr[SG_MAX_QUEUE]; /* used as singly-linked list */
char force_packid; /* 1 -> pack_id input to read(), 0 -> ignored */
@@ -942,7 +942,7 @@ static int max_sectors_bytes(struct request_queue *q)
if (!access_ok(VERIFY_WRITE, ip, sizeof (int)))
return -EFAULT;
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- for (srp = sfp->headrp; srp; srp = srp->nextrp) {
+ list_for_each_entry(srp, &sfp->rq_list, nextrp) {
if ((1 == srp->done) && (!srp->sg_io_owned)) {
read_unlock_irqrestore(&sfp->rq_list_lock,
iflags);
@@ -955,7 +955,8 @@ static int max_sectors_bytes(struct request_queue *q)
return 0;
case SG_GET_NUM_WAITING:
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- for (val = 0, srp = sfp->headrp; srp; srp = srp->nextrp) {
+ val = 0;
+ list_for_each_entry(srp, &sfp->rq_list, nextrp) {
if ((1 == srp->done) && (!srp->sg_io_owned))
++val;
}
@@ -1026,35 +1027,33 @@ static int max_sectors_bytes(struct request_queue *q)
if (!rinfo)
return -ENOMEM;
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- for (srp = sfp->headrp, val = 0; val < SG_MAX_QUEUE;
-++val, srp = srp ? srp->nextrp : srp) {
+ val = 0;
+ list_for_each_entry(srp, &sfp->rq_list, nextrp) {
+ if (val > SG_MAX_QUEUE)
+ break;
memset(&rinfo[val], 0, SZ_SG_REQ_INFO);
- if (srp) {
- rinfo[val].req_state = srp->done + 1;
- rinfo[val].problem =
- srp->header.masked_status &
- srp->header.host_status &
- srp->header.driver_status;
- if (srp->done)
- rinfo[val].duration =
- srp->header.duration;
- else {
- ms = jiffies_to_msecs(jiffies);
- rinfo[val].duration =
- (ms > srp->header.duration)
?
- (ms - srp->header.duration)
: 0;
- }
- rinfo[val].orphan = srp->orphan;
- rinfo[val].sg_io_owned =
- srp->sg_io_owned;
- rinfo[val].pack_id =
- srp->header.pack_id;
- rinfo[val].usr_ptr =
- srp->header.usr_ptr;
+ rinfo[val].req_state = srp->done + 1;
+ rinfo[val].problem =
+ srp->header.masked_status &
+ srp->header.host_status &
+ srp->header.driver_s
[PATCH 0/4] sanitize sg
Hi all, the infamous syzkaller incovered some more issues in the sg driver. This patchset fixes those two issues (and adds a fix for yet another potential issue; checking for a NULL dxferp when dxfer_len is not 0). It also removes handling of the SET_FORCE_LOW_DMA ioctl, which never worked since the initial git checkin. And does some code cleanup by removing the private list implementation, using standard lists instead. As usual, comments and reviews are welcome. Hannes Reinecke (3): sg: disable SET_FORCE_LOW_DMA sg: protect access to to 'reserved' page array sg: use standard lists for sg_requests Johannes Thumshirn (1): sg: check for valid direction before starting the request drivers/scsi/sg.c | 183 -- include/scsi/sg.h | 1 - 2 files changed, 80 insertions(+), 104 deletions(-) -- 1.8.5.6
[PATCH 1/4] sg: disable SET_FORCE_LOW_DMA
The ioctl SET_FORCE_LOW_DMA has never worked since the initial
git check-in, and the respective setting is nowadays handled
correctly.
So disable it entirely.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/sg.c | 27 +--
include/scsi/sg.h | 1 -
2 files changed, 5 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index dbe5b4b..8a959b2 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -149,7 +149,6 @@
Sg_request *headrp; /* head of request slist, NULL->empty */
struct fasync_struct *async_qp; /* used by asynchronous notification */
Sg_request req_arr[SG_MAX_QUEUE]; /* used as singly-linked list */
- char low_dma; /* as in parent but possibly overridden to 1 */
char force_packid; /* 1 -> pack_id input to read(), 0 -> ignored */
char cmd_q; /* 1 -> allow command queuing, 0 -> don't */
unsigned char next_cmd_len; /* 0: automatic, >0: use on next write() */
@@ -887,24 +886,9 @@ static int max_sectors_bytes(struct request_queue *q)
/* strange ..., for backward compatibility */
return sfp->timeout_user;
case SG_SET_FORCE_LOW_DMA:
- result = get_user(val, ip);
- if (result)
- return result;
- if (val) {
- sfp->low_dma = 1;
- if ((0 == sfp->low_dma) && (0 == sg_res_in_use(sfp))) {
- val = (int) sfp->reserve.bufflen;
- sg_remove_scat(sfp, &sfp->reserve);
- sg_build_reserve(sfp, val);
- }
- } else {
- if (atomic_read(&sdp->detaching))
- return -ENODEV;
- sfp->low_dma = sdp->device->host->unchecked_isa_dma;
- }
- return 0;
+ return -EINVAL;
case SG_GET_LOW_DMA:
- return put_user((int) sfp->low_dma, ip);
+ return put_user((int) sdp->device->host->unchecked_isa_dma, ip);
case SG_GET_SCSI_ID:
if (!access_ok(VERIFY_WRITE, p, sizeof (sg_scsi_id_t)))
return -EFAULT;
@@ -1821,6 +1805,7 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
int sg_tablesize = sfp->parentdp->sg_tablesize;
int blk_size = buff_size, order;
gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN;
+ struct sg_device *sdp = sfp->parentdp;
if (blk_size < 0)
return -EFAULT;
@@ -1846,7 +1831,7 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
scatter_elem_sz_prev = num;
}
- if (sfp->low_dma)
+ if (sdp->device->host->unchecked_isa_dma)
gfp_mask |= GFP_DMA;
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
@@ -2132,8 +2117,6 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
sfp->timeout = SG_DEFAULT_TIMEOUT;
sfp->timeout_user = SG_DEFAULT_TIMEOUT_USER;
sfp->force_packid = SG_DEF_FORCE_PACK_ID;
- sfp->low_dma = (SG_DEF_FORCE_LOW_DMA == 0) ?
- sdp->device->host->unchecked_isa_dma : 1;
sfp->cmd_q = SG_DEF_COMMAND_Q;
sfp->keep_orphan = SG_DEF_KEEP_ORPHAN;
sfp->parentdp = sdp;
@@ -2603,7 +2586,7 @@ static void sg_proc_debug_helper(struct seq_file *s,
Sg_device * sdp)
jiffies_to_msecs(fp->timeout),
fp->reserve.bufflen,
(int) fp->reserve.k_use_sg,
- (int) fp->low_dma);
+ (int) sdp->device->host->unchecked_isa_dma);
seq_printf(s, " cmd_q=%d f_packid=%d k_orphan=%d closed=0\n",
(int) fp->cmd_q, (int) fp->force_packid,
(int) fp->keep_orphan);
diff --git a/include/scsi/sg.h b/include/scsi/sg.h
index 3afec70..20bc71c 100644
--- a/include/scsi/sg.h
+++ b/include/scsi/sg.h
@@ -197,7 +197,6 @@
#define SG_DEFAULT_RETRIES 0
/* Defaults, commented if they differ from original sg driver */
-#define SG_DEF_FORCE_LOW_DMA 0 /* was 1 -> memory below 16MB on i386 */
#define SG_DEF_FORCE_PACK_ID 0
#define SG_DEF_KEEP_ORPHAN 0
#define SG_DEF_RESERVED_SIZE SG_SCATTER_SZ /* load time option */
--
1.8.5.6
[PATCH 3/4] sg: check for valid direction before starting the request
From: Johannes Thumshirn
Check for a valid direction before starting the request, otherwise we risk
running into an assertion in the scsi midlayer checking for vaild requests.
Signed-off-by: Johannes Thumshirn
Link: http://www.spinics.net/lists/linux-scsi/msg104400.html
Reported-by: Dmitry Vyukov
Reviewed-by: Hannes Reinecke
---
drivers/scsi/sg.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index c29962c..3599551 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -752,6 +752,20 @@ static int sg_allow_access(struct file *filp, unsigned
char *cmd)
return count;
}
+static bool sg_is_valid_direction(int dxfer_direction)
+{
+ switch (dxfer_direction) {
+ case SG_DXFER_NONE:
+ case SG_DXFER_TO_DEV:
+ case SG_DXFER_FROM_DEV:
+ case SG_DXFER_TO_FROM_DEV:
+ case SG_DXFER_UNKNOWN:
+ return true;
+ default:
+ return false;
+ }
+}
+
static int
sg_common_write(Sg_fd * sfp, Sg_request * srp,
unsigned char *cmnd, int timeout, int blocking)
@@ -772,6 +786,11 @@ static int sg_allow_access(struct file *filp, unsigned
char *cmd)
"sg_common_write: scsi opcode=0x%02x, cmd_size=%d\n",
(int) cmnd[0], (int) hp->cmd_len));
+ if (!sg_is_valid_direction(hp->dxfer_direction))
+ return -EINVAL;
+ if (hp->dxferp == NULL && hp->dxfer_len > 0)
+ return -EINVAL;
+
k = sg_start_req(srp, cmnd);
if (k) {
SCSI_LOG_TIMEOUT(1, sg_printk(KERN_INFO, sfp->parentdp,
--
1.8.5.6
[PATCH 2/4] sg: protect access to to 'reserved' page array
The 'reserved' page array is used as a short-cut for mapping
data, saving us to allocate pages per request.
However, the 'reserved' array is only capable of holding one
request, so we need to protect it against concurrent accesses.
Cc: sta...@vger.kernel.org
Reported-by: Dmitry Vyukov
Link: http://www.spinics.net/lists/linux-scsi/msg104326.html
Signed-off-by: Hannes Reinecke
Tested-by: Johannes Thumshirn
---
drivers/scsi/sg.c | 30 --
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 8a959b2..c29962c 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -154,6 +154,8 @@
unsigned char next_cmd_len; /* 0: automatic, >0: use on next write() */
char keep_orphan; /* 0 -> drop orphan (def), 1 -> keep for read()
*/
char mmap_called; /* 0 -> mmap() never called on this fd */
+ unsigned long flags;
+#define SG_RESERVED_IN_USE 1
struct kref f_ref;
struct execute_work ew;
} Sg_fd;
@@ -197,7 +199,6 @@ static int sg_common_write(Sg_fd * sfp, Sg_request * srp,
static Sg_request *sg_get_rq_mark(Sg_fd * sfp, int pack_id);
static Sg_request *sg_add_request(Sg_fd * sfp);
static int sg_remove_request(Sg_fd * sfp, Sg_request * srp);
-static int sg_res_in_use(Sg_fd * sfp);
static Sg_device *sg_get_dev(int dev);
static void sg_device_destroy(struct kref *kref);
@@ -720,7 +721,7 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
sg_remove_request(sfp, srp);
return -EINVAL; /* either MMAP_IO or DIRECT_IO (not
both) */
}
- if (sg_res_in_use(sfp)) {
+ if (test_bit(SG_RESERVED_IN_USE, &sfp->flags)) {
sg_remove_request(sfp, srp);
return -EBUSY; /* reserve buffer already being used */
}
@@ -952,10 +953,14 @@ static int max_sectors_bytes(struct request_queue *q)
val = min_t(int, val,
max_sectors_bytes(sdp->device->request_queue));
if (val != sfp->reserve.bufflen) {
- if (sg_res_in_use(sfp) || sfp->mmap_called)
+ if (sfp->mmap_called)
+ return -EBUSY;
+ if (test_and_set_bit(SG_RESERVED_IN_USE, &sfp->flags))
return -EBUSY;
+
sg_remove_scat(sfp, &sfp->reserve);
sg_build_reserve(sfp, val);
+ clear_bit(SG_RESERVED_IN_USE, &sfp->flags);
}
return 0;
case SG_GET_RESERVED_SIZE:
@@ -1709,7 +1714,9 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
md = &map_data;
if (md) {
- if (!sg_res_in_use(sfp) && dxfer_len <= rsv_schp->bufflen)
+ if (dxfer_len <= rsv_schp->bufflen &&
+ test_and_set_bit(SG_RESERVED_IN_USE,
+&sfp->flags) == 0)
sg_link_reserve(sfp, srp, dxfer_len);
else {
res = sg_build_indirect(req_schp, sfp, dxfer_len);
@@ -2003,6 +2010,7 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
req_schp->sglist_len = 0;
sfp->save_scat_len = 0;
srp->res_used = 0;
+ clear_bit(SG_RESERVED_IN_USE, &sfp->flags);
}
static Sg_request *
@@ -2187,20 +2195,6 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
schedule_work(&sfp->ew.work);
}
-static int
-sg_res_in_use(Sg_fd * sfp)
-{
- const Sg_request *srp;
- unsigned long iflags;
-
- read_lock_irqsave(&sfp->rq_list_lock, iflags);
- for (srp = sfp->headrp; srp; srp = srp->nextrp)
- if (srp->res_used)
- break;
- read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
- return srp ? 1 : 0;
-}
-
#ifdef CONFIG_SCSI_PROC_FS
static int
sg_idr_max_id(int id, void *p, void *data)
--
1.8.5.6
Re: [PATCH V4 08/24] aacraid: Added support for response path
On 02/03/2017 12:53 AM, Raghava Aditya Renukunta wrote: This patch enables the driver to actually process the I/O, or srb replies from adapter. In addition to any HBA1000 or SmartIOC2000 adapter events. Signed-off-by: Raghava Aditya Renukunta Signed-off-by: Dave Carroll --- Reviewed-by: Johannes Thumshirn
Re: [PATCH V4 10/24] aacraid: Reworked aac_command_thread
On 02/03/2017 12:53 AM, Raghava Aditya Renukunta wrote: Reworked aac_command_thread into aac_process_events Signed-off-by: Raghava Aditya Renukunta Signed-off-by: Dave Carroll --- Reviewed-by: Johannes Thumshirn
Re: [PATCH V4 11/24] aacraid: Added support for periodic wellness sync
On 02/03/2017 12:53 AM, Raghava Aditya Renukunta wrote: This patch adds a new functions that periodically sync the time of host to the adapter. In addition also informs the adapter that the driver is alive and kicking. Only applicable to the HBA1000 and SMARTIOC2000. Signed-off-by: Raghava Aditya Renukunta Signed-off-by: Dave Carroll --- Reviewed-by: Johannes Thumshirn
Re: [PATCH V4 12/24] aacraid: Retrieve Queue Depth from Adapter FW
On 02/03/2017 12:53 AM, Raghava Aditya Renukunta wrote: Retrieved queue depth from fw and saved it for future use. Only applicable for HBA1000 drives. Signed-off-by: Raghava Aditya Renukunta Signed-off-by: Dave Carroll --- Reviewed-by: Johannes Thumshirn
Re: [PATCH V4 13/24] aacraid: Added support to set QD of attached drives
On 02/03/2017 12:53 AM, Raghava Aditya Renukunta wrote: Added support to set qd of drives in slave_configure.This only works for HBA1000 attached drives. Signed-off-by: Raghava Aditya Renukunta Signed-off-by: Dave Carroll --- Reviewed-by: Johannes Thumshirn
Re: [PATCH V4 14/24] aacraid: Added support for hotplug
On 02/03/2017 12:53 AM, Raghava Aditya Renukunta wrote: Added support for drive hotplug add and removal Signed-off-by: Raghava Aditya Renukunta Signed-off-by: Dave Carroll --- Reviewed-by: Johannes Thumshirn
Re: [PATCH v5 02/11] phy: exynos-ufs: add UFS PHY driver for EXYNOS SoC
Hi Kishon,
On 11/19/2015 07:09 PM, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 17 November 2015 01:41 PM, Alim Akhtar wrote:
Hi
Thanks again for looking into this.
On 11/17/2015 11:46 AM, Kishon Vijay Abraham I wrote:
Hi,
On Monday 09 November 2015 10:56 AM, Alim Akhtar wrote:
From: Seungwon Jeon
This patch introduces Exynos UFS PHY driver. This driver
supports to deal with phy calibration and power control
according to UFS host driver's behavior.
Signed-off-by: Seungwon Jeon
Signed-off-by: Alim Akhtar
Cc: Kishon Vijay Abraham I
---
drivers/phy/Kconfig|7 ++
drivers/phy/Makefile |1 +
drivers/phy/phy-exynos-ufs.c | 241
drivers/phy/phy-exynos-ufs.h | 85 +
drivers/phy/phy-exynos7-ufs.h | 89 +
include/linux/phy/phy-exynos-ufs.h | 85 +
6 files changed, 508 insertions(+)
create mode 100644 drivers/phy/phy-exynos-ufs.c
create mode 100644 drivers/phy/phy-exynos-ufs.h
create mode 100644 drivers/phy/phy-exynos7-ufs.h
create mode 100644 include/linux/phy/phy-exynos-ufs.h
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 7eb5859dd035..7d38a92e0297 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -389,4 +389,11 @@ config PHY_CYGNUS_PCIE
Enable this to support the Broadcom Cygnus PCIe PHY.
If unsure, say N.
+config PHY_EXYNOS_UFS
+tristate "EXYNOS SoC series UFS PHY driver"
+depends on OF && ARCH_EXYNOS || COMPILE_TEST
+select GENERIC_PHY
+help
+ Support for UFS PHY on Samsung EXYNOS chipsets.
+
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 075db1a81aa5..9bec4d1a89e1 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)+=
phy-armada375-usb2.o
obj-$(CONFIG_BCM_KONA_USB2_PHY)+= phy-bcm-kona-usb2.o
obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)+= phy-exynos-dp-video.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)+= phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_EXYNOS_UFS)+= phy-exynos-ufs.o
obj-$(CONFIG_PHY_LPC18XX_USB_OTG)+= phy-lpc18xx-usb-otg.o
obj-$(CONFIG_PHY_PXA_28NM_USB2)+= phy-pxa-28nm-usb2.o
obj-$(CONFIG_PHY_PXA_28NM_HSIC)+= phy-pxa-28nm-hsic.o
diff --git a/drivers/phy/phy-exynos-ufs.c b/drivers/phy/phy-exynos-ufs.c
new file mode 100644
index ..cb1aeaa3d4eb
--- /dev/null
+++ b/drivers/phy/phy-exynos-ufs.c
@@ -0,0 +1,241 @@
+/*
+ * UFS PHY driver for Samsung EXYNOS SoC
+ *
+ * Copyright (C) 2015 Samsung Electronics Co., Ltd.
+ * Author: Seungwon Jeon
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "phy-exynos-ufs.h"
+
+#define for_each_phy_lane(phy, i) \
+for (i = 0; i < (phy)->lane_cnt; i++)
+#define for_each_phy_cfg(cfg) \
+for (; (cfg)->id; (cfg)++)
+
+#define PHY_DEF_LANE_CNT1
+
+static void exynos_ufs_phy_config(struct exynos_ufs_phy *phy,
+const struct exynos_ufs_phy_cfg *cfg, u8 lane)
+{
+enum {LANE_0, LANE_1}; /* lane index */
+
+switch (lane) {
+case LANE_0:
+writel(cfg->val, (phy)->reg_pma + cfg->off_0);
+break;
+case LANE_1:
+if (cfg->id == PHY_TRSV_BLK)
+writel(cfg->val, (phy)->reg_pma + cfg->off_1);
+break;
+}
+}
+
+static bool match_cfg_to_pwr_mode(u8 desc, u8 required_pwr)
+{
+if (IS_PWR_MODE_ANY(desc))
+return true;
+
+if (IS_PWR_MODE_HS(required_pwr) && IS_PWR_MODE_HS_ANY(desc))
+return true;
+
+if (COMP_PWR_MODE(required_pwr, desc))
+return true;
+
+if (COMP_PWR_MODE_MD(required_pwr, desc) &&
+COMP_PWR_MODE_GEAR(required_pwr, desc) &&
+COMP_PWR_MODE_SER(required_pwr, desc))
+return true;
+
+return false;
+}
+
+int exynos_ufs_phy_calibrate(struct phy *phy,
+enum phy_cfg_tag tag, u8 pwr)
This is similar to the first version of your patch without EXPORT_SYMBOL.
I think you have to create a new generic PHY_OPS for calibrate PHY while making
sure that it is as generic as possible (which means calibrate_phy shouldn't
have tag and pwr arguments or a strong justification as to why those arguments
are required in a generic API).
I don't see the advantage to making this a generic phy_ops, this is exynos
specific ufs-phy, please have a look at other implementations
only the implementation is specific to exynos. I've seen lot of other vendors
want to do something like calibrate phy.
So if we add something like (*calibrate)(struct phy *phy), then it can be used
by others as well. Russell King also want to minimize t
Re: [PATCH V4 16/24] aacraid: Add task management functionality
On 02/03/2017 12:53 AM, Raghava Aditya Renukunta wrote:
Added support to send out task management commands.
Signed-off-by: Raghava Aditya Renukunta
Signed-off-by: Dave Carroll
---
[...]
Zapped C++ comment
[...]
+static int aac_adapter_hba(struct fib *fib, struct scsi_cmnd *cmd)
+{
+ struct aac_hba_cmd_req *hbacmd = aac_construct_hbacmd(fib, cmd);
+ struct aac_dev *dev;
+ // u16 fibsize;
Here you missed one ;-)
Anyways,
Reviewed-by: Johannes Thumshirn
Re: [PATCH V4 18/24] aacraid: VPD 83 type3 support
On 02/03/2017 12:53 AM, Raghava Aditya Renukunta wrote: This patch adds support to retrieve the unique identifier data (VPD page 83 type3) for Logical drives created on SmartIOC 2000 products. In addition added a sysfs device structure to expose the id information. Signed-off-by: Raghava Aditya Renukunta Signed-off-by: Dave Carroll --- Reviewed-by: Johannes Thumshirn
Re: [PATCH V4 19/24] aacraid: Added new IWBR reset
On 02/03/2017 12:53 AM, Raghava Aditya Renukunta wrote: Added a new IWBR soft reset type, reworked the IOP reset interface for a bit. Signed-off-by: Raghava Aditya Renukunta Signed-off-by: Dave Carroll --- Reviewed-by: Johannes Thumshirn
Re: [PATCH 1/4] sg: disable SET_FORCE_LOW_DMA
On 02/03/2017 09:54 AM, Hannes Reinecke wrote:
The ioctl SET_FORCE_LOW_DMA has never worked since the initial
git check-in, and the respective setting is nowadays handled
correctly.
So disable it entirely.
Signed-off-by: Hannes Reinecke
---
[...]
case SG_SET_FORCE_LOW_DMA:
- result = get_user(val, ip);
- if (result)
- return result;
- if (val) {
- sfp->low_dma = 1;
- if ((0 == sfp->low_dma) && (0 == sg_res_in_use(sfp))) {
- val = (int) sfp->reserve.bufflen;
- sg_remove_scat(sfp, &sfp->reserve);
- sg_build_reserve(sfp, val);
- }
- } else {
- if (atomic_read(&sdp->detaching))
- return -ENODEV;
- sfp->low_dma = sdp->device->host->unchecked_isa_dma;
- }
- return 0;
+ return -EINVAL;
I'm not sure if returning a bogus '0' is better here to not break any
existing application, which assumed it always worked.
Re: [PATCH 2/4] sg: protect access to to 'reserved' page array
On 02/03/2017 09:54 AM, Hannes Reinecke wrote: The 'reserved' page array is used as a short-cut for mapping data, saving us to allocate pages per request. However, the 'reserved' array is only capable of holding one request, so we need to protect it against concurrent accesses. Cc: sta...@vger.kernel.org Reported-by: Dmitry Vyukov Link: http://www.spinics.net/lists/linux-scsi/msg104326.html Signed-off-by: Hannes Reinecke Tested-by: Johannes Thumshirn --- Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH 4/4] sg: use standard lists for sg_requests
On 02/03/2017 09:54 AM, Hannes Reinecke wrote: 'Sg_request' is using a private list implementation; convert it to standard lists. Signed-off-by: Hannes Reinecke --- Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH 1/4] sg: disable SET_FORCE_LOW_DMA
On 02/03/2017 10:32 AM, Johannes Thumshirn wrote:
>
> On 02/03/2017 09:54 AM, Hannes Reinecke wrote:
>> The ioctl SET_FORCE_LOW_DMA has never worked since the initial
>> git check-in, and the respective setting is nowadays handled
>> correctly.
>> So disable it entirely.
>>
>> Signed-off-by: Hannes Reinecke
>> ---
>
> [...]
>
>> case SG_SET_FORCE_LOW_DMA:
>> -result = get_user(val, ip);
>> -if (result)
>> -return result;
>> -if (val) {
>> -sfp->low_dma = 1;
>> -if ((0 == sfp->low_dma) && (0 == sg_res_in_use(sfp))) {
>> -val = (int) sfp->reserve.bufflen;
>> -sg_remove_scat(sfp, &sfp->reserve);
>> -sg_build_reserve(sfp, val);
>> -}
>> -} else {
>> -if (atomic_read(&sdp->detaching))
>> -return -ENODEV;
>> -sfp->low_dma = sdp->device->host->unchecked_isa_dma;
>> -}
>> -return 0;
>> +return -EINVAL;
>
> I'm not sure if returning a bogus '0' is better here to not break any
> existing application, which assumed it always worked.
>
Well, the ioctl already was returning error numbers, so any calling
application had to protect against that.
And I don't really see the point to protect against failures in legacy
applications relying on a functionality which never did anything.
Cheers,
Hannes
--
Dr. Hannes ReineckezSeries & Storage
h...@suse.com +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH 1/4] sg: disable SET_FORCE_LOW_DMA
On Fri, Feb 03, 2017 at 10:32:28AM +0100, Johannes Thumshirn wrote: > I'm not sure if returning a bogus '0' is better here to not break any > existing application, which assumed it always worked. Agreed, this should return 0, and maybe also print a (ratelimited) warning.
Re: [PATCH 2/4] sg: protect access to to 'reserved' page array
On Fri, Feb 03, 2017 at 09:54:49AM +0100, Hannes Reinecke wrote: > The 'reserved' page array is used as a short-cut for mapping > data, saving us to allocate pages per request. > However, the 'reserved' array is only capable of holding one > request, so we need to protect it against concurrent accesses. Can you please explain how you protect the access here a bit more, as mentioned before the set_bit for exclusion trick is always suspicious, so the changelog needs to have a justification for it.
Re: [PATCH 3/4] sg: check for valid direction before starting the request
On Fri, Feb 03, 2017 at 09:54:50AM +0100, Hannes Reinecke wrote:
> From: Johannes Thumshirn
>
> Check for a valid direction before starting the request, otherwise we risk
> running into an assertion in the scsi midlayer checking for vaild requests.
Good idea, but..
> +static bool sg_is_valid_direction(int dxfer_direction)
> +{
> + switch (dxfer_direction) {
> + case SG_DXFER_NONE:
> + case SG_DXFER_TO_DEV:
> + case SG_DXFER_FROM_DEV:
> + case SG_DXFER_TO_FROM_DEV:
This isn't strictly valid as sg doesn't actually handle real bidi
commands, but we work around it.
It might be a good idea to move the warning about it from sg_write
to here and use printk_ratelimited instead of the hand-rolled
per-thread warning there.
> + case SG_DXFER_UNKNOWN:
> + return true;
And how valid is this one (Question to Doug I guess): for a 0-sized
transfer we should be fine, but otherwise it should probably be
rejected.
Re: [PATCH 4/4] sg: use standard lists for sg_requests
> typedef struct sg_request { /* SG_MAX_QUEUE requests outstanding per file */
> - struct sg_request *nextrp; /* NULL -> tail request (slist) */
> + struct list_head nextrp;/* list entry */
s/nextrp/entry/
> @@ -2078,16 +2076,13 @@ static long sg_compat_ioctl(struct file *filp,
> unsigned int cmd_in, unsigned lon
> if (k < SG_MAX_QUEUE) {
> memset(rp, 0, sizeof (Sg_request));
> rp->parentfp = sfp;
> + list_add(&rp->nextrp, &sfp->rq_list);
The old code did a tail insertation. And this whole function should
become a lot simpler with proper lists anyway:
static Sg_request *
sg_add_request(Sg_fd * sfp)
{
int k;
unsigned long iflags;
Sg_request *rp = sfp->req_arr;
write_lock_irqsave(&sfp->rq_list_lock, iflags);
if (!list_empty(&sfp->rq_list)) {
if (!sfp->cmd_q)
goto out_unlock;
for (k = 0; k < SG_MAX_QUEUE; ++k, ++rp) {
if (!rp->parentfp)
break;
}
if (k >= SG_MAX_QUEUE)
goto out_unlock;
}
memset(rp, 0, sizeof (Sg_request));
rp->parentfp = sfp;
rp->header.duration = jiffies_to_msecs(jiffies);
list_add_tail(&rp->nextrp, &sfp->rq_list);
write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
return rp;
out_unlock:
write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
return NULL;
> + if ((!sfp) || (!srp) || (list_empty(&sfp->rq_list)))
No need for all these braces.
> + if (!list_empty(&srp->nextrp)) {
> + list_del_init(&srp->nextrp);
I don't think we need the _init as we never check for an empty entry.
> {
> struct sg_fd *sfp = container_of(work, struct sg_fd, ew.work);
> struct sg_device *sdp = sfp->parentdp;
> + Sg_request *srp, *tmp;
>
> /* Cleanup any responses which were never read(). */
> - while (sfp->headrp)
> - sg_finish_rem_req(sfp->headrp);
> + list_for_each_entry_safe(srp, tmp, &sfp->rq_list, nextrp)
> + sg_finish_rem_req(srp);
What protects us from concurrent removals here?
Either way I'd rather keep the whіle not emptry style even with
proper lists.
Re: [PATCH 2/4] sg: protect access to to 'reserved' page array
On 02/03/2017 11:24 AM, Christoph Hellwig wrote: > On Fri, Feb 03, 2017 at 09:54:49AM +0100, Hannes Reinecke wrote: >> The 'reserved' page array is used as a short-cut for mapping >> data, saving us to allocate pages per request. >> However, the 'reserved' array is only capable of holding one >> request, so we need to protect it against concurrent accesses. > > Can you please explain how you protect the access here a bit more, > as mentioned before the set_bit for exclusion trick is always > suspicious, so the changelog needs to have a justification for it. > The 'reserved' array provides for a fast/reliable mechanism for mapping data of a request. However, it only has enough room to hold one request at a time. Plus we can change the size of the buffer during runtime via an ioctl. So we need to mark the array as 'in use' atomically, and keep that marker as long as the request using it is active. While I surely can introduce a variable 'in_use' and protect accesses to it via mutex or somesuch, I found this to be a bit pointless given that it's actually just one bit which needs to be checked. Which is what I did. But okay, I'll update the description. Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 4/4] sg: use standard lists for sg_requests
On 02/03/2017 11:43 AM, Christoph Hellwig wrote:
>> typedef struct sg_request { /* SG_MAX_QUEUE requests outstanding per file */
>> -struct sg_request *nextrp; /* NULL -> tail request (slist) */
>> +struct list_head nextrp;/* list entry */
>
> s/nextrp/entry/
>
>> @@ -2078,16 +2076,13 @@ static long sg_compat_ioctl(struct file *filp,
>> unsigned int cmd_in, unsigned lon
>> if (k < SG_MAX_QUEUE) {
>> memset(rp, 0, sizeof (Sg_request));
>> rp->parentfp = sfp;
>> +list_add(&rp->nextrp, &sfp->rq_list);
>
> The old code did a tail insertation. And this whole function should
> become a lot simpler with proper lists anyway:
>
Yeah, thought about that, too, but then I just went for the sloppy
approach to minimize changes.
> static Sg_request *
> sg_add_request(Sg_fd * sfp)
> {
> int k;
> unsigned long iflags;
> Sg_request *rp = sfp->req_arr;
>
> write_lock_irqsave(&sfp->rq_list_lock, iflags);
> if (!list_empty(&sfp->rq_list)) {
> if (!sfp->cmd_q)
> goto out_unlock;
>
> for (k = 0; k < SG_MAX_QUEUE; ++k, ++rp) {
> if (!rp->parentfp)
> break;
> }
> if (k >= SG_MAX_QUEUE)
> goto out_unlock;
> }
>
> memset(rp, 0, sizeof (Sg_request));
> rp->parentfp = sfp;
> rp->header.duration = jiffies_to_msecs(jiffies);
> list_add_tail(&rp->nextrp, &sfp->rq_list);
> write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
> return rp;
>
> out_unlock:
> write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
> return NULL;
>
Okay, will be updating the patch.
>> +if ((!sfp) || (!srp) || (list_empty(&sfp->rq_list)))
>
> No need for all these braces.
>
Okay.
>> +if (!list_empty(&srp->nextrp)) {
>> +list_del_init(&srp->nextrp);
>
> I don't think we need the _init as we never check for an empty entry.
>
Yes.
>> {
>> struct sg_fd *sfp = container_of(work, struct sg_fd, ew.work);
>> struct sg_device *sdp = sfp->parentdp;
>> +Sg_request *srp, *tmp;
>>
>> /* Cleanup any responses which were never read(). */
>> -while (sfp->headrp)
>> -sg_finish_rem_req(sfp->headrp);
>> +list_for_each_entry_safe(srp, tmp, &sfp->rq_list, nextrp)
>> +sg_finish_rem_req(srp);
>
> What protects us from concurrent removals here?
>
Nothing.
But this patch is intended to just replace the hand-rolled list
implementation, not fixing bugs here.
The problem is that 'sg_finish_rem_req()' is taking the rq_list_lock,
so it needs a bit of rework to make that work properly.
But I'll give it a go.
> Either way I'd rather keep the whіle not empty style even with
> proper lists.
>
Okay.
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH 3/4] sg: check for valid direction before starting the request
On 02/03/2017 11:28 AM, Christoph Hellwig wrote:
> On Fri, Feb 03, 2017 at 09:54:50AM +0100, Hannes Reinecke wrote:
>> From: Johannes Thumshirn
>>
>> Check for a valid direction before starting the request, otherwise we risk
>> running into an assertion in the scsi midlayer checking for vaild requests.
>
> Good idea, but..
>
>> +static bool sg_is_valid_direction(int dxfer_direction)
>> +{
>> +switch (dxfer_direction) {
>> +case SG_DXFER_NONE:
>> +case SG_DXFER_TO_DEV:
>> +case SG_DXFER_FROM_DEV:
>> +case SG_DXFER_TO_FROM_DEV:
>
> This isn't strictly valid as sg doesn't actually handle real bidi
> commands, but we work around it.
>
> It might be a good idea to move the warning about it from sg_write
> to here and use printk_ratelimited instead of the hand-rolled
> per-thread warning there.
>
>> +case SG_DXFER_UNKNOWN:
>> +return true;
>
> And how valid is this one (Question to Doug I guess): for a 0-sized
> transfer we should be fine, but otherwise it should probably be
> rejected.
>
yes, we should be checking for dxferp and dxfer_len here, too.
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH 3/4] sg: check for valid direction before starting the request
On 02/03/2017 11:28 AM, Christoph Hellwig wrote:
> On Fri, Feb 03, 2017 at 09:54:50AM +0100, Hannes Reinecke wrote:
>> From: Johannes Thumshirn
>>
>> Check for a valid direction before starting the request, otherwise we risk
>> running into an assertion in the scsi midlayer checking for vaild requests.
>
> Good idea, but..
>
>> +static bool sg_is_valid_direction(int dxfer_direction)
>> +{
>> +switch (dxfer_direction) {
>> +case SG_DXFER_NONE:
>> +case SG_DXFER_TO_DEV:
>> +case SG_DXFER_FROM_DEV:
>> +case SG_DXFER_TO_FROM_DEV:
>
> This isn't strictly valid as sg doesn't actually handle real bidi
> commands, but we work around it.
>
> It might be a good idea to move the warning about it from sg_write
> to here and use printk_ratelimited instead of the hand-rolled
> per-thread warning there.
>
Well, that warning is only applicable to sg_write(); sg_new_write() is
supposed to have the correct header, so that warning doesn't apply.
We could drop the per-thread-thingie, though.
>> +case SG_DXFER_UNKNOWN:
>> +return true;
>
> And how valid is this one (Question to Doug I guess): for a 0-sized
> transfer we should be fine, but otherwise it should probably be
> rejected.
>
Yes, true.
Will be fixing it up.
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
[PATCHv2 6/6] sg: close race condition in sg_remove_sfp_usercontext()
sg_remove_sfp_usercontext() is clearing any sg requests,
but needs to take 'rq_list_lock' when modifying the list.
Reported-by: Christoph Hellwig
Signed-off-by: Hannes Reinecke
---
drivers/scsi/sg.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 4b7e140..2b9d3325b 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -524,6 +524,7 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
} else
count = (old_hdr->result == 0) ? 0 : -EIO;
sg_finish_rem_req(srp);
+ sg_remove_request(sfp, srp);
retval = count;
free_old_hdr:
kfree(old_hdr);
@@ -564,6 +565,7 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
}
err_out:
err2 = sg_finish_rem_req(srp);
+ sg_remove_request(sfp, srp);
return err ? : err2 ? : count;
}
@@ -803,6 +805,7 @@ static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
SCSI_LOG_TIMEOUT(1, sg_printk(KERN_INFO, sfp->parentdp,
"sg_common_write: start_req err=%d\n", k));
sg_finish_rem_req(srp);
+ sg_remove_request(sfp, srp);
return k; /* probably out of space --> ENOMEM */
}
if (atomic_read(&sdp->detaching)) {
@@ -815,6 +818,7 @@ static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
}
sg_finish_rem_req(srp);
+ sg_remove_request(sfp, srp);
return -ENODEV;
}
@@ -1291,6 +1295,7 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
struct sg_fd *sfp = srp->parentfp;
sg_finish_rem_req(srp);
+ sg_remove_request(sfp, srp);
kref_put(&sfp->f_ref, sg_remove_sfp);
}
@@ -1825,8 +1830,6 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
else
sg_remove_scat(sfp, req_schp);
- sg_remove_request(sfp, srp);
-
return ret;
}
@@ -2171,12 +2174,17 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
struct sg_fd *sfp = container_of(work, struct sg_fd, ew.work);
struct sg_device *sdp = sfp->parentdp;
Sg_request *srp;
+ unsigned long iflags;
/* Cleanup any responses which were never read(). */
+ write_lock_irqsave(&sfp->rq_list_lock, iflags);
while (!list_empty(&sfp->rq_list)) {
srp = list_first_entry(&sfp->rq_list, Sg_request, entry);
sg_finish_rem_req(srp);
+ list_del(&srp->entry);
+ srp->parentfp = NULL;
}
+ write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
if (sfp->reserve.bufflen > 0) {
SCSI_LOG_TIMEOUT(6, sg_printk(KERN_INFO, sdp,
--
1.8.5.6
[PATCHv2 4/6] sg: check for valid direction before starting the request
From: Johannes Thumshirn
Check for a valid direction before starting the request, otherwise we risk
running into an assertion in the scsi midlayer checking for vaild requests.
Signed-off-by: Johannes Thumshirn
Link: http://www.spinics.net/lists/linux-scsi/msg104400.html
Reported-by: Dmitry Vyukov
Signed-off-by: Hannes Reinecke
---
drivers/scsi/sg.c | 46 ++
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 89ca76c..3045370 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -663,18 +663,14 @@ static int sg_allow_access(struct file *filp, unsigned
char *cmd)
* is a non-zero input_size, so emit a warning.
*/
if (hp->dxfer_direction == SG_DXFER_TO_FROM_DEV) {
- static char cmd[TASK_COMM_LEN];
- if (strcmp(current->comm, cmd)) {
- printk_ratelimited(KERN_WARNING
- "sg_write: data in/out %d/%d bytes "
- "for SCSI command 0x%x-- guessing "
- "data in;\n program %s not setting
"
- "count and/or reply_len properly\n",
- old_hdr.reply_len -
(int)SZ_SG_HEADER,
- input_size, (unsigned int) cmnd[0],
- current->comm);
- strcpy(cmd, current->comm);
- }
+ printk_ratelimited(KERN_WARNING
+ "sg_write: data in/out %d/%d bytes "
+ "for SCSI command 0x%x-- guessing "
+ "data in;\n program %s not setting "
+ "count and/or reply_len properly\n",
+ old_hdr.reply_len - (int)SZ_SG_HEADER,
+ input_size, (unsigned int) cmnd[0],
+ current->comm);
}
k = sg_common_write(sfp, srp, cmnd, sfp->timeout, blocking);
return (k < 0) ? k : count;
@@ -756,6 +752,29 @@ static int sg_allow_access(struct file *filp, unsigned
char *cmd)
return count;
}
+static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
+{
+ switch (dxfer_direction) {
+ case SG_DXFER_NONE:
+ if (hp->dxferp || hp->dxfer_len > 0)
+ return false;
+ return true;
+ case SG_DXFER_TO_DEV:
+ case SG_DXFER_FROM_DEV:
+ case SG_DXFER_TO_FROM_DEV:
+ if (!hp->dxferp || hp->dxfer_len == 0)
+ return false;
+ return true;
+ case SG_DXFER_UNKNOWN:
+ if ((!hp->dxferp && hp->dxfer_len) ||
+ (hp->dxferp && hp->dxfer_len == 0))
+ return false;
+ return true;
+ default:
+ return false;
+ }
+}
+
static int
sg_common_write(Sg_fd * sfp, Sg_request * srp,
unsigned char *cmnd, int timeout, int blocking)
@@ -776,6 +795,9 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
"sg_common_write: scsi opcode=0x%02x, cmd_size=%d\n",
(int) cmnd[0], (int) hp->cmd_len));
+ if (!sg_is_valid_dxfer(hp))
+ return -EINVAL;
+
k = sg_start_req(srp, cmnd);
if (k) {
SCSI_LOG_TIMEOUT(1, sg_printk(KERN_INFO, sfp->parentdp,
--
1.8.5.6
[PATCHv2 0/6] sanitize sg
Hi all, the infamous syzkaller incovered some more issues in the sg driver. This patchset fixes those two issues (and adds a fix for yet another potential issue; checking for a NULL dxferp when dxfer_len is not 0). It also removes handling of the SET_FORCE_LOW_DMA ioctl, which never worked since the initial git checkin. And does some code cleanup by removing the private list implementation, using standard lists instead. As usual, comments and reviews are welcome. Changes to v1: - Include reviews from Christoph - Add patch to close race condition in sg_remove_sfp_usercontext() - Remove stale variable 'save_scat_len' Hannes Reinecke (5): sg: disable SET_FORCE_LOW_DMA sg: remove 'save_scat_len' sg: protect accesses to 'reserved' page array sg: use standard lists for sg_requests sg: close race condition in sg_remove_sfp_usercontext() Johannes Thumshirn (1): sg: check for valid direction before starting the request drivers/scsi/sg.c | 284 +++--- include/scsi/sg.h | 1 - 2 files changed, 141 insertions(+), 144 deletions(-) -- 1.8.5.6
[PATCHv2 2/6] sg: remove 'save_scat_len'
Unused. Signed-off-by: Hannes Reinecke --- drivers/scsi/sg.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 55cd641..0b1279d 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -145,7 +145,6 @@ int timeout;/* defaults to SG_DEFAULT_TIMEOUT */ int timeout_user; /* defaults to SG_DEFAULT_TIMEOUT_USER */ Sg_scatter_hold reserve;/* buffer held for this file descriptor */ - unsigned save_scat_len; /* original length of trunc. scat. element */ Sg_request *headrp; /* head of request slist, NULL->empty */ struct fasync_struct *async_qp; /* used by asynchronous notification */ Sg_request req_arr[SG_MAX_QUEUE]; /* used as singly-linked list */ @@ -2006,7 +2005,6 @@ static long sg_compat_ioctl(struct file *filp, unsigned int cmd_in, unsigned lon req_schp->pages = NULL; req_schp->page_order = 0; req_schp->sglist_len = 0; - sfp->save_scat_len = 0; srp->res_used = 0; } -- 1.8.5.6
[PATCHv2 5/6] sg: use standard lists for sg_requests
'Sg_request' is using a private list implementation; convert it
to standard lists.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/sg.c | 149 +++---
1 file changed, 62 insertions(+), 87 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 3045370..4b7e140 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -122,7 +122,7 @@
struct sg_fd;
typedef struct sg_request {/* SG_MAX_QUEUE requests outstanding per file */
- struct sg_request *nextrp; /* NULL -> tail request (slist) */
+ struct list_head entry; /* list entry */
struct sg_fd *parentfp; /* NULL -> not in use */
Sg_scatter_hold data; /* hold buffer, perhaps scatter list */
sg_io_hdr_t header; /* scsi command+info, see */
@@ -146,7 +146,7 @@
int timeout;/* defaults to SG_DEFAULT_TIMEOUT */
int timeout_user; /* defaults to SG_DEFAULT_TIMEOUT_USER */
Sg_scatter_hold reserve;/* buffer held for this file descriptor
*/
- Sg_request *headrp; /* head of request slist, NULL->empty */
+ struct list_head rq_list; /* head of request list */
struct fasync_struct *async_qp; /* used by asynchronous notification */
Sg_request req_arr[SG_MAX_QUEUE]; /* used as singly-linked list */
char force_packid; /* 1 -> pack_id input to read(), 0 -> ignored */
@@ -754,7 +754,7 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
{
- switch (dxfer_direction) {
+ switch (hp->dxfer_direction) {
case SG_DXFER_NONE:
if (hp->dxferp || hp->dxfer_len > 0)
return false;
@@ -954,7 +954,7 @@ static int max_sectors_bytes(struct request_queue *q)
if (!access_ok(VERIFY_WRITE, ip, sizeof (int)))
return -EFAULT;
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- for (srp = sfp->headrp; srp; srp = srp->nextrp) {
+ list_for_each_entry(srp, &sfp->rq_list, entry) {
if ((1 == srp->done) && (!srp->sg_io_owned)) {
read_unlock_irqrestore(&sfp->rq_list_lock,
iflags);
@@ -967,7 +967,8 @@ static int max_sectors_bytes(struct request_queue *q)
return 0;
case SG_GET_NUM_WAITING:
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- for (val = 0, srp = sfp->headrp; srp; srp = srp->nextrp) {
+ val = 0;
+ list_for_each_entry(srp, &sfp->rq_list, entry) {
if ((1 == srp->done) && (!srp->sg_io_owned))
++val;
}
@@ -1042,35 +1043,33 @@ static int max_sectors_bytes(struct request_queue *q)
if (!rinfo)
return -ENOMEM;
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- for (srp = sfp->headrp, val = 0; val < SG_MAX_QUEUE;
-++val, srp = srp ? srp->nextrp : srp) {
+ val = 0;
+ list_for_each_entry(srp, &sfp->rq_list, entry) {
+ if (val > SG_MAX_QUEUE)
+ break;
memset(&rinfo[val], 0, SZ_SG_REQ_INFO);
- if (srp) {
- rinfo[val].req_state = srp->done + 1;
- rinfo[val].problem =
- srp->header.masked_status &
- srp->header.host_status &
- srp->header.driver_status;
- if (srp->done)
- rinfo[val].duration =
- srp->header.duration;
- else {
- ms = jiffies_to_msecs(jiffies);
- rinfo[val].duration =
- (ms > srp->header.duration)
?
- (ms - srp->header.duration)
: 0;
- }
- rinfo[val].orphan = srp->orphan;
- rinfo[val].sg_io_owned =
- srp->sg_io_owned;
- rinfo[val].pack_id =
- srp->header.pack_id;
- rinfo[val].usr_ptr =
- srp->header.usr_ptr;
[PATCHv2 3/6] sg: protect accesses to 'reserved' page array
The 'reserved' page array is used as a short-cut for mapping
data, saving us to allocate pages per request.
However, the 'reserved' array is only capable of holding one
request, so this patch introduces a mutex for protect
'sg_fd' against concurrent accesses.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/sg.c | 47 +++
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 0b1279d..89ca76c 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -142,6 +142,7 @@
struct sg_device *parentdp; /* owning device */
wait_queue_head_t read_wait;/* queue read until command done */
rwlock_t rq_list_lock; /* protect access to list in req_arr */
+ struct mutex f_mutex; /* protect against changes in this fd */
int timeout;/* defaults to SG_DEFAULT_TIMEOUT */
int timeout_user; /* defaults to SG_DEFAULT_TIMEOUT_USER */
Sg_scatter_hold reserve;/* buffer held for this file descriptor
*/
@@ -153,6 +154,7 @@
unsigned char next_cmd_len; /* 0: automatic, >0: use on next write() */
char keep_orphan; /* 0 -> drop orphan (def), 1 -> keep for read()
*/
char mmap_called; /* 0 -> mmap() never called on this fd */
+ char res_in_use;/* 1 -> 'reserve' array in use */
struct kref f_ref;
struct execute_work ew;
} Sg_fd;
@@ -196,7 +198,6 @@ static int sg_common_write(Sg_fd * sfp, Sg_request * srp,
static Sg_request *sg_get_rq_mark(Sg_fd * sfp, int pack_id);
static Sg_request *sg_add_request(Sg_fd * sfp);
static int sg_remove_request(Sg_fd * sfp, Sg_request * srp);
-static int sg_res_in_use(Sg_fd * sfp);
static Sg_device *sg_get_dev(int dev);
static void sg_device_destroy(struct kref *kref);
@@ -612,6 +613,7 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
}
buf += SZ_SG_HEADER;
__get_user(opcode, buf);
+ mutex_lock(&sfp->f_mutex);
if (sfp->next_cmd_len > 0) {
cmd_size = sfp->next_cmd_len;
sfp->next_cmd_len = 0; /* reset so only this write() effected
*/
@@ -620,6 +622,7 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
if ((opcode >= 0xc0) && old_hdr.twelve_byte)
cmd_size = 12;
}
+ mutex_unlock(&sfp->f_mutex);
SCSI_LOG_TIMEOUT(4, sg_printk(KERN_INFO, sdp,
"sg_write: scsi opcode=0x%02x, cmd_size=%d\n", (int) opcode,
cmd_size));
/* Determine buffer size. */
@@ -719,10 +722,13 @@ static int sg_allow_access(struct file *filp, unsigned
char *cmd)
sg_remove_request(sfp, srp);
return -EINVAL; /* either MMAP_IO or DIRECT_IO (not
both) */
}
- if (sg_res_in_use(sfp)) {
+ mutex_lock(&sfp->f_mutex);
+ if (sfp->res_in_use) {
+ mutex_unlock(&sfp->f_mutex);
sg_remove_request(sfp, srp);
return -EBUSY; /* reserve buffer already being used */
}
+ mutex_unlock(&sfp->f_mutex);
}
ul_timeout = msecs_to_jiffies(srp->header.timeout);
timeout = (ul_timeout < INT_MAX) ? ul_timeout : INT_MAX;
@@ -955,12 +961,18 @@ static int max_sectors_bytes(struct request_queue *q)
return -EINVAL;
val = min_t(int, val,
max_sectors_bytes(sdp->device->request_queue));
+ mutex_lock(&sfp->f_mutex);
if (val != sfp->reserve.bufflen) {
- if (sg_res_in_use(sfp) || sfp->mmap_called)
+ if (sfp->mmap_called ||
+ sfp->res_in_use) {
+ mutex_unlock(&sfp->f_mutex);
return -EBUSY;
+ }
+
sg_remove_scat(sfp, &sfp->reserve);
sg_build_reserve(sfp, val);
}
+ mutex_unlock(&sfp->f_mutex);
return 0;
case SG_GET_RESERVED_SIZE:
val = min_t(int, sfp->reserve.bufflen,
@@ -986,7 +998,9 @@ static int max_sectors_bytes(struct request_queue *q)
result = get_user(val, ip);
if (result)
return result;
+ mutex_lock(&sfp->f_mutex);
sfp->next_cmd_len = (val > 0) ? val : 0;
+ mutex_unlock(&sfp->f_mutex);
return 0;
case SG_GET_VERSION_NUM:
return put_user(sg_version_num, ip);
@@ -1713,13 +1727,19 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
md = &map_data;
if (md) {
- if (!sg_res_in_use(sfp) && dxfer_len <= rsv_schp->bufflen)
+ mutex_lo
[PATCHv2 1/6] sg: disable SET_FORCE_LOW_DMA
The ioctl SET_FORCE_LOW_DMA has never worked since the initial
git check-in, and the respective setting is nowadays handled
correctly.
So disable it entirely.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/sg.c | 30 +-
include/scsi/sg.h | 1 -
2 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index dbe5b4b..55cd641 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -149,7 +149,6 @@
Sg_request *headrp; /* head of request slist, NULL->empty */
struct fasync_struct *async_qp; /* used by asynchronous notification */
Sg_request req_arr[SG_MAX_QUEUE]; /* used as singly-linked list */
- char low_dma; /* as in parent but possibly overridden to 1 */
char force_packid; /* 1 -> pack_id input to read(), 0 -> ignored */
char cmd_q; /* 1 -> allow command queuing, 0 -> don't */
unsigned char next_cmd_len; /* 0: automatic, >0: use on next write() */
@@ -887,24 +886,14 @@ static int max_sectors_bytes(struct request_queue *q)
/* strange ..., for backward compatibility */
return sfp->timeout_user;
case SG_SET_FORCE_LOW_DMA:
- result = get_user(val, ip);
- if (result)
- return result;
- if (val) {
- sfp->low_dma = 1;
- if ((0 == sfp->low_dma) && (0 == sg_res_in_use(sfp))) {
- val = (int) sfp->reserve.bufflen;
- sg_remove_scat(sfp, &sfp->reserve);
- sg_build_reserve(sfp, val);
- }
- } else {
- if (atomic_read(&sdp->detaching))
- return -ENODEV;
- sfp->low_dma = sdp->device->host->unchecked_isa_dma;
- }
+ /*
+* N.B. This ioctl never worked properly, but failed to
+* return an error value. So returning '0' to keep compability
+* with legacy applications.
+*/
return 0;
case SG_GET_LOW_DMA:
- return put_user((int) sfp->low_dma, ip);
+ return put_user((int) sdp->device->host->unchecked_isa_dma, ip);
case SG_GET_SCSI_ID:
if (!access_ok(VERIFY_WRITE, p, sizeof (sg_scsi_id_t)))
return -EFAULT;
@@ -1821,6 +1810,7 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
int sg_tablesize = sfp->parentdp->sg_tablesize;
int blk_size = buff_size, order;
gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN;
+ struct sg_device *sdp = sfp->parentdp;
if (blk_size < 0)
return -EFAULT;
@@ -1846,7 +1836,7 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
scatter_elem_sz_prev = num;
}
- if (sfp->low_dma)
+ if (sdp->device->host->unchecked_isa_dma)
gfp_mask |= GFP_DMA;
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
@@ -2132,8 +2122,6 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
sfp->timeout = SG_DEFAULT_TIMEOUT;
sfp->timeout_user = SG_DEFAULT_TIMEOUT_USER;
sfp->force_packid = SG_DEF_FORCE_PACK_ID;
- sfp->low_dma = (SG_DEF_FORCE_LOW_DMA == 0) ?
- sdp->device->host->unchecked_isa_dma : 1;
sfp->cmd_q = SG_DEF_COMMAND_Q;
sfp->keep_orphan = SG_DEF_KEEP_ORPHAN;
sfp->parentdp = sdp;
@@ -2603,7 +2591,7 @@ static void sg_proc_debug_helper(struct seq_file *s,
Sg_device * sdp)
jiffies_to_msecs(fp->timeout),
fp->reserve.bufflen,
(int) fp->reserve.k_use_sg,
- (int) fp->low_dma);
+ (int) sdp->device->host->unchecked_isa_dma);
seq_printf(s, " cmd_q=%d f_packid=%d k_orphan=%d closed=0\n",
(int) fp->cmd_q, (int) fp->force_packid,
(int) fp->keep_orphan);
diff --git a/include/scsi/sg.h b/include/scsi/sg.h
index 3afec70..20bc71c 100644
--- a/include/scsi/sg.h
+++ b/include/scsi/sg.h
@@ -197,7 +197,6 @@
#define SG_DEFAULT_RETRIES 0
/* Defaults, commented if they differ from original sg driver */
-#define SG_DEF_FORCE_LOW_DMA 0 /* was 1 -> memory below 16MB on i386 */
#define SG_DEF_FORCE_PACK_ID 0
#define SG_DEF_KEEP_ORPHAN 0
#define SG_DEF_RESERVED_SIZE SG_SCATTER_SZ /* load time option */
--
1.8.5.6
Re: [PATCHv2 1/6] sg: disable SET_FORCE_LOW_DMA
On 02/03/2017 01:18 PM, Hannes Reinecke wrote: The ioctl SET_FORCE_LOW_DMA has never worked since the initial git check-in, and the respective setting is nowadays handled correctly. So disable it entirely. Signed-off-by: Hannes Reinecke --- Reviewed-by: Johannes Thumshirn
Re: [PATCHv2 2/6] sg: remove 'save_scat_len'
On 02/03/2017 01:18 PM, Hannes Reinecke wrote: Unused. Signed-off-by: Hannes Reinecke --- Reviewed-by: Johannes Thumshirn
Re: [PATCHv2 3/6] sg: protect accesses to 'reserved' page array
On 02/03/2017 01:18 PM, Hannes Reinecke wrote: The 'reserved' page array is used as a short-cut for mapping data, saving us to allocate pages per request. However, the 'reserved' array is only capable of holding one request, so this patch introduces a mutex for protect 'sg_fd' against concurrent accesses. Signed-off-by: Hannes Reinecke --- Tested-by: Johannes Thumshirn Reviewed-by: Johannes Thumshirn
Re: [PATCHv2 4/6] sg: check for valid direction before starting the request
On 02/03/2017 01:18 PM, Hannes Reinecke wrote:
From: Johannes Thumshirn
Check for a valid direction before starting the request, otherwise we risk
running into an assertion in the scsi midlayer checking for vaild requests.
Signed-off-by: Johannes Thumshirn
Link: http://www.spinics.net/lists/linux-scsi/msg104400.html
Reported-by: Dmitry Vyukov
Signed-off-by: Hannes Reinecke
---
[...]
+static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
+{
+ switch (dxfer_direction) {
This wont build now... needs a +hp
Re: [PATCHv2 5/6] sg: use standard lists for sg_requests
On 02/03/2017 01:18 PM, Hannes Reinecke wrote:
'Sg_request' is using a private list implementation; convert it
to standard lists.
Signed-off-by: Hannes Reinecke
---
[..]
static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
{
- switch (dxfer_direction) {
+ switch (hp->dxfer_direction) {
And here it is
Re: [PATCHv2 0/6] sanitize sg
On 02/03/2017 01:18 PM, Hannes Reinecke wrote: Hi all, the infamous syzkaller incovered some more issues in the sg driver. This patchset fixes those two issues (and adds a fix for yet another potential issue; checking for a NULL dxferp when dxfer_len is not 0). It also removes handling of the SET_FORCE_LOW_DMA ioctl, which never worked since the initial git checkin. And does some code cleanup by removing the private list implementation, using standard lists instead. As usual, comments and reviews are welcome. Changes to v1: - Include reviews from Christoph - Add patch to close race condition in sg_remove_sfp_usercontext() - Remove stale variable 'save_scat_len' Hannes Reinecke (5): sg: disable SET_FORCE_LOW_DMA sg: remove 'save_scat_len' sg: protect accesses to 'reserved' page array sg: use standard lists for sg_requests sg: close race condition in sg_remove_sfp_usercontext() Johannes Thumshirn (1): sg: check for valid direction before starting the request drivers/scsi/sg.c | 284 +++--- include/scsi/sg.h | 1 - 2 files changed, 141 insertions(+), 144 deletions(-) For the whole series Tested-by: Johannes Thumshirn (sg_inq not broken, sg_turs not broken, syzcaller bug on fixed and syzcaller use-after-free fixed, no additional messages in dmesg with a KASAN and LOCKDEP enabled kernel) -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
[PATCHv3 2/6] sg: remove 'save_scat_len'
Unused. Signed-off-by: Hannes Reinecke Reviewed-by: Johannes Thumshirn Tested-by: Johannes Thumshirn --- drivers/scsi/sg.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 55cd641..0b1279d 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -145,7 +145,6 @@ int timeout;/* defaults to SG_DEFAULT_TIMEOUT */ int timeout_user; /* defaults to SG_DEFAULT_TIMEOUT_USER */ Sg_scatter_hold reserve;/* buffer held for this file descriptor */ - unsigned save_scat_len; /* original length of trunc. scat. element */ Sg_request *headrp; /* head of request slist, NULL->empty */ struct fasync_struct *async_qp; /* used by asynchronous notification */ Sg_request req_arr[SG_MAX_QUEUE]; /* used as singly-linked list */ @@ -2006,7 +2005,6 @@ static long sg_compat_ioctl(struct file *filp, unsigned int cmd_in, unsigned lon req_schp->pages = NULL; req_schp->page_order = 0; req_schp->sglist_len = 0; - sfp->save_scat_len = 0; srp->res_used = 0; } -- 1.8.5.6
[PATCHv3 4/6] sg: check for valid direction before starting the request
From: Johannes Thumshirn
Check for a valid direction before starting the request, otherwise we risk
running into an assertion in the scsi midlayer checking for vaild requests.
Signed-off-by: Johannes Thumshirn
Link: http://www.spinics.net/lists/linux-scsi/msg104400.html
Reported-by: Dmitry Vyukov
Signed-off-by: Hannes Reinecke
Tested-by: Johannes Thumshirn
---
drivers/scsi/sg.c | 46 ++
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 89ca76c..3045370 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -663,18 +663,14 @@ static int sg_allow_access(struct file *filp, unsigned
char *cmd)
* is a non-zero input_size, so emit a warning.
*/
if (hp->dxfer_direction == SG_DXFER_TO_FROM_DEV) {
- static char cmd[TASK_COMM_LEN];
- if (strcmp(current->comm, cmd)) {
- printk_ratelimited(KERN_WARNING
- "sg_write: data in/out %d/%d bytes "
- "for SCSI command 0x%x-- guessing "
- "data in;\n program %s not setting
"
- "count and/or reply_len properly\n",
- old_hdr.reply_len -
(int)SZ_SG_HEADER,
- input_size, (unsigned int) cmnd[0],
- current->comm);
- strcpy(cmd, current->comm);
- }
+ printk_ratelimited(KERN_WARNING
+ "sg_write: data in/out %d/%d bytes "
+ "for SCSI command 0x%x-- guessing "
+ "data in;\n program %s not setting "
+ "count and/or reply_len properly\n",
+ old_hdr.reply_len - (int)SZ_SG_HEADER,
+ input_size, (unsigned int) cmnd[0],
+ current->comm);
}
k = sg_common_write(sfp, srp, cmnd, sfp->timeout, blocking);
return (k < 0) ? k : count;
@@ -756,6 +752,29 @@ static int sg_allow_access(struct file *filp, unsigned
char *cmd)
return count;
}
+static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
+{
+ switch (dxfer_direction) {
+ case SG_DXFER_NONE:
+ if (hp->dxferp || hp->dxfer_len > 0)
+ return false;
+ return true;
+ case SG_DXFER_TO_DEV:
+ case SG_DXFER_FROM_DEV:
+ case SG_DXFER_TO_FROM_DEV:
+ if (!hp->dxferp || hp->dxfer_len == 0)
+ return false;
+ return true;
+ case SG_DXFER_UNKNOWN:
+ if ((!hp->dxferp && hp->dxfer_len) ||
+ (hp->dxferp && hp->dxfer_len == 0))
+ return false;
+ return true;
+ default:
+ return false;
+ }
+}
+
static int
sg_common_write(Sg_fd * sfp, Sg_request * srp,
unsigned char *cmnd, int timeout, int blocking)
@@ -776,6 +795,9 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
"sg_common_write: scsi opcode=0x%02x, cmd_size=%d\n",
(int) cmnd[0], (int) hp->cmd_len));
+ if (!sg_is_valid_dxfer(hp))
+ return -EINVAL;
+
k = sg_start_req(srp, cmnd);
if (k) {
SCSI_LOG_TIMEOUT(1, sg_printk(KERN_INFO, sfp->parentdp,
--
1.8.5.6
[PATCHv3 6/6] sg: close race condition in sg_remove_sfp_usercontext()
sg_remove_sfp_usercontext() is clearing any sg requests,
but needs to take 'rq_list_lock' when modifying the list.
Reported-by: Christoph Hellwig
Signed-off-by: Hannes Reinecke
Reviewed-by: Johannes Thumshirn
Tested-by: Johannes Thumshirn
---
drivers/scsi/sg.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 4b7e140..2b9d3325b 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -524,6 +524,7 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
} else
count = (old_hdr->result == 0) ? 0 : -EIO;
sg_finish_rem_req(srp);
+ sg_remove_request(sfp, srp);
retval = count;
free_old_hdr:
kfree(old_hdr);
@@ -564,6 +565,7 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
}
err_out:
err2 = sg_finish_rem_req(srp);
+ sg_remove_request(sfp, srp);
return err ? : err2 ? : count;
}
@@ -803,6 +805,7 @@ static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
SCSI_LOG_TIMEOUT(1, sg_printk(KERN_INFO, sfp->parentdp,
"sg_common_write: start_req err=%d\n", k));
sg_finish_rem_req(srp);
+ sg_remove_request(sfp, srp);
return k; /* probably out of space --> ENOMEM */
}
if (atomic_read(&sdp->detaching)) {
@@ -815,6 +818,7 @@ static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
}
sg_finish_rem_req(srp);
+ sg_remove_request(sfp, srp);
return -ENODEV;
}
@@ -1291,6 +1295,7 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
struct sg_fd *sfp = srp->parentfp;
sg_finish_rem_req(srp);
+ sg_remove_request(sfp, srp);
kref_put(&sfp->f_ref, sg_remove_sfp);
}
@@ -1825,8 +1830,6 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
else
sg_remove_scat(sfp, req_schp);
- sg_remove_request(sfp, srp);
-
return ret;
}
@@ -2171,12 +2174,17 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
struct sg_fd *sfp = container_of(work, struct sg_fd, ew.work);
struct sg_device *sdp = sfp->parentdp;
Sg_request *srp;
+ unsigned long iflags;
/* Cleanup any responses which were never read(). */
+ write_lock_irqsave(&sfp->rq_list_lock, iflags);
while (!list_empty(&sfp->rq_list)) {
srp = list_first_entry(&sfp->rq_list, Sg_request, entry);
sg_finish_rem_req(srp);
+ list_del(&srp->entry);
+ srp->parentfp = NULL;
}
+ write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
if (sfp->reserve.bufflen > 0) {
SCSI_LOG_TIMEOUT(6, sg_printk(KERN_INFO, sdp,
--
1.8.5.6
[PATCHv3 0/6] sanitize sg
Hi all, the infamous syzkaller incovered some more issues in the sg driver. This patchset fixes those two issues (and adds a fix for yet another potential issue; checking for a NULL dxferp when dxfer_len is not 0). It also removes handling of the SET_FORCE_LOW_DMA ioctl, which never worked since the initial git checkin. And does some code cleanup by removing the private list implementation, using standard lists instead. As usual, comments and reviews are welcome. Changes to v1: - Include reviews from Christoph - Add patch to close race condition in sg_remove_sfp_usercontext() - Remove stale variable 'save_scat_len' Changes to v2: - Move misplaced hunk - Add Reviewed-by: and Tested-by: tags Hannes Reinecke (5): sg: disable SET_FORCE_LOW_DMA sg: remove 'save_scat_len' sg: protect accesses to 'reserved' page array sg: use standard lists for sg_requests sg: close race condition in sg_remove_sfp_usercontext() Johannes Thumshirn (1): sg: check for valid direction before starting the request drivers/scsi/sg.c | 284 +++--- include/scsi/sg.h | 1 - 2 files changed, 141 insertions(+), 144 deletions(-) -- 1.8.5.6
[PATCHv3 1/6] sg: disable SET_FORCE_LOW_DMA
The ioctl SET_FORCE_LOW_DMA has never worked since the initial
git check-in, and the respective setting is nowadays handled
correctly.
So disable it entirely.
Signed-off-by: Hannes Reinecke
Reviewed-by: Johannes Thumshirn
Tested-by: Johannes Thumshirn
---
drivers/scsi/sg.c | 30 +-
include/scsi/sg.h | 1 -
2 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index dbe5b4b..55cd641 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -149,7 +149,6 @@
Sg_request *headrp; /* head of request slist, NULL->empty */
struct fasync_struct *async_qp; /* used by asynchronous notification */
Sg_request req_arr[SG_MAX_QUEUE]; /* used as singly-linked list */
- char low_dma; /* as in parent but possibly overridden to 1 */
char force_packid; /* 1 -> pack_id input to read(), 0 -> ignored */
char cmd_q; /* 1 -> allow command queuing, 0 -> don't */
unsigned char next_cmd_len; /* 0: automatic, >0: use on next write() */
@@ -887,24 +886,14 @@ static int max_sectors_bytes(struct request_queue *q)
/* strange ..., for backward compatibility */
return sfp->timeout_user;
case SG_SET_FORCE_LOW_DMA:
- result = get_user(val, ip);
- if (result)
- return result;
- if (val) {
- sfp->low_dma = 1;
- if ((0 == sfp->low_dma) && (0 == sg_res_in_use(sfp))) {
- val = (int) sfp->reserve.bufflen;
- sg_remove_scat(sfp, &sfp->reserve);
- sg_build_reserve(sfp, val);
- }
- } else {
- if (atomic_read(&sdp->detaching))
- return -ENODEV;
- sfp->low_dma = sdp->device->host->unchecked_isa_dma;
- }
+ /*
+* N.B. This ioctl never worked properly, but failed to
+* return an error value. So returning '0' to keep compability
+* with legacy applications.
+*/
return 0;
case SG_GET_LOW_DMA:
- return put_user((int) sfp->low_dma, ip);
+ return put_user((int) sdp->device->host->unchecked_isa_dma, ip);
case SG_GET_SCSI_ID:
if (!access_ok(VERIFY_WRITE, p, sizeof (sg_scsi_id_t)))
return -EFAULT;
@@ -1821,6 +1810,7 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
int sg_tablesize = sfp->parentdp->sg_tablesize;
int blk_size = buff_size, order;
gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN;
+ struct sg_device *sdp = sfp->parentdp;
if (blk_size < 0)
return -EFAULT;
@@ -1846,7 +1836,7 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
scatter_elem_sz_prev = num;
}
- if (sfp->low_dma)
+ if (sdp->device->host->unchecked_isa_dma)
gfp_mask |= GFP_DMA;
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
@@ -2132,8 +2122,6 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
sfp->timeout = SG_DEFAULT_TIMEOUT;
sfp->timeout_user = SG_DEFAULT_TIMEOUT_USER;
sfp->force_packid = SG_DEF_FORCE_PACK_ID;
- sfp->low_dma = (SG_DEF_FORCE_LOW_DMA == 0) ?
- sdp->device->host->unchecked_isa_dma : 1;
sfp->cmd_q = SG_DEF_COMMAND_Q;
sfp->keep_orphan = SG_DEF_KEEP_ORPHAN;
sfp->parentdp = sdp;
@@ -2603,7 +2591,7 @@ static void sg_proc_debug_helper(struct seq_file *s,
Sg_device * sdp)
jiffies_to_msecs(fp->timeout),
fp->reserve.bufflen,
(int) fp->reserve.k_use_sg,
- (int) fp->low_dma);
+ (int) sdp->device->host->unchecked_isa_dma);
seq_printf(s, " cmd_q=%d f_packid=%d k_orphan=%d closed=0\n",
(int) fp->cmd_q, (int) fp->force_packid,
(int) fp->keep_orphan);
diff --git a/include/scsi/sg.h b/include/scsi/sg.h
index 3afec70..20bc71c 100644
--- a/include/scsi/sg.h
+++ b/include/scsi/sg.h
@@ -197,7 +197,6 @@
#define SG_DEFAULT_RETRIES 0
/* Defaults, commented if they differ from original sg driver */
-#define SG_DEF_FORCE_LOW_DMA 0 /* was 1 -> memory below 16MB on i386 */
#define SG_DEF_FORCE_PACK_ID 0
#define SG_DEF_KEEP_ORPHAN 0
#define SG_DEF_RESERVED_SIZE SG_SCATTER_SZ /* load time option */
--
1.8.5.6
[PATCHv3 3/6] sg: protect accesses to 'reserved' page array
The 'reserved' page array is used as a short-cut for mapping
data, saving us to allocate pages per request.
However, the 'reserved' array is only capable of holding one
request, so this patch introduces a mutex for protect
'sg_fd' against concurrent accesses.
Signed-off-by: Hannes Reinecke
Reviewed-by: Johannes Thumshirn
Tested-by: Johannes Thumshirn
---
drivers/scsi/sg.c | 47 +++
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 0b1279d..89ca76c 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -142,6 +142,7 @@
struct sg_device *parentdp; /* owning device */
wait_queue_head_t read_wait;/* queue read until command done */
rwlock_t rq_list_lock; /* protect access to list in req_arr */
+ struct mutex f_mutex; /* protect against changes in this fd */
int timeout;/* defaults to SG_DEFAULT_TIMEOUT */
int timeout_user; /* defaults to SG_DEFAULT_TIMEOUT_USER */
Sg_scatter_hold reserve;/* buffer held for this file descriptor
*/
@@ -153,6 +154,7 @@
unsigned char next_cmd_len; /* 0: automatic, >0: use on next write() */
char keep_orphan; /* 0 -> drop orphan (def), 1 -> keep for read()
*/
char mmap_called; /* 0 -> mmap() never called on this fd */
+ char res_in_use;/* 1 -> 'reserve' array in use */
struct kref f_ref;
struct execute_work ew;
} Sg_fd;
@@ -196,7 +198,6 @@ static int sg_common_write(Sg_fd * sfp, Sg_request * srp,
static Sg_request *sg_get_rq_mark(Sg_fd * sfp, int pack_id);
static Sg_request *sg_add_request(Sg_fd * sfp);
static int sg_remove_request(Sg_fd * sfp, Sg_request * srp);
-static int sg_res_in_use(Sg_fd * sfp);
static Sg_device *sg_get_dev(int dev);
static void sg_device_destroy(struct kref *kref);
@@ -612,6 +613,7 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
}
buf += SZ_SG_HEADER;
__get_user(opcode, buf);
+ mutex_lock(&sfp->f_mutex);
if (sfp->next_cmd_len > 0) {
cmd_size = sfp->next_cmd_len;
sfp->next_cmd_len = 0; /* reset so only this write() effected
*/
@@ -620,6 +622,7 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
if ((opcode >= 0xc0) && old_hdr.twelve_byte)
cmd_size = 12;
}
+ mutex_unlock(&sfp->f_mutex);
SCSI_LOG_TIMEOUT(4, sg_printk(KERN_INFO, sdp,
"sg_write: scsi opcode=0x%02x, cmd_size=%d\n", (int) opcode,
cmd_size));
/* Determine buffer size. */
@@ -719,10 +722,13 @@ static int sg_allow_access(struct file *filp, unsigned
char *cmd)
sg_remove_request(sfp, srp);
return -EINVAL; /* either MMAP_IO or DIRECT_IO (not
both) */
}
- if (sg_res_in_use(sfp)) {
+ mutex_lock(&sfp->f_mutex);
+ if (sfp->res_in_use) {
+ mutex_unlock(&sfp->f_mutex);
sg_remove_request(sfp, srp);
return -EBUSY; /* reserve buffer already being used */
}
+ mutex_unlock(&sfp->f_mutex);
}
ul_timeout = msecs_to_jiffies(srp->header.timeout);
timeout = (ul_timeout < INT_MAX) ? ul_timeout : INT_MAX;
@@ -955,12 +961,18 @@ static int max_sectors_bytes(struct request_queue *q)
return -EINVAL;
val = min_t(int, val,
max_sectors_bytes(sdp->device->request_queue));
+ mutex_lock(&sfp->f_mutex);
if (val != sfp->reserve.bufflen) {
- if (sg_res_in_use(sfp) || sfp->mmap_called)
+ if (sfp->mmap_called ||
+ sfp->res_in_use) {
+ mutex_unlock(&sfp->f_mutex);
return -EBUSY;
+ }
+
sg_remove_scat(sfp, &sfp->reserve);
sg_build_reserve(sfp, val);
}
+ mutex_unlock(&sfp->f_mutex);
return 0;
case SG_GET_RESERVED_SIZE:
val = min_t(int, sfp->reserve.bufflen,
@@ -986,7 +998,9 @@ static int max_sectors_bytes(struct request_queue *q)
result = get_user(val, ip);
if (result)
return result;
+ mutex_lock(&sfp->f_mutex);
sfp->next_cmd_len = (val > 0) ? val : 0;
+ mutex_unlock(&sfp->f_mutex);
return 0;
case SG_GET_VERSION_NUM:
return put_user(sg_version_num, ip);
@@ -1713,13 +1727,19 @@ static long sg_compat_ioctl(struct file *filp, unsigned
int cmd_in, unsigned lon
md = &map_data;
if (md) {
- if (!sg_res_in_use(
[PATCHv3 5/6] sg: use standard lists for sg_requests
'Sg_request' is using a private list implementation; convert it
to standard lists.
Signed-off-by: Hannes Reinecke
Reviewed-by: Johannes Thumshirn
Tested-by: Johannes Thumshirn
---
drivers/scsi/sg.c | 149 +++---
1 file changed, 62 insertions(+), 87 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 3045370..4b7e140 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -122,7 +122,7 @@
struct sg_fd;
typedef struct sg_request {/* SG_MAX_QUEUE requests outstanding per file */
- struct sg_request *nextrp; /* NULL -> tail request (slist) */
+ struct list_head entry; /* list entry */
struct sg_fd *parentfp; /* NULL -> not in use */
Sg_scatter_hold data; /* hold buffer, perhaps scatter list */
sg_io_hdr_t header; /* scsi command+info, see */
@@ -146,7 +146,7 @@
int timeout;/* defaults to SG_DEFAULT_TIMEOUT */
int timeout_user; /* defaults to SG_DEFAULT_TIMEOUT_USER */
Sg_scatter_hold reserve;/* buffer held for this file descriptor
*/
- Sg_request *headrp; /* head of request slist, NULL->empty */
+ struct list_head rq_list; /* head of request list */
struct fasync_struct *async_qp; /* used by asynchronous notification */
Sg_request req_arr[SG_MAX_QUEUE]; /* used as singly-linked list */
char force_packid; /* 1 -> pack_id input to read(), 0 -> ignored */
@@ -754,7 +754,7 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
{
- switch (dxfer_direction) {
+ switch (hp->dxfer_direction) {
case SG_DXFER_NONE:
if (hp->dxferp || hp->dxfer_len > 0)
return false;
@@ -954,7 +954,7 @@ static int max_sectors_bytes(struct request_queue *q)
if (!access_ok(VERIFY_WRITE, ip, sizeof (int)))
return -EFAULT;
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- for (srp = sfp->headrp; srp; srp = srp->nextrp) {
+ list_for_each_entry(srp, &sfp->rq_list, entry) {
if ((1 == srp->done) && (!srp->sg_io_owned)) {
read_unlock_irqrestore(&sfp->rq_list_lock,
iflags);
@@ -967,7 +967,8 @@ static int max_sectors_bytes(struct request_queue *q)
return 0;
case SG_GET_NUM_WAITING:
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- for (val = 0, srp = sfp->headrp; srp; srp = srp->nextrp) {
+ val = 0;
+ list_for_each_entry(srp, &sfp->rq_list, entry) {
if ((1 == srp->done) && (!srp->sg_io_owned))
++val;
}
@@ -1042,35 +1043,33 @@ static int max_sectors_bytes(struct request_queue *q)
if (!rinfo)
return -ENOMEM;
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- for (srp = sfp->headrp, val = 0; val < SG_MAX_QUEUE;
-++val, srp = srp ? srp->nextrp : srp) {
+ val = 0;
+ list_for_each_entry(srp, &sfp->rq_list, entry) {
+ if (val > SG_MAX_QUEUE)
+ break;
memset(&rinfo[val], 0, SZ_SG_REQ_INFO);
- if (srp) {
- rinfo[val].req_state = srp->done + 1;
- rinfo[val].problem =
- srp->header.masked_status &
- srp->header.host_status &
- srp->header.driver_status;
- if (srp->done)
- rinfo[val].duration =
- srp->header.duration;
- else {
- ms = jiffies_to_msecs(jiffies);
- rinfo[val].duration =
- (ms > srp->header.duration)
?
- (ms - srp->header.duration)
: 0;
- }
- rinfo[val].orphan = srp->orphan;
- rinfo[val].sg_io_owned =
- srp->sg_io_owned;
- rinfo[val].pack_id =
- srp->header.pack_id;
- rinfo[val].usr_ptr =
-
Re: [PATCHv3 1/6] sg: disable SET_FORCE_LOW_DMA
Looks fine: Reviewed-by: Christoph Hellwig
Re: [PATCHv3 2/6] sg: remove 'save_scat_len'
Looks fine, Reviewed-by: Christoph Hellwig
Re: [PATCHv3 3/6] sg: protect accesses to 'reserved' page array
> - if (sg_res_in_use(sfp)) {
> + mutex_lock(&sfp->f_mutex);
> + if (sfp->res_in_use) {
> + mutex_unlock(&sfp->f_mutex);
> sg_remove_request(sfp, srp);
> return -EBUSY; /* reserve buffer already being used */
> }
> + mutex_unlock(&sfp->f_mutex);
Holding a mutex over a the check of a single scalar doesn't make sense.
Re: [PATCHv3 3/6] sg: protect accesses to 'reserved' page array
On 02/03/2017 02:31 PM, Christoph Hellwig wrote:
>> -if (sg_res_in_use(sfp)) {
>> +mutex_lock(&sfp->f_mutex);
>> +if (sfp->res_in_use) {
>> +mutex_unlock(&sfp->f_mutex);
>> sg_remove_request(sfp, srp);
>> return -EBUSY; /* reserve buffer already being used */
>> }
>> +mutex_unlock(&sfp->f_mutex);
>
> Holding a mutex over a the check of a single scalar doesn't make sense.
>
It's adds a synchronisation point, doesn't it?
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCHv2 4/6] sg: check for valid direction before starting the request
Hi Johannes,
[auto build test ERROR on scsi/for-next]
[also build test ERROR on v4.10-rc6 next-20170203]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Hannes-Reinecke/sanitize-sg/20170203-202314
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: x86_64-acpi-redef (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
Note: the linux-review/Hannes-Reinecke/sanitize-sg/20170203-202314 HEAD
c442f1293c347b17429edecbc5d29d4eb39152b0 builds fine.
It only hurts bisectibility.
All error/warnings (new ones prefixed by >>):
drivers/scsi/sg.c: In function 'sg_is_valid_dxfer':
>> drivers/scsi/sg.c:757:10: error: 'dxfer_direction' undeclared (first use in
>> this function)
switch (dxfer_direction) {
^~~
drivers/scsi/sg.c:757:10: note: each undeclared identifier is reported only
once for each function it appears in
>> drivers/scsi/sg.c:776:1: warning: control reaches end of non-void function
>> [-Wreturn-type]
}
^
vim +/dxfer_direction +757 drivers/scsi/sg.c
751 *o_srp = srp;
752 return count;
753 }
754
755 static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
756 {
> 757 switch (dxfer_direction) {
758 case SG_DXFER_NONE:
759 if (hp->dxferp || hp->dxfer_len > 0)
760 return false;
761 return true;
762 case SG_DXFER_TO_DEV:
763 case SG_DXFER_FROM_DEV:
764 case SG_DXFER_TO_FROM_DEV:
765 if (!hp->dxferp || hp->dxfer_len == 0)
766 return false;
767 return true;
768 case SG_DXFER_UNKNOWN:
769 if ((!hp->dxferp && hp->dxfer_len) ||
770 (hp->dxferp && hp->dxfer_len == 0))
771 return false;
772 return true;
773 default:
774 return false;
775 }
> 776 }
777
778 static int
779 sg_common_write(Sg_fd * sfp, Sg_request * srp,
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [REGRESSION v4.10-rc1] blkdev_issue_zeroout() returns -EREMOTEIO on the first call for SCSI device that doesn't support WRITE SAME
On 02/03/2017 12:55 AM, Junichi Nomura wrote:
> I found following ext4 error occurs on a certain storage since v4.10-rc1:
> EXT4-fs (sdc1): Delayed block allocation failed for inode 12 at logical
> offset 100 with max blocks 2 with error 121
> EXT4-fs (sdc1): This should not happen!! Data will be lost
>
> Error 121 (EREMOTEIO) was returned from blkdev_issue_zeroout().
> That came from sd driver because WRITE SAME was sent to the device
> which didn't support it.
>
> The problem was introduced by commit e73c23ff736e ("block: add async
> variant of blkdev_issue_zeroout"). Before the commit, blkdev_issue_zeroout
> fell back to normal zero writing when WRITE SAME failed and it seems
> sd driver's heuristics depends on that behaviour.
CC Christoph and Chaitanya.
--
Jens Axboe
Re: [REGRESSION v4.10-rc1] blkdev_issue_zeroout() returns -EREMOTEIO on the first call for SCSI device that doesn't support WRITE SAME
On Fri, Feb 03, 2017 at 08:21:31AM -0700, Jens Axboe wrote:
> > Error 121 (EREMOTEIO) was returned from blkdev_issue_zeroout().
> > That came from sd driver because WRITE SAME was sent to the device
> > which didn't support it.
> >
> > The problem was introduced by commit e73c23ff736e ("block: add async
> > variant of blkdev_issue_zeroout"). Before the commit, blkdev_issue_zeroout
> > fell back to normal zero writing when WRITE SAME failed and it seems
> > sd driver's heuristics depends on that behaviour.
>
> CC Christoph and Chaitanya.
And adding Martin as the sd.c Write Same code is his.
I suspect we'll have to restore the old way this works for 4.10 as it's
too late in the cycle, but that whole idea of trying Write Same first
and just disabling it if it doesn't work is a receipe for desaster -
it kinda works for a synchronous blkdev_issue_zeroout, but if we want
to be able to submit it asynchronously it's getting too hairy to handle.
I think we should fix sd.c to only send WRITE SAME if either of the
variants are explicitly listed as supported through
REPORT SUPPORTED OPERATION CODES, or maybe through a whitelist if
there are important enough devices.
Re: [REGRESSION v4.10-rc1] blkdev_issue_zeroout() returns -EREMOTEIO on the first call for SCSI device that doesn't support WRITE SAME
On 02/03/2017 09:12 AM, Christoph Hellwig wrote:
> On Fri, Feb 03, 2017 at 08:21:31AM -0700, Jens Axboe wrote:
>>> Error 121 (EREMOTEIO) was returned from blkdev_issue_zeroout().
>>> That came from sd driver because WRITE SAME was sent to the device
>>> which didn't support it.
>>>
>>> The problem was introduced by commit e73c23ff736e ("block: add async
>>> variant of blkdev_issue_zeroout"). Before the commit, blkdev_issue_zeroout
>>> fell back to normal zero writing when WRITE SAME failed and it seems
>>> sd driver's heuristics depends on that behaviour.
>>
>> CC Christoph and Chaitanya.
>
> And adding Martin as the sd.c Write Same code is his.
>
> I suspect we'll have to restore the old way this works for 4.10 as it's
> too late in the cycle, but that whole idea of trying Write Same first
> and just disabling it if it doesn't work is a receipe for desaster -
> it kinda works for a synchronous blkdev_issue_zeroout, but if we want
> to be able to submit it asynchronously it's getting too hairy to handle.
I agree, the current approach is a hot and ugly mess.
> I think we should fix sd.c to only send WRITE SAME if either of the
> variants are explicitly listed as supported through
> REPORT SUPPORTED OPERATION CODES, or maybe through a whitelist if
> there are important enough devices.
Yep
--
Jens Axboe
Re: [PATCHv3 3/6] sg: protect accesses to 'reserved' page array
On Fri, Feb 03, 2017 at 02:38:35PM +0100, Hannes Reinecke wrote:
> On 02/03/2017 02:31 PM, Christoph Hellwig wrote:
> >> - if (sg_res_in_use(sfp)) {
> >> + mutex_lock(&sfp->f_mutex);
> >> + if (sfp->res_in_use) {
> >> + mutex_unlock(&sfp->f_mutex);
> >>sg_remove_request(sfp, srp);
> >>return -EBUSY; /* reserve buffer already being used */
> >>}
> >> + mutex_unlock(&sfp->f_mutex);
> >
> > Holding a mutex over a the check of a single scalar doesn't make sense.
> >
> It's adds a synchronisation point, doesn't it?
It does, but it doesn't actually protect anything..
Re: [PATCH 00/15] qla2xxx: Bug Fixes and updates for target.
On Thu, 2017-02-02 at 11:42 -0800, Himanshu Madhani wrote: > Please consider this series for inclusion in target-pending. Hello Himanshu, What tree have these patches been generated against? Not all patches can be applied on top of the scsi-target-for-v4.11 branch. Thanks, Bart.
Re: [PATCH 1/2] qla2xxx: Fix a recently introduced memory leak
On Wed, 2017-01-25 at 18:28 -0500, Martin K. Petersen wrote: > > > > > > "Bart" == Bart Van Assche writes: > > Bart> qla2x00_probe_one() allocates IRQs before it initializes rsp_q_map > Bart> so IRQs must be freed even if rsp_q_map allocation did not occur. > Bart> This was detected by kmemleak. > > I queued this one yesterday but was waiting for a resolution on patch > 2... Hello Martin, Since there is now a resolution for patch 2: have you already decided when to submit these patches upstream? Thanks, Bart.
Re: [PATCH 00/15] qla2xxx: Bug Fixes and updates for target.
Hi Bart, On 2/3/17, 8:26 AM, "Bart Van Assche" wrote: >On Thu, 2017-02-02 at 11:42 -0800, Himanshu Madhani wrote: >> Please consider this series for inclusion in target-pending. > >Hello Himanshu, > >What tree have these patches been generated against? Not all patches can be >applied on top of the scsi-target-for-v4.11 branch. These patches were rebased on 4.10-rc6+ and qla2xxx patches accepted for 4.11 branch. I’ll rebase them on top of scsi-target-for-v4.11 brach and repost. > >Thanks, > >Bart.
Re: [PATCHv3 3/6] sg: protect accesses to 'reserved' page array
On 02/03/2017 05:19 PM, Christoph Hellwig wrote:
On Fri, Feb 03, 2017 at 02:38:35PM +0100, Hannes Reinecke wrote:
On 02/03/2017 02:31 PM, Christoph Hellwig wrote:
- if (sg_res_in_use(sfp)) {
+ mutex_lock(&sfp->f_mutex);
+ if (sfp->res_in_use) {
+ mutex_unlock(&sfp->f_mutex);
sg_remove_request(sfp, srp);
return -EBUSY; /* reserve buffer already being used */
}
+ mutex_unlock(&sfp->f_mutex);
Holding a mutex over a the check of a single scalar doesn't make sense.
It's adds a synchronisation point, doesn't it?
It does, but it doesn't actually protect anything..
But all the other mutex_{un,}locks() do (for instance guarding
sg_build_indirect()) and this one provides a synchronization point.
Sorry but I really don't get your point here.
The sole purpose is to guard the reserved list from being altered while
blk_rq_map_* or similar functions are in progess (that's what the
syzcaller reproducer was doing).
Byte,
Johannes
Re: [PATCHv3 3/6] sg: protect accesses to 'reserved' page array
On Fri, 2017-02-03 at 19:06 +0100, Johannes Thumshirn wrote:
>
> On 02/03/2017 05:19 PM, Christoph Hellwig wrote:
> > On Fri, Feb 03, 2017 at 02:38:35PM +0100, Hannes Reinecke wrote:
> > > On 02/03/2017 02:31 PM, Christoph Hellwig wrote:
> > > > > - if (sg_res_in_use(sfp)) {
> > > > > + mutex_lock(&sfp->f_mutex);
> > > > > + if (sfp->res_in_use) {
> > > > > + mutex_unlock(&sfp->f_mutex);
> > > > > sg_remove_request(sfp, srp);
> > > > > return -EBUSY; /* reserve
> > > > > buffer already being used */
> > > > > }
> > > > > + mutex_unlock(&sfp->f_mutex);
> > > > Holding a mutex over a the check of a single scalar doesn't
> > > > make sense.
> > > >
> > > It's adds a synchronisation point, doesn't it?
> > It does, but it doesn't actually protect anything..
>
> But all the other mutex_{un,}locks() do (for instance guarding
> sg_build_indirect()) and this one provides a synchronization point.
>
> Sorry but I really don't get your point here.
>
> The sole purpose is to guard the reserved list from being altered
> while blk_rq_map_* or similar functions are in progess (that's what
> the syzcaller reproducer was doing).
What he means is that naturally aligned reads are always atomic, so
adding further synchronisation gains nothing (you already atomically
get either the prior or next value) and only causes an unnecessary
pipeline stall. From a reviewer's perspective, the sequence
lock
read
unlock
is always a red flag because it means the writer may not understand how
locking works. Usually because the writer thinks there's some other
synchronization they need that this sequence doesn't give.
James
[PATCH V3 0/2] Add QLogic FastLinQ FCoE (qedf) driver
From: "Dupuis, Chad"
Dave, please apply the qed patch to net-next at your earliest convenience.
Martin, the qed patch needs to be applied first as the qedf patch is dependent
on the FCoE bits in the first qed driver patch.
This series introduces the hardware offload FCoE initiator driver for the
41000 Series Converged Network Adapters (579xx chip) by Cavium. The overall
driver design includes a common module ('qed') and protocol specific
dependent modules ('qedf' for FCoE).
This driver uses the kernel components of libfc and libfcoe as is and does not
make use of the open-fcoe user space components. Therefore, no changes will
need to be
made to any open-fcoe components.
The 'qed' common module, under drivers/net/ethernet/qlogic/qed/, is
enhanced with functionality required for FCoE support.
Changes from V2 -> V3
- Fix uninitialized variables reported by kbuild robot in qedf
- Remove superfluous comments from qedf.h
- Introduce new qedf_ctx flag to different stopping I/O for debug purposes.
- Don't take lport->disc.disc_mutex when restarting an rport.
- Remove extra whitespace in qedf_hsi.h
Changes from V1 -> V2
Changes in qed:
- Fix compiler warning when CONFIG_DCB is not set.
Fixes in qedf:
- Add qedf to scsi directory Makefile.
- Updates to convert LightL2 and I/O processing kthreads to workqueues.
Changes from RFC -> V1
- Squash qedf patches to one patch now that the initial review has taken place
- Convert qedf to use hotplug state machine
- Return via va_end to match corresponding va_start in logging functions
- Convert qedf_ctx offloaded port list to a RCU list so searches do not need
to make use of spinlocks. Also eliminates the need to fcport conn_id's.
- Use IS_ERR(fp) in qedf_flogi_resp() instead of checking individual FC_EX_*
errors.
- Remove scsi_block_target when executing TMF request.
- Checkpatch fixes in the qed and qedf patches
Arun Easi (1):
qed: Add support for hardware offloaded FCoE.
Dupuis, Chad (1):
qedf: Add QLogic FastLinQ offload FCoE driver framework.
MAINTAINERS |6 +
drivers/net/ethernet/qlogic/Kconfig |3 +
drivers/net/ethernet/qlogic/qed/Makefile |1 +
drivers/net/ethernet/qlogic/qed/qed.h | 11 +
drivers/net/ethernet/qlogic/qed/qed_cxt.c | 98 +-
drivers/net/ethernet/qlogic/qed/qed_cxt.h |3 +
drivers/net/ethernet/qlogic/qed/qed_dcbx.c| 13 +-
drivers/net/ethernet/qlogic/qed/qed_dcbx.h|5 +-
drivers/net/ethernet/qlogic/qed/qed_dev.c | 205 +-
drivers/net/ethernet/qlogic/qed/qed_dev_api.h | 42 +
drivers/net/ethernet/qlogic/qed/qed_fcoe.c| 990 ++
drivers/net/ethernet/qlogic/qed/qed_fcoe.h| 52 +
drivers/net/ethernet/qlogic/qed/qed_hsi.h | 781 -
drivers/net/ethernet/qlogic/qed/qed_hw.c |3 +
drivers/net/ethernet/qlogic/qed/qed_ll2.c | 25 +
drivers/net/ethernet/qlogic/qed/qed_ll2.h |2 +-
drivers/net/ethernet/qlogic/qed/qed_main.c|7 +
drivers/net/ethernet/qlogic/qed/qed_mcp.c |3 +
drivers/net/ethernet/qlogic/qed/qed_mcp.h |1 +
drivers/net/ethernet/qlogic/qed/qed_reg_addr.h|8 +
drivers/net/ethernet/qlogic/qed/qed_sp.h |4 +
drivers/net/ethernet/qlogic/qed/qed_sp_commands.c |3 +
drivers/scsi/Kconfig |1 +
drivers/scsi/Makefile |1 +
drivers/scsi/qedf/Kconfig | 11 +
drivers/scsi/qedf/Makefile|5 +
drivers/scsi/qedf/qedf.h | 545
drivers/scsi/qedf/qedf_attr.c | 165 +
drivers/scsi/qedf/qedf_dbg.c | 195 ++
drivers/scsi/qedf/qedf_dbg.h | 154 +
drivers/scsi/qedf/qedf_debugfs.c | 460 +++
drivers/scsi/qedf/qedf_els.c | 981 ++
drivers/scsi/qedf/qedf_fip.c | 269 ++
drivers/scsi/qedf/qedf_hsi.h | 422 +++
drivers/scsi/qedf/qedf_io.c | 2282 ++
drivers/scsi/qedf/qedf_main.c | 3336 +
drivers/scsi/qedf/qedf_version.h | 15 +
include/linux/qed/common_hsi.h| 10 +-
include/linux/qed/fcoe_common.h | 715 +
include/linux/qed/qed_fcoe_if.h | 145 +
include/linux/qed/qed_if.h| 41 +-
41 files changed, 12000 insertions(+), 19 deletions(-)
create mode 100644 drivers/net/ethernet/qlogic/qed/qed_fcoe.c
create mode 100644 drivers/net/ethernet/qlogic/qed/qed_fcoe.h
create mode 100644 drivers/scsi/qedf/Kconfig
create mode 100644 drivers/scsi/qedf/Makefile
create mode 100644 drivers/scsi/qedf/qedf.h
create mode 100644 drivers/scsi/qedf/qedf_attr.c
create mode 100644 drivers/scsi/qedf/qedf_dbg.c
create mode 10
[PATCH V3 net-next 1/2] qed: Add support for hardware offloaded FCoE.
From: Arun Easi
This adds the backbone required for the various HW initalizations
which are necessary for the FCoE driver (qedf) for QLogic FastLinQ
4 line of adapters - FW notification, resource initializations, etc.
Signed-off-by: Arun Easi
Signed-off-by: Yuval Mintz
---
drivers/net/ethernet/qlogic/Kconfig | 3 +
drivers/net/ethernet/qlogic/qed/Makefile | 1 +
drivers/net/ethernet/qlogic/qed/qed.h | 11 +
drivers/net/ethernet/qlogic/qed/qed_cxt.c | 98 ++-
drivers/net/ethernet/qlogic/qed/qed_cxt.h | 3 +
drivers/net/ethernet/qlogic/qed/qed_dcbx.c| 13 +-
drivers/net/ethernet/qlogic/qed/qed_dcbx.h| 5 +-
drivers/net/ethernet/qlogic/qed/qed_dev.c | 205 -
drivers/net/ethernet/qlogic/qed/qed_dev_api.h | 42 +
drivers/net/ethernet/qlogic/qed/qed_fcoe.c| 990 ++
drivers/net/ethernet/qlogic/qed/qed_fcoe.h| 52 ++
drivers/net/ethernet/qlogic/qed/qed_hsi.h | 781 -
drivers/net/ethernet/qlogic/qed/qed_hw.c | 3 +
drivers/net/ethernet/qlogic/qed/qed_ll2.c | 25 +
drivers/net/ethernet/qlogic/qed/qed_ll2.h | 2 +-
drivers/net/ethernet/qlogic/qed/qed_main.c| 7 +
drivers/net/ethernet/qlogic/qed/qed_mcp.c | 3 +
drivers/net/ethernet/qlogic/qed/qed_mcp.h | 1 +
drivers/net/ethernet/qlogic/qed/qed_reg_addr.h| 8 +
drivers/net/ethernet/qlogic/qed/qed_sp.h | 4 +
drivers/net/ethernet/qlogic/qed/qed_sp_commands.c | 3 +
include/linux/qed/common_hsi.h| 10 +-
include/linux/qed/fcoe_common.h | 715
include/linux/qed/qed_fcoe_if.h | 145
include/linux/qed/qed_if.h| 41 +-
25 files changed, 3152 insertions(+), 19 deletions(-)
create mode 100644 drivers/net/ethernet/qlogic/qed/qed_fcoe.c
create mode 100644 drivers/net/ethernet/qlogic/qed/qed_fcoe.h
create mode 100644 include/linux/qed/fcoe_common.h
create mode 100644 include/linux/qed/qed_fcoe_if.h
diff --git a/drivers/net/ethernet/qlogic/Kconfig
b/drivers/net/ethernet/qlogic/Kconfig
index 3cfd105..737b303 100644
--- a/drivers/net/ethernet/qlogic/Kconfig
+++ b/drivers/net/ethernet/qlogic/Kconfig
@@ -113,4 +113,7 @@ config QED_RDMA
config QED_ISCSI
bool
+config QED_FCOE
+ bool
+
endif # NET_VENDOR_QLOGIC
diff --git a/drivers/net/ethernet/qlogic/qed/Makefile
b/drivers/net/ethernet/qlogic/qed/Makefile
index 729e437..e234083 100644
--- a/drivers/net/ethernet/qlogic/qed/Makefile
+++ b/drivers/net/ethernet/qlogic/qed/Makefile
@@ -7,3 +7,4 @@ qed-$(CONFIG_QED_SRIOV) += qed_sriov.o qed_vf.o
qed-$(CONFIG_QED_LL2) += qed_ll2.o
qed-$(CONFIG_QED_RDMA) += qed_roce.o
qed-$(CONFIG_QED_ISCSI) += qed_iscsi.o qed_ooo.o
+qed-$(CONFIG_QED_FCOE) += qed_fcoe.o
diff --git a/drivers/net/ethernet/qlogic/qed/qed.h
b/drivers/net/ethernet/qlogic/qed/qed.h
index 1f61cf3..08f2885 100644
--- a/drivers/net/ethernet/qlogic/qed/qed.h
+++ b/drivers/net/ethernet/qlogic/qed/qed.h
@@ -60,6 +60,7 @@
#define QED_WFQ_UNIT 100
#define ISCSI_BDQ_ID(_port_id) (_port_id)
+#define FCOE_BDQ_ID(_port_id) ((_port_id) + 2)
#define QED_WID_SIZE(1024)
#define QED_PF_DEMS_SIZE(4)
@@ -167,6 +168,7 @@ struct qed_tunn_update_params {
*/
enum qed_pci_personality {
QED_PCI_ETH,
+ QED_PCI_FCOE,
QED_PCI_ISCSI,
QED_PCI_ETH_ROCE,
QED_PCI_DEFAULT /* default in shmem */
@@ -204,6 +206,7 @@ enum QED_FEATURE {
QED_VF,
QED_RDMA_CNQ,
QED_VF_L2_QUE,
+ QED_FCOE_CQ,
QED_MAX_FEATURES,
};
@@ -221,6 +224,7 @@ enum QED_PORT_MODE {
enum qed_dev_cap {
QED_DEV_CAP_ETH,
+ QED_DEV_CAP_FCOE,
QED_DEV_CAP_ISCSI,
QED_DEV_CAP_ROCE,
};
@@ -255,6 +259,10 @@ struct qed_hw_info {
u32 part_num[4];
unsigned char hw_mac_addr[ETH_ALEN];
+ u64 node_wwn;
+ u64 port_wwn;
+
+ u16 num_fcoe_conns;
struct qed_igu_info *p_igu_info;
@@ -410,6 +418,7 @@ struct qed_hwfn {
struct qed_ooo_info *p_ooo_info;
struct qed_rdma_info*p_rdma_info;
struct qed_iscsi_info *p_iscsi_info;
+ struct qed_fcoe_info*p_fcoe_info;
struct qed_pf_paramspf_params;
bool b_rdma_enabled_in_prs;
@@ -618,11 +627,13 @@ struct qed_dev {
u8 protocol;
#define IS_QED_ETH_IF(cdev) ((cdev)->protocol == QED_PROTOCOL_ETH)
+#define IS_QED_FCOE_IF(cdev)((cdev)->protocol == QED_PROTOCOL_FCOE)
/* Callbacks to protocol driver */
union {
struct qed_common_cb_ops*common;
struct qed_eth_cb_ops *eth;
+ struct qed_fcoe_cb_o
[PATCH] scsi, block: fix memory leak of sdpk on when gd fails to allocate
From: Colin Ian King
On an allocation failure of gd, the current exit path is via out_free_devt
which leaves sdpk still allocated and hence it gets leaked. Fix this by
correcting the order of resource free'ing with a change in the error exit
path labels.
Detected by CoverityScan, CID#1399519 ("Resource Leak")
Fixes: 0dba1314d4f81115dc ("scsi, block: fix duplicate bdi name registration
crashes")
Signed-off-by: Colin Ian King
---
drivers/scsi/sd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index cb6e68d..99e1206 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3213,10 +3213,10 @@ static int sd_probe(struct device *dev)
sd_devt = NULL;
out_put:
put_disk(gd);
- out_free:
- kfree(sdkp);
out_free_devt:
kfree(sd_devt);
+ out_free:
+ kfree(sdkp);
out:
scsi_autopm_put_device(sdp);
return error;
--
2.10.2
[GIT PULL] SCSI fixes for 4.10-rc6
A single fix this time: a fix for a virtqueue removal bug which only
appears to affect S390, but which results in the queue hanging forever
thus causing the machine to fail shutdown.
The patch is available here:
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes
The short changelog is:
Eric Farman (1):
scsi: virtio_scsi: Reject commands when virtqueue is broken
And the diffstat:
drivers/scsi/virtio_scsi.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
With full diff below.
James
---
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index ec91bd0..c680d76 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -534,7 +534,9 @@ static int virtscsi_queuecommand(struct virtio_scsi *vscsi,
{
struct Scsi_Host *shost = virtio_scsi_host(vscsi->vdev);
struct virtio_scsi_cmd *cmd = scsi_cmd_priv(sc);
+ unsigned long flags;
int req_size;
+ int ret;
BUG_ON(scsi_sg_count(sc) > shost->sg_tablesize);
@@ -562,8 +564,15 @@ static int virtscsi_queuecommand(struct virtio_scsi *vscsi,
req_size = sizeof(cmd->req.cmd);
}
- if (virtscsi_kick_cmd(req_vq, cmd, req_size, sizeof(cmd->resp.cmd)) !=
0)
+ ret = virtscsi_kick_cmd(req_vq, cmd, req_size, sizeof(cmd->resp.cmd));
+ if (ret == -EIO) {
+ cmd->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
+ spin_lock_irqsave(&req_vq->vq_lock, flags);
+ virtscsi_complete_cmd(vscsi, cmd);
+ spin_unlock_irqrestore(&req_vq->vq_lock, flags);
+ } else if (ret != 0) {
return SCSI_MLQUEUE_HOST_BUSY;
+ }
return 0;
}
[PATCH v2 03/14] qla2xxx: Allow vref count to timeout on vport delete.
From: Joe Carnuccio
Signed-off-by: Joe Carnuccio
Signed-off-by: Himanshu Madhani
---
drivers/scsi/qla2xxx/qla_attr.c | 4 +---
drivers/scsi/qla2xxx/qla_mid.c | 12 +++-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index f610103..435ff7f 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -2154,8 +2154,6 @@ struct device_attribute *qla2x00_host_attrs[] = {
"Timer for the VP[%d] has stopped\n", vha->vp_idx);
}
- BUG_ON(atomic_read(&vha->vref_count));
-
qla2x00_free_fcports(vha);
mutex_lock(&ha->vport_lock);
@@ -2166,7 +2164,7 @@ struct device_attribute *qla2x00_host_attrs[] = {
dma_free_coherent(&ha->pdev->dev, vha->gnl.size, vha->gnl.l,
vha->gnl.ldma);
- if (vha->qpair->vp_idx == vha->vp_idx) {
+ if (vha->qpair && vha->qpair->vp_idx == vha->vp_idx) {
if (qla2xxx_delete_qpair(vha, vha->qpair) != QLA_SUCCESS)
ql_log(ql_log_warn, vha, 0x7087,
"Queue Pair delete failed.\n");
diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
index c6d6f0d..bf8e1e2 100644
--- a/drivers/scsi/qla2xxx/qla_mid.c
+++ b/drivers/scsi/qla2xxx/qla_mid.c
@@ -65,6 +65,7 @@
uint16_t vp_id;
struct qla_hw_data *ha = vha->hw;
unsigned long flags = 0;
+ unsigned int count = 10;
mutex_lock(&ha->vport_lock);
/*
@@ -74,13 +75,14 @@
* ensures no active vp_list traversal while the vport is removed
* from the queue)
*/
- spin_lock_irqsave(&ha->vport_slock, flags);
- while (atomic_read(&vha->vref_count)) {
- spin_unlock_irqrestore(&ha->vport_slock, flags);
-
+ while (count-- && atomic_read(&vha->vref_count))
msleep(500);
- spin_lock_irqsave(&ha->vport_slock, flags);
+ spin_lock_irqsave(&ha->vport_slock, flags);
+ if (atomic_read(&vha->vref_count)) {
+ ql_dbg(ql_dbg_vport, vha, 0xfffa,
+ "vha->vref_count=%u timeout\n", vha->vref_count.counter);
+ vha->vref_count = (atomic_t)ATOMIC_INIT(0);
}
list_del(&vha->list);
qlt_update_vp_map(vha, RESET_VP_IDX);
--
1.8.3.1
[PATCH v2 04/14] qla2xxx: Use IOCB interface to submit non-critical MBX.
From: Quinn Tran
The Mailbox interface is currently over subscribed. We like
to reserve the Mailbox interface for the chip managment and
link initialization. Any non essential Mailbox command will
be routed through the IOCB interface. The IOCB interface is
able to absorb more commands.
Following commands are being routed through IOCB interface
- Get ID List (007Ch)
- Get Port DB (0064h)
- Get Link Priv Stats (006Dh)
Signed-off-by: Quinn Tran
Signed-off-by: Himanshu Madhani
---
drivers/scsi/qla2xxx/qla_def.h| 8 +-
drivers/scsi/qla2xxx/qla_gbl.h| 10 +-
drivers/scsi/qla2xxx/qla_init.c | 46 +--
drivers/scsi/qla2xxx/qla_isr.c| 2 +-
drivers/scsi/qla2xxx/qla_mbx.c| 273 --
drivers/scsi/qla2xxx/qla_target.c | 4 +-
6 files changed, 279 insertions(+), 64 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index bf2ae8d3..f92977b 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -395,11 +395,15 @@ struct srb_iocb {
struct completion comp;
} abt;
struct ct_arg ctarg;
+#define MAX_IOCB_MB_REG 28
+#define SIZEOF_IOCB_MB_REG (MAX_IOCB_MB_REG * sizeof(uint16_t))
struct {
- __le16 in_mb[28]; /* fr fw */
- __le16 out_mb[28]; /* to fw */
+ __le16 in_mb[MAX_IOCB_MB_REG]; /* from FW */
+ __le16 out_mb[MAX_IOCB_MB_REG]; /* to FW */
void *out, *in;
dma_addr_t out_dma, in_dma;
+ struct completion comp;
+ int rc;
} mbx;
struct {
struct imm_ntfy_from_isp *ntfy;
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index 862d5f5..48a1190 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -194,6 +194,7 @@ void qla2x00_handle_login_done_event(struct scsi_qla_host
*, fc_port_t *,
uint16_t *);
int qla24xx_post_gnl_work(struct scsi_qla_host *, fc_port_t *);
void qla2x00_schedule_work(struct scsi_qla_host *);
+int qla24xx_async_abort_cmd(srb_t *);
/*
* Global Functions in qla_mid.c source file.
@@ -369,7 +370,7 @@ extern int qla24xx_build_scsi_crc_2_iocbs(srb_t *,
extern int
qla24xx_get_isp_stats(scsi_qla_host_t *, struct link_statistics *,
-dma_addr_t, uint);
+dma_addr_t, uint16_t);
extern int qla24xx_abort_command(srb_t *);
extern int qla24xx_async_abort_command(srb_t *);
@@ -473,6 +474,13 @@ extern int qla24xx_set_fcp_prio(scsi_qla_host_t *,
uint16_t, uint16_t,
extern int
qla26xx_dport_diagnostics(scsi_qla_host_t *, void *, uint, uint);
+int qla24xx_send_mb_cmd(struct scsi_qla_host *, mbx_cmd_t *);
+int qla24xx_gpdb_wait(struct scsi_qla_host *, fc_port_t *, u8);
+int qla24xx_gidlist_wait(struct scsi_qla_host *, void *, dma_addr_t,
+uint16_t *);
+int __qla24xx_parse_gpdb(struct scsi_qla_host *, fc_port_t *,
+ struct port_database_24xx *);
+
/*
* Global Function Prototypes in qla_isr.c source file.
*/
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index e84ab37..5aca3df 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -629,7 +629,6 @@ void qla24xx_async_gpdb_sp_done(void *s, int res)
struct srb *sp = s;
struct scsi_qla_host *vha = sp->vha;
struct qla_hw_data *ha = vha->hw;
- uint64_t zero = 0;
struct port_database_24xx *pd;
fc_port_t *fcport = sp->fcport;
u16 *mb = sp->u.iocb_cmd.u.mbx.in_mb;
@@ -649,48 +648,7 @@ void qla24xx_async_gpdb_sp_done(void *s, int res)
pd = (struct port_database_24xx *)sp->u.iocb_cmd.u.mbx.in;
- /* Check for logged in state. */
- if (pd->current_login_state != PDS_PRLI_COMPLETE &&
- pd->last_login_state != PDS_PRLI_COMPLETE) {
- ql_dbg(ql_dbg_mbx, vha, 0x,
- "Unable to verify login-state (%x/%x) for "
- "loop_id %x.\n", pd->current_login_state,
- pd->last_login_state, fcport->loop_id);
- rval = QLA_FUNCTION_FAILED;
- goto gpd_error_out;
- }
-
- if (fcport->loop_id == FC_NO_LOOP_ID ||
- (memcmp(fcport->port_name, (uint8_t *)&zero, 8) &&
- memcmp(fcport->port_name, pd->port_name, 8))) {
- /* We lost the device mid way. */
- rval = QLA_NOT_LOGGED_IN;
- goto gpd_error_out;
- }
-
- /* Names are little-endian. */
- memcpy(fcport->node_name, pd->node_name, WWN_SIZE);
-
- /* Get port_id of device. */
- fcport->d_id.b.domain = pd->port_id[0];
- fcport->d_id.b.area = pd->port_id[1];
- fcport->d_id.b.al_pa = pd->port_id[2];
- fcport->d_id.b.rsvd_1 = 0;
-
- /* If not targe
[PATCH v2 14/14] qla2xxx: Update driver version to 9.00.00.00-k
Signed-off-by: Himanshu Madhani signed-off-by: Giridhar Malavali --- drivers/scsi/qla2xxx/qla_version.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_version.h b/drivers/scsi/qla2xxx/qla_version.h index 3cb1964..45bc84e 100644 --- a/drivers/scsi/qla2xxx/qla_version.h +++ b/drivers/scsi/qla2xxx/qla_version.h @@ -7,9 +7,9 @@ /* * Driver version */ -#define QLA2XXX_VERSION "8.07.00.38-k" +#define QLA2XXX_VERSION "9.00.00.00-k" -#define QLA_DRIVER_MAJOR_VER 8 -#define QLA_DRIVER_MINOR_VER 7 +#define QLA_DRIVER_MAJOR_VER 9 +#define QLA_DRIVER_MINOR_VER 0 #define QLA_DRIVER_PATCH_VER 0 #define QLA_DRIVER_BETA_VER0 -- 1.8.3.1
[PATCH v2 10/14] qla2xxx: Fix request queue corruption.
From: Quinn Tran
When FW notify driver or driver detects low FW resource,
driver tries to send out Busy SCSI Status to tell Initiator
side to back off. During the send process, the lock was not held.
Signed-off-by: Quinn Tran
Signed-off-by: Himanshu Madhani
---
drivers/scsi/qla2xxx/qla_target.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c
b/drivers/scsi/qla2xxx/qla_target.c
index b61cbb8..b5fb9c55 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -5170,16 +5170,22 @@ static int __qlt_send_busy(struct scsi_qla_host *vha,
static int
qlt_chk_qfull_thresh_hold(struct scsi_qla_host *vha,
- struct atio_from_isp *atio)
+ struct atio_from_isp *atio, uint8_t ha_locked)
{
struct qla_hw_data *ha = vha->hw;
uint16_t status;
+ unsigned long flags;
if (ha->tgt.num_pend_cmds < Q_FULL_THRESH_HOLD(ha))
return 0;
+ if (!ha_locked)
+ spin_lock_irqsave(&ha->hardware_lock, flags);
status = temp_sam_status;
qlt_send_busy(vha, atio, status);
+ if (!ha_locked)
+ spin_unlock_irqrestore(&ha->hardware_lock, flags);
+
return 1;
}
@@ -5224,7 +5230,7 @@ static void qlt_24xx_atio_pkt(struct scsi_qla_host *vha,
if (likely(atio->u.isp24.fcp_cmnd.task_mgmt_flags == 0)) {
- rc = qlt_chk_qfull_thresh_hold(vha, atio);
+ rc = qlt_chk_qfull_thresh_hold(vha, atio, ha_locked);
if (rc != 0) {
tgt->atio_irq_cmd_count--;
return;
@@ -5347,7 +5353,7 @@ static void qlt_response_pkt(struct scsi_qla_host *vha,
response_t *pkt)
break;
}
- rc = qlt_chk_qfull_thresh_hold(vha, atio);
+ rc = qlt_chk_qfull_thresh_hold(vha, atio, 1);
if (rc != 0) {
tgt->irq_cmd_count--;
return;
--
1.8.3.1
[PATCH v2 05/14] qla2xxx: Add DebugFS node to display Port Database
Signed-off-by: Himanshu Madhani
Signed-off-by: Giridhar Malavali
---
drivers/scsi/qla2xxx/qla_def.h | 2 +
drivers/scsi/qla2xxx/qla_dfs.c | 92 --
2 files changed, 90 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index f92977b..8bc 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -3278,6 +3278,8 @@ struct qlt_hw_data {
uint8_t tgt_node_name[WWN_SIZE];
struct dentry *dfs_tgt_sess;
+ struct dentry *dfs_tgt_port_database;
+
struct list_head q_full_list;
uint32_t num_pend_cmds;
uint32_t num_qfull_cmds_alloc;
diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c
index b48cce6..0fb33e6 100644
--- a/drivers/scsi/qla2xxx/qla_dfs.c
+++ b/drivers/scsi/qla2xxx/qla_dfs.c
@@ -19,11 +19,11 @@
struct qla_hw_data *ha = vha->hw;
unsigned long flags;
struct fc_port *sess = NULL;
- struct qla_tgt *tgt= vha->vha_tgt.qla_tgt;
+ struct qla_tgt *tgt = vha->vha_tgt.qla_tgt;
- seq_printf(s, "%s\n",vha->host_str);
+ seq_printf(s, "%s\n", vha->host_str);
if (tgt) {
- seq_printf(s, "Port ID Port NameHandle\n");
+ seq_puts(s, "Port ID Port NameHandle\n");
spin_lock_irqsave(&ha->tgt.sess_lock, flags);
list_for_each_entry(sess, &vha->vp_fcports, list)
@@ -44,7 +44,6 @@
return single_open(file, qla2x00_dfs_tgt_sess_show, vha);
}
-
static const struct file_operations dfs_tgt_sess_ops = {
.open = qla2x00_dfs_tgt_sess_open,
.read = seq_read,
@@ -53,6 +52,78 @@
};
static int
+qla2x00_dfs_tgt_port_database_show(struct seq_file *s, void *unused)
+{
+ scsi_qla_host_t *vha = s->private;
+ struct qla_hw_data *ha = vha->hw;
+ struct gid_list_info *gid_list;
+ dma_addr_t gid_list_dma;
+ fc_port_t fc_port;
+ char *id_iter;
+ int rc, i;
+ uint16_t entries, loop_id;
+ struct qla_tgt *tgt = vha->vha_tgt.qla_tgt;
+
+ seq_printf(s, "%s\n", vha->host_str);
+ if (tgt) {
+ gid_list = dma_alloc_coherent(&ha->pdev->dev,
+ qla2x00_gid_list_size(ha),
+ &gid_list_dma, GFP_KERNEL);
+ if (!gid_list) {
+ ql_dbg(ql_dbg_user, vha, 0x705c,
+ "DMA allocation failed for %u\n",
+qla2x00_gid_list_size(ha));
+ return 0;
+ }
+
+ rc = qla24xx_gidlist_wait(vha, gid_list, gid_list_dma,
+ &entries);
+ if (rc != QLA_SUCCESS)
+ goto out_free_id_list;
+
+ id_iter = (char *)gid_list;
+
+ seq_puts(s, "Port Name Port ID Loop ID\n");
+
+ for (i = 0; i < entries; i++) {
+ struct gid_list_info *gid =
+ (struct gid_list_info *)id_iter;
+ loop_id = le16_to_cpu(gid->loop_id);
+ memset(&fc_port, 0, sizeof(fc_port_t));
+
+ fc_port.loop_id = loop_id;
+
+ rc = qla24xx_gpdb_wait(vha, &fc_port, 0);
+ seq_printf(s, "%8phC %02x%02x%02x %d\n",
+ fc_port.port_name, fc_port.d_id.b.domain,
+ fc_port.d_id.b.area, fc_port.d_id.b.al_pa,
+ fc_port.loop_id);
+ id_iter += ha->gid_list_info_size;
+ }
+out_free_id_list:
+ dma_free_coherent(&ha->pdev->dev, qla2x00_gid_list_size(ha),
+ gid_list, gid_list_dma);
+ }
+
+ return 0;
+}
+
+static int
+qla2x00_dfs_tgt_port_database_open(struct inode *inode, struct file *file)
+{
+ scsi_qla_host_t *vha = inode->i_private;
+
+ return single_open(file, qla2x00_dfs_tgt_port_database_show, vha);
+}
+
+static const struct file_operations dfs_tgt_port_database_ops = {
+ .open = qla2x00_dfs_tgt_port_database_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release= single_release,
+};
+
+static int
qla_dfs_fw_resource_cnt_show(struct seq_file *s, void *unused)
{
struct scsi_qla_host *vha = s->private;
@@ -281,6 +352,14 @@
goto out;
}
+ ha->tgt.dfs_tgt_port_database = debugfs_create_file("tgt_port_database",
+ S_IRUSR, ha->dfs_dir, vha, &dfs_tgt_port_database_ops);
+ if (!ha->tgt.dfs_tgt_port_database) {
+ ql_log(ql_log_warn, vha, 0x,
+ "Unable to create debugFS tgt_port_database node.\n");
+ goto out;
+ }
+
ha->dfs_fce = debugfs_create_file("fce", S_IRUSR, ha->dfs_dir, vha,
&d
[PATCH v2 01/14] qla2xxx: Fix delayed response to command for loop mode/direct connect.
From: Quinn Tran
Current driver wait for FW to be in the ready state before
processing in-coming commands. For Arbitrated Loop or
Point-to- Point (not switch), FW Ready state can take a while.
FW will transition to ready state after all Nports have been
logged in. In the mean time, certain initiators have completed
the login and starts IO. Driver needs to start processing all
queues if FW is already started.
Signed-off-by: Quinn Tran
Signed-off-by: Himanshu Madhani
---
drivers/scsi/qla2xxx/qla_def.h| 13 +--
drivers/scsi/qla2xxx/qla_gbl.h| 1 +
drivers/scsi/qla2xxx/qla_init.c | 12 +++
drivers/scsi/qla2xxx/qla_isr.c| 14 +++-
drivers/scsi/qla2xxx/qla_mbx.c| 6 +++---
drivers/scsi/qla2xxx/qla_os.c | 45 ++-
drivers/scsi/qla2xxx/qla_target.c | 38 ++---
7 files changed, 109 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index ac37e91..fbaa39b 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -3290,6 +3290,10 @@ struct qlt_hw_data {
#define LEAK_EXCHG_THRESH_HOLD_PERCENT 75 /* 75 percent */
+#define QLA_EARLY_LINKUP(_ha) \
+ ((_ha->flags.n2n_ae || _ha->flags.lip_ae) && \
+_ha->flags.fw_started && !_ha->flags.fw_init_done)
+
/*
* Qlogic host adapter specific data structure.
*/
@@ -3339,7 +3343,11 @@ struct qla_hw_data {
uint32_tfawwpn_enabled:1;
uint32_texlogins_enabled:1;
uint32_texchoffld_enabled:1;
- /* 35 bits */
+
+ uint32_tlip_ae:1;
+ uint32_tn2n_ae:1;
+ uint32_tfw_started:1;
+ uint32_tfw_init_done:1;
} flags;
/* This spinlock is used to protect "io transactions", you must
@@ -3432,7 +3440,6 @@ struct qla_hw_data {
#define P2P_LOOP 3
uint8_t interrupts_on;
uint32_tisp_abort_cnt;
-
#define PCI_DEVICE_ID_QLOGIC_ISP25320x2532
#define PCI_DEVICE_ID_QLOGIC_ISP84320x8432
#define PCI_DEVICE_ID_QLOGIC_ISP8001 0x8001
@@ -3913,6 +3920,7 @@ struct qla_tgt_counters {
struct list_head vp_fcports;/* list of fcports */
struct list_head work_list;
spinlock_t work_lock;
+ struct work_struct iocb_work;
/* Commonly used flags and state information. */
struct Scsi_Host *host;
@@ -3932,6 +3940,7 @@ struct qla_tgt_counters {
uint32_tfw_tgt_reported:1;
uint32_tbbcr_enable:1;
uint32_tqpairs_available:1;
+ uint32_tiocb_work_sheduled:1;
} flags;
atomic_tloop_state;
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index b3d6441..862d5f5 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -193,6 +193,7 @@ extern struct scsi_qla_host *qla2x00_create_host(struct
scsi_host_template *,
void qla2x00_handle_login_done_event(struct scsi_qla_host *, fc_port_t *,
uint16_t *);
int qla24xx_post_gnl_work(struct scsi_qla_host *, fc_port_t *);
+void qla2x00_schedule_work(struct scsi_qla_host *);
/*
* Global Functions in qla_mid.c source file.
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index f654314..9f3db52 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -3212,6 +3212,7 @@ void qla2x00_fcport_event_handler(scsi_qla_host_t *vha,
struct event_arg *ea)
} else {
ql_dbg(ql_dbg_init, vha, 0x00d3,
"Init Firmware -- success.\n");
+ ha->flags.fw_started = 1;
}
return (rval);
@@ -4036,6 +4037,7 @@ static void qla2xxx_nvram_wwn_from_ofw(scsi_qla_host_t
*vha, nvram_t *nv)
atomic_set(&vha->loop_state, LOOP_READY);
ql_dbg(ql_dbg_disc, vha, 0x2069,
"LOOP READY.\n");
+ ha->flags.fw_init_done = 1;
/*
* Process any ATIO queue entries that came in
@@ -5526,6 +5528,11 @@ int qla2x00_perform_loop_resync(scsi_qla_host_t *ha)
if (!(IS_P3P_TYPE(ha)))
ha->isp_ops->reset_chip(vha);
+ ha->flags.n2n_ae = 0;
+ ha->flags.lip_ae = 0;
+ ha->current_topology = 0;
+ ha->flags.fw_started = 0;
+ ha->flags.fw_init_done = 0;
ha->chip_reset++;
atomic_set(&vha->loop_down_timer, LOOP_DOWN_TIME);
@@ -6802,6 +6809,8 @@ uint8_t qla27xx_find_valid_image(struct scsi_qla_host
*vha)
return;
if (!ha->fw_major_version)
return;
+ if (!ha->flags.fw_started)
+ return;
ret = qla2x00_stop_firmware(vha);
for (retries = 5; ret != QLA_SUCCESS
[PATCH v2 02/14] qla2xxx: Allow relogin to proceed if remote login did not finish
From: Quinn Tran
If the remote port have started the login process, then the
PLOGI and PRLI should be back to back. Driver will allow
the remote port to complete the process. For the case where
the remote port decide to back off from sending PRLI, this
local port sets an expiration timer for the PRLI. Once the
expiration time passes, the relogin retry logic is allowed
to go through and perform login with the remote port.
Signed-off-by: Quinn Tran
Signed-off-by: Himanshu Madhani
---
drivers/scsi/qla2xxx/qla_def.h| 2 ++
drivers/scsi/qla2xxx/qla_init.c | 16 ++--
drivers/scsi/qla2xxx/qla_isr.c| 25 +++--
drivers/scsi/qla2xxx/qla_target.c | 1 +
4 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index fbaa39b..bf2ae8d3 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -2300,6 +2300,8 @@ enum rscn_addr_format {
struct ct_sns_desc ct_desc;
enum discovery_state disc_state;
enum login_state fw_login_state;
+ unsigned long plogi_nack_done_jiff;
+
u32 login_gen, last_login_gen;
u32 rscn_gen, last_rscn_gen;
u32 chip_reset;
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 9f3db52..e84ab37 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -876,10 +876,16 @@ int qla24xx_fcport_handle_login(struct scsi_qla_host
*vha, fc_port_t *fcport)
fcport->login_retry--;
if ((fcport->fw_login_state == DSC_LS_PLOGI_PEND) ||
- (fcport->fw_login_state == DSC_LS_PLOGI_COMP) ||
(fcport->fw_login_state == DSC_LS_PRLI_PEND))
return 0;
+ if (fcport->fw_login_state == DSC_LS_PLOGI_COMP) {
+ unsigned long t = fcport->plogi_nack_done_jiff + HZ;
+
+ if (time_before_eq(jiffies, t))
+ return 0;
+ }
+
/* for pure Target Mode. Login will not be initiated */
if (vha->host->active_mode == MODE_TARGET)
return 0;
@@ -1041,10 +1047,16 @@ void qla24xx_handle_relogin_event(scsi_qla_host_t *vha,
fcport->flags);
if ((fcport->fw_login_state == DSC_LS_PLOGI_PEND) ||
- (fcport->fw_login_state == DSC_LS_PLOGI_COMP) ||
(fcport->fw_login_state == DSC_LS_PRLI_PEND))
return;
+ if (fcport->fw_login_state == DSC_LS_PLOGI_COMP) {
+ unsigned long t = fcport->plogi_nack_done_jiff + HZ;
+
+ if (time_before_eq(jiffies, t))
+ return;
+ }
+
if (fcport->flags & FCF_ASYNC_SENT) {
fcport->login_retry++;
set_bit(RELOGIN_NEEDED, &vha->dpc_flags);
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 0fd3258..f1da53d 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1630,9 +1630,9 @@ static void qla2x00_error_entry(scsi_qla_host_t *, struct
rsp_que *,
QLA_LOGIO_LOGIN_RETRIED : 0;
if (logio->entry_status) {
ql_log(ql_log_warn, fcport->vha, 0x5034,
- "Async-%s error entry - hdl=%x"
+ "Async-%s error entry - %8phC hdl=%x"
"portid=%02x%02x%02x entry-status=%x.\n",
- type, sp->handle, fcport->d_id.b.domain,
+ type, fcport->port_name, sp->handle, fcport->d_id.b.domain,
fcport->d_id.b.area, fcport->d_id.b.al_pa,
logio->entry_status);
ql_dump_buffer(ql_dbg_async + ql_dbg_buffer, vha, 0x504d,
@@ -1643,8 +1643,9 @@ static void qla2x00_error_entry(scsi_qla_host_t *, struct
rsp_que *,
if (le16_to_cpu(logio->comp_status) == CS_COMPLETE) {
ql_dbg(ql_dbg_async, fcport->vha, 0x5036,
- "Async-%s complete - hdl=%x portid=%02x%02x%02x "
- "iop0=%x.\n", type, sp->handle, fcport->d_id.b.domain,
+ "Async-%s complete - %8phC hdl=%x portid=%02x%02x%02x "
+ "iop0=%x.\n", type, fcport->port_name, sp->handle,
+ fcport->d_id.b.domain,
fcport->d_id.b.area, fcport->d_id.b.al_pa,
le32_to_cpu(logio->io_parameter[0]));
@@ -1684,6 +1685,17 @@ static void qla2x00_error_entry(scsi_qla_host_t *,
struct rsp_que *,
case LSC_SCODE_NPORT_USED:
data[0] = MBS_LOOP_ID_USED;
break;
+ case LSC_SCODE_CMD_FAILED:
+ if (iop[1] == 0x0606) {
+ /*
+* PLOGI/PRLI Completed. We must have Recv PLOGI/PRLI,
+* Target side acked.
+*/
+ data[0] = MBS_COMMAND_COMPLETE;
+ goto logio_done;
+ }
+ data[0] = MBS_COMMAND_E
[PATCH v2 07/14] qla2xxx: Export DIF stats via debugfs
From: Anil Gurumurthy
Signed-off-by: Anil Gurumurthy
Signed-off-by: Himanshu Madhani
---
drivers/scsi/qla2xxx/qla_def.h | 12
drivers/scsi/qla2xxx/qla_dfs.c | 15 +++
2 files changed, 27 insertions(+)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index d6436fc..a15614d 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -3129,6 +3129,16 @@ struct qla_chip_state_84xx {
uint32_t gold_fw_version;
};
+struct qla_dif_statistics {
+ uint64_t dif_input_bytes;
+ uint64_t dif_output_bytes;
+ uint64_t dif_input_requests;
+ uint64_t dif_output_requests;
+ uint32_t dif_guard_err;
+ uint32_t dif_ref_tag_err;
+ uint32_t dif_app_tag_err;
+};
+
struct qla_statistics {
uint32_t total_isp_aborts;
uint64_t input_bytes;
@@ -3141,6 +3151,8 @@ struct qla_statistics {
uint32_t stat_max_pend_cmds;
uint32_t stat_max_qfull_cmds_alloc;
uint32_t stat_max_qfull_cmds_dropped;
+
+ struct qla_dif_statistics qla_dif_stats;
};
struct bidi_statistics {
diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c
index 0fb33e6..989e17b 100644
--- a/drivers/scsi/qla2xxx/qla_dfs.c
+++ b/drivers/scsi/qla2xxx/qla_dfs.c
@@ -185,6 +185,21 @@
seq_printf(s, "num Q full sent = %lld\n",
vha->tgt_counters.num_q_full_sent);
+ /* DIF stats */
+ seq_printf(s, "DIF Inp Bytes = %lld\n",
+ vha->qla_stats.qla_dif_stats.dif_input_bytes);
+ seq_printf(s, "DIF Outp Bytes = %lld\n",
+ vha->qla_stats.qla_dif_stats.dif_output_bytes);
+ seq_printf(s, "DIF Inp Req = %lld\n",
+ vha->qla_stats.qla_dif_stats.dif_input_requests);
+ seq_printf(s, "DIF Outp Req = %lld\n",
+ vha->qla_stats.qla_dif_stats.dif_output_requests);
+ seq_printf(s, "DIF Guard err = %d\n",
+ vha->qla_stats.qla_dif_stats.dif_guard_err);
+ seq_printf(s, "DIF Ref tag err = %d\n",
+ vha->qla_stats.qla_dif_stats.dif_ref_tag_err);
+ seq_printf(s, "DIF App tag err = %d\n",
+ vha->qla_stats.qla_dif_stats.dif_app_tag_err);
return 0;
}
--
1.8.3.1
[PATCH v2 06/14] qla2xxx: Improve T10-DIF/PI handling in driver.
From: Quinn Tran
Add routines to support T10 DIF tag.
Signed-off-by: Quinn Tran
Signed-off-by: Anil Gurumurthy
Signed-off-by: Himanshu Madhani
---
drivers/scsi/qla2xxx/qla_dbg.h | 1 +
drivers/scsi/qla2xxx/qla_def.h | 17 ++
drivers/scsi/qla2xxx/qla_target.c | 598 +
drivers/scsi/qla2xxx/qla_target.h | 37 ++-
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 84 +-
5 files changed, 465 insertions(+), 272 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_dbg.h b/drivers/scsi/qla2xxx/qla_dbg.h
index e1fc4e6..c6bffe9 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.h
+++ b/drivers/scsi/qla2xxx/qla_dbg.h
@@ -348,6 +348,7 @@ void __attribute__((format (printf, 4, 5)))
#define ql_dbg_tgt 0x4000 /* Target mode */
#define ql_dbg_tgt_mgt 0x2000 /* Target mode management */
#define ql_dbg_tgt_tmr 0x1000 /* Target mode task management */
+#define ql_dbg_tgt_dif 0x0800 /* Target mode dif */
extern int qla27xx_dump_mpi_ram(struct qla_hw_data *, uint32_t, uint32_t *,
uint32_t, void **);
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 8bc..d6436fc 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -2189,6 +2189,23 @@ struct qlt_plogi_ack_t {
void*fcport;
};
+enum qla_tgt_prot_op {
+ QLA_PROT_NORMAL = 0,
+ QLA_PROT_DIN_INSERT,
+ QLA_PROT_DOUT_INSERT,
+ QLA_PROT_DIN_STRIP,
+ QLA_PROT_DOUT_STRIP,
+ QLA_PROT_DIN_PASS,
+ QLA_PROT_DOUT_PASS,
+};
+
+enum qla_tgt_prot_type {
+ QLA_TGT_PROT_TYPE0,
+ QLA_TGT_PROT_TYPE1,
+ QLA_TGT_PROT_TYPE2,
+ QLA_TGT_PROT_TYPE3,
+};
+
struct ct_sns_desc {
struct ct_sns_pkt *ct_sns;
dma_addr_t ct_sns_dma;
diff --git a/drivers/scsi/qla2xxx/qla_target.c
b/drivers/scsi/qla2xxx/qla_target.c
index a4cc14b..a36258d 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -36,8 +36,6 @@
#include
#include
#include
-#include
-#include
#include "qla_def.h"
#include "qla_target.h"
@@ -140,6 +138,20 @@ static struct fc_port *qlt_create_sess(struct
scsi_qla_host *vha,
static DEFINE_MUTEX(qla_tgt_mutex);
static LIST_HEAD(qla_tgt_glist);
+static char *prot_op_str(u32 prot_op)
+{
+ switch (prot_op) {
+ case QLA_PROT_NORMAL: return "NORMAL";
+ case QLA_PROT_DIN_INSERT: return "DIN_INSERT";
+ case QLA_PROT_DOUT_INSERT: return "DOUT_INSERT";
+ case QLA_PROT_DIN_STRIP:return "DIN_STRIP";
+ case QLA_PROT_DOUT_STRIP: return "DOUT_STRIP";
+ case QLA_PROT_DIN_PASS: return "DIN_PASS";
+ case QLA_PROT_DOUT_PASS:return "DOUT_PASS";
+ default:return "UNKNOWN";
+ }
+}
+
/* This API intentionally takes dest as a parameter, rather than returning
* int value to avoid caller forgetting to issue wmb() after the store */
void qlt_do_generation_tick(struct scsi_qla_host *vha, int *dest)
@@ -1997,6 +2009,68 @@ void qlt_free_mcmd(struct qla_tgt_mgmt_cmd *mcmd)
}
EXPORT_SYMBOL(qlt_free_mcmd);
+/*
+ * ha->hardware_lock supposed to be held on entry. Might drop it, then
+ * reacquire
+ */
+void qlt_send_resp_ctio(scsi_qla_host_t *vha, struct qla_tgt_cmd *cmd,
+uint8_t scsi_status, uint8_t sense_key, uint8_t asc, uint8_t ascq)
+{
+ struct atio_from_isp *atio = &cmd->atio;
+ struct ctio7_to_24xx *ctio;
+ uint16_t temp;
+
+ ql_dbg(ql_dbg_tgt_dif, vha, 0x3066,
+ "Sending response CTIO7 (vha=%p, atio=%p, scsi_status=%02x, "
+ "sense_key=%02x, asc=%02x, ascq=%02x",
+ vha, atio, scsi_status, sense_key, asc, ascq);
+
+ ctio = (struct ctio7_to_24xx *)qla2x00_alloc_iocbs(vha, NULL);
+ if (!ctio) {
+ ql_dbg(ql_dbg_async, vha, 0x3067,
+ "qla2x00t(%ld): %s failed: unable to allocate request
packet",
+ vha->host_no, __func__);
+ goto out;
+ }
+
+ ctio->entry_type = CTIO_TYPE7;
+ ctio->entry_count = 1;
+ ctio->handle = QLA_TGT_SKIP_HANDLE;
+ ctio->nport_handle = cmd->sess->loop_id;
+ ctio->timeout = cpu_to_le16(QLA_TGT_TIMEOUT);
+ ctio->vp_index = vha->vp_idx;
+ ctio->initiator_id[0] = atio->u.isp24.fcp_hdr.s_id[2];
+ ctio->initiator_id[1] = atio->u.isp24.fcp_hdr.s_id[1];
+ ctio->initiator_id[2] = atio->u.isp24.fcp_hdr.s_id[0];
+ ctio->exchange_addr = atio->u.isp24.exchange_addr;
+ ctio->u.status1.flags = (atio->u.isp24.attr << 9) |
+ cpu_to_le16(CTIO7_FLAGS_STATUS_MODE_1 | CTIO7_FLAGS_SEND_STATUS);
+ temp = be16_to_cpu(atio->u.isp24.fcp_hdr.ox_id);
+ ctio->u.status1.ox_id = cpu_to_le16(temp);
+ ctio->u.status1.scsi_status =
+ cpu_to_le16(SS_RESPONSE_INFO_LEN_VALID | scsi_status);
+ ctio->u.status1.response_len = cpu_to_le16(18);
+ ctio->
[PATCH v2 11/14] qla2xxx: Fix inadequate lock protection for ABTS.
From: Quinn Tran
Normally, ABTS is sent to Target Core as Task MGMT command.
In the case of error, qla2xxx needs to send response, hardware_lock
is required to prevent request queue corruption.
Signed-off-by: Quinn Tran
Signed-off-by: Himanshu Madhani
---
drivers/scsi/qla2xxx/qla_target.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c
b/drivers/scsi/qla2xxx/qla_target.c
index b5fb9c55..8b372b2 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -128,6 +128,9 @@ static void qlt_send_term_imm_notif(struct scsi_qla_host
*vha,
static struct fc_port *qlt_create_sess(struct scsi_qla_host *vha,
fc_port_t *fcport, bool local);
void qlt_unreg_sess(struct fc_port *sess);
+static void qlt_24xx_handle_abts(struct scsi_qla_host *,
+ struct abts_recv_from_24xx *);
+
/*
* Global Variables
*/
@@ -403,6 +406,8 @@ static bool qlt_24xx_atio_pkt_all_vps(struct scsi_qla_host
*vha,
(struct abts_recv_from_24xx *)atio;
struct scsi_qla_host *host = qlt_find_host_by_vp_idx(vha,
entry->vp_index);
+ unsigned long flags;
+
if (unlikely(!host)) {
ql_dbg(ql_dbg_tgt, vha, 0x,
"qla_target(%d): Response pkt (ABTS_RECV_24XX) "
@@ -410,9 +415,12 @@ static bool qlt_24xx_atio_pkt_all_vps(struct scsi_qla_host
*vha,
vha->vp_idx, entry->vp_index);
break;
}
- qlt_response_pkt(host, (response_t *)atio);
+ if (!ha_locked)
+ spin_lock_irqsave(&host->hw->hardware_lock, flags);
+ qlt_24xx_handle_abts(host, (struct abts_recv_from_24xx *)atio);
+ if (!ha_locked)
+ spin_unlock_irqrestore(&host->hw->hardware_lock, flags);
break;
-
}
/* case PUREX_IOCB_TYPE: ql2xmvasynctoatio */
--
1.8.3.1
[PATCH v2 00/14] qla2xxx: Bug Fixes and updates for target.
Hi Bart, Please consider this series for inclusion in target-pending. This series contains following changes. o Fix for the deadlock because of inconsistent lock usage reported by you. o Added patch to submit non-critical MBX command via IOCB path. o Improved T10-DIF/PI handling with target stack. o Changed scsi host lookup method. o Some minor bug fixes. Changes from v1 -> v2 o Dropped patch for indentation as its already part of scsi-target-for-v4.11 branch. o Rebased series based on scsi-target-for-v4.11 branch. Please apply to target-pending. Thanks, Himanshu Anil Gurumurthy (1): qla2xxx: Export DIF stats via debugfs Himanshu Madhani (2): qla2xxx: Add DebugFS node to display Port Database qla2xxx: Update driver version to 9.00.00.00-k Joe Carnuccio (1): qla2xxx: Allow vref count to timeout on vport delete. Quinn Tran (10): qla2xxx: Fix delayed response to command for loop mode/direct connect. qla2xxx: Allow relogin to proceed if remote login did not finish qla2xxx: Use IOCB interface to submit non-critical MBX. qla2xxx: Improve T10-DIF/PI handling in driver. qla2xxx: Change scsi host lookup method qla2xxx: Fix memory leak for abts processing qla2xxx: Fix request queue corruption. qla2xxx: Fix inadequate lock protection for ABTS. qla2xxx: Add async new target notification qla2xxx: Fix sess_lock & hardware_lock lock order problem. drivers/scsi/qla2xxx/Kconfig | 1 + drivers/scsi/qla2xxx/qla_attr.c| 4 +- drivers/scsi/qla2xxx/qla_dbg.h | 1 + drivers/scsi/qla2xxx/qla_def.h | 56 ++- drivers/scsi/qla2xxx/qla_dfs.c | 107 - drivers/scsi/qla2xxx/qla_gbl.h | 13 +- drivers/scsi/qla2xxx/qla_init.c| 88 ++-- drivers/scsi/qla2xxx/qla_isr.c | 41 +- drivers/scsi/qla2xxx/qla_mbx.c | 307 -- drivers/scsi/qla2xxx/qla_mid.c | 12 +- drivers/scsi/qla2xxx/qla_os.c | 46 ++- drivers/scsi/qla2xxx/qla_target.c | 804 +++-- drivers/scsi/qla2xxx/qla_target.h | 38 +- drivers/scsi/qla2xxx/qla_version.h | 6 +- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 84 +++- 15 files changed, 1158 insertions(+), 450 deletions(-) -- 1.8.3.1
[PATCH v2 12/14] qla2xxx: Add async new target notification
From: Quinn Tran
Signed-off-by: Quinn Tran
Signed-off-by: Himanshu Madhani
---
drivers/scsi/qla2xxx/qla_target.c | 6 +++---
drivers/scsi/qla2xxx/qla_target.h | 1 +
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c
b/drivers/scsi/qla2xxx/qla_target.c
index 8b372b2..2909b7b 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -5998,13 +5998,13 @@ int qlt_add_target(struct qla_hw_data *ha, struct
scsi_qla_host *base_vha)
tgt->datasegs_per_cmd = QLA_TGT_DATASEGS_PER_CMD_24XX;
tgt->datasegs_per_cont = QLA_TGT_DATASEGS_PER_CONT_24XX;
- if (base_vha->fc_vport)
- return 0;
-
mutex_lock(&qla_tgt_mutex);
list_add_tail(&tgt->tgt_list_entry, &qla_tgt_glist);
mutex_unlock(&qla_tgt_mutex);
+ if (ha->tgt.tgt_ops && ha->tgt.tgt_ops->add_target)
+ ha->tgt.tgt_ops->add_target(base_vha);
+
return 0;
}
diff --git a/drivers/scsi/qla2xxx/qla_target.h
b/drivers/scsi/qla2xxx/qla_target.h
index 82d53f3..20e25e5 100644
--- a/drivers/scsi/qla2xxx/qla_target.h
+++ b/drivers/scsi/qla2xxx/qla_target.h
@@ -693,6 +693,7 @@ struct qla_tgt_func_tmpl {
void (*shutdown_sess)(struct fc_port *);
int (*get_dif_tags)(struct qla_tgt_cmd *cmd, uint16_t *pfw_prot_opts);
int (*chk_dif_tags)(uint32_t tag);
+ void (*add_target)(struct scsi_qla_host *);
};
int qla2x00_wait_for_hba_online(struct scsi_qla_host *);
--
1.8.3.1
[PATCH v2 08/14] qla2xxx: Change scsi host lookup method
From: Quinn Tran
For target mode, when new scsi command arrive, driver first performs
a look up of the SCSI Host. The current look up method is based on
the ALPA portion of the NPort ID. For Cisco switch, the ALPA can
not be used as the index. Instead, the new search method is based
on the full value of the Nport_ID via btree lib.
Signed-off-by: Quinn Tran
Signed-off-by: Himanshu Madhani
---
drivers/scsi/qla2xxx/Kconfig | 1 +
drivers/scsi/qla2xxx/qla_def.h| 2 +
drivers/scsi/qla2xxx/qla_gbl.h| 2 +
drivers/scsi/qla2xxx/qla_init.c | 14 +++---
drivers/scsi/qla2xxx/qla_mbx.c| 28
drivers/scsi/qla2xxx/qla_os.c | 1 +
drivers/scsi/qla2xxx/qla_target.c | 90 +--
7 files changed, 98 insertions(+), 40 deletions(-)
diff --git a/drivers/scsi/qla2xxx/Kconfig b/drivers/scsi/qla2xxx/Kconfig
index 67c0d5a..de95293 100644
--- a/drivers/scsi/qla2xxx/Kconfig
+++ b/drivers/scsi/qla2xxx/Kconfig
@@ -3,6 +3,7 @@ config SCSI_QLA_FC
depends on PCI && SCSI
depends on SCSI_FC_ATTRS
select FW_LOADER
+ select BTREE
---help---
This qla2xxx driver supports all QLogic Fibre Channel
PCI and PCIe host adapters.
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index a15614d..160d74c 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
@@ -3318,6 +3319,7 @@ struct qlt_hw_data {
spinlock_t sess_lock;
int rspq_vector_cpuid;
spinlock_t atio_lock cacheline_aligned;
+ struct btree_head32 host_map;
};
#define MAX_QFULL_CMDS_ALLOC 8192
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index 48a1190..b1e7b82 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -855,5 +855,7 @@ extern struct fc_port
*qlt_find_sess_invalidate_other(scsi_qla_host_t *,
uint64_t wwn, port_id_t port_id, uint16_t loop_id, struct fc_port **);
void qla24xx_delete_sess_fn(struct work_struct *);
void qlt_unknown_atio_work_fn(struct work_struct *);
+void qlt_update_host_map(struct scsi_qla_host *, port_id_t);
+void qlt_remove_target_resources(struct qla_hw_data *);
#endif /* _QLA_GBL_H */
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 5aca3df..ea6ddcf 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -3345,8 +3345,8 @@ void qla2x00_fcport_event_handler(scsi_qla_host_t *vha,
struct event_arg *ea)
uint8_t domain;
charconnect_type[22];
struct qla_hw_data *ha = vha->hw;
- unsigned long flags;
scsi_qla_host_t *base_vha = pci_get_drvdata(ha->pdev);
+ port_id_t id;
/* Get host addresses. */
rval = qla2x00_get_adapter_id(vha,
@@ -3424,13 +3424,11 @@ void qla2x00_fcport_event_handler(scsi_qla_host_t *vha,
struct event_arg *ea)
/* Save Host port and loop ID. */
/* byte order - Big Endian */
- vha->d_id.b.domain = domain;
- vha->d_id.b.area = area;
- vha->d_id.b.al_pa = al_pa;
-
- spin_lock_irqsave(&ha->vport_slock, flags);
- qlt_update_vp_map(vha, SET_AL_PA);
- spin_unlock_irqrestore(&ha->vport_slock, flags);
+ id.b.domain = domain;
+ id.b.area = area;
+ id.b.al_pa = al_pa;
+ id.b.rsvd_1 = 0;
+ qlt_update_host_map(vha, id);
if (!vha->flags.init_done)
ql_log(ql_log_info, vha, 0x2010,
diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index 4225256..f352889 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -3624,6 +3624,7 @@ struct tsk_mgmt_cmd {
scsi_qla_host_t *vp = NULL;
unsigned long flags;
int found;
+ port_id_t id;
ql_dbg(ql_dbg_mbx + ql_dbg_verbose, vha, 0x10b6,
"Entered %s.\n", __func__);
@@ -3631,6 +3632,11 @@ struct tsk_mgmt_cmd {
if (rptid_entry->entry_status != 0)
return;
+ id.b.domain = rptid_entry->port_id[2];
+ id.b.area = rptid_entry->port_id[1];
+ id.b.al_pa = rptid_entry->port_id[0];
+ id.b.rsvd_1 = 0;
+
if (rptid_entry->format == 0) {
/* loop */
ql_dbg(ql_dbg_async, vha, 0x10b7,
@@ -3642,13 +3648,7 @@ struct tsk_mgmt_cmd {
rptid_entry->port_id[2], rptid_entry->port_id[1],
rptid_entry->port_id[0]);
- vha->d_id.b.domain = rptid_entry->port_id[2];
- vha->d_id.b.area = rptid_entry->port_id[1];
- vha->d_id.b.al_pa = rptid_entry->port_id[0];
-
- spin_lock_irqsave(&ha->vport_slock, flags);
- qlt_update_vp_map(vha, SET_AL_PA);
- spin_unlock_irqrestore(&ha->vport_slock, flags);
+ qlt_update_
[PATCH v2 09/14] qla2xxx: Fix memory leak for abts processing
From: Quinn Tran Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_target.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index 359a79d..b61cbb8 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -6753,6 +6753,8 @@ static void qlt_disable_vha(struct scsi_qla_host *vha) spin_lock_irqsave(&ha->hardware_lock, flags); qlt_response_pkt_all_vps(vha, (response_t *)&op->atio); spin_unlock_irqrestore(&ha->hardware_lock, flags); + + kfree(op); } void -- 1.8.3.1
[PATCH v2 13/14] qla2xxx: Fix sess_lock & hardware_lock lock order problem.
From: Quinn Tran
The main lock that needs to be held for CMD or TMR submission
to upper layer is the sess_lock. The sess_lock is used to
serialize cmd submission and session deletion. The addition
of hardware_lock being held is not necessary. This patch removes
hardware_lock dependency from CMD/TMR submission.
Use hardware_lock only for error response in this case.
Path1
CPU0CPU1
lock(&(&ha->tgt.sess_lock)->rlock);
lock(&(&ha->hardware_lock)->rlock);
lock(&(&ha->tgt.sess_lock)->rlock);
lock(&(&ha->hardware_lock)->rlock);
Path2/deadlock
*** DEADLOCK ***
Call Trace:
dump_stack+0x85/0xc2
print_circular_bug+0x1e3/0x250
__lock_acquire+0x1425/0x1620
lock_acquire+0xbf/0x210
_raw_spin_lock_irqsave+0x53/0x70
qlt_sess_work_fn+0x21d/0x480 [qla2xxx]
process_one_work+0x1f4/0x6e0
Cc:
Cc: Bart Van Assche
Reported-by: Bart Van Assche
Signed-off-by: Quinn Tran
Signed-off-by: Himanshu Madhani
---
drivers/scsi/qla2xxx/qla_target.c | 41 +--
1 file changed, 18 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c
b/drivers/scsi/qla2xxx/qla_target.c
index 2909b7b..fb7156c 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -5818,30 +5818,23 @@ static void qlt_abort_work(struct qla_tgt *tgt,
}
}
- spin_lock_irqsave(&ha->hardware_lock, flags);
-
- if (tgt->tgt_stop)
- goto out_term;
-
rc = __qlt_24xx_handle_abts(vha, &prm->abts, sess);
+ ha->tgt.tgt_ops->put_sess(sess);
+ spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
+
if (rc != 0)
goto out_term;
- spin_unlock_irqrestore(&ha->hardware_lock, flags);
- if (sess)
- ha->tgt.tgt_ops->put_sess(sess);
- spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
return;
out_term2:
- spin_lock_irqsave(&ha->hardware_lock, flags);
+ if (sess)
+ ha->tgt.tgt_ops->put_sess(sess);
+ spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
out_term:
+ spin_lock_irqsave(&ha->hardware_lock, flags);
qlt_24xx_send_abts_resp(vha, &prm->abts, FCP_TMF_REJECTED, false);
spin_unlock_irqrestore(&ha->hardware_lock, flags);
-
- if (sess)
- ha->tgt.tgt_ops->put_sess(sess);
- spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
}
static void qlt_tmr_work(struct qla_tgt *tgt,
@@ -5861,7 +5854,7 @@ static void qlt_tmr_work(struct qla_tgt *tgt,
spin_lock_irqsave(&ha->tgt.sess_lock, flags);
if (tgt->tgt_stop)
- goto out_term;
+ goto out_term2;
s_id = prm->tm_iocb2.u.isp24.fcp_hdr.s_id;
sess = ha->tgt.tgt_ops->find_sess_by_s_id(vha, s_id);
@@ -5873,11 +5866,11 @@ static void qlt_tmr_work(struct qla_tgt *tgt,
spin_lock_irqsave(&ha->tgt.sess_lock, flags);
if (!sess)
- goto out_term;
+ goto out_term2;
} else {
if (sess->deleted) {
sess = NULL;
- goto out_term;
+ goto out_term2;
}
if (!kref_get_unless_zero(&sess->sess_kref)) {
@@ -5885,7 +5878,7 @@ static void qlt_tmr_work(struct qla_tgt *tgt,
"%s: kref_get fail %8phC\n",
__func__, sess->port_name);
sess = NULL;
- goto out_term;
+ goto out_term2;
}
}
@@ -5895,17 +5888,19 @@ static void qlt_tmr_work(struct qla_tgt *tgt,
unpacked_lun = scsilun_to_int((struct scsi_lun *)&lun);
rc = qlt_issue_task_mgmt(sess, unpacked_lun, fn, iocb, 0);
- if (rc != 0)
- goto out_term;
-
ha->tgt.tgt_ops->put_sess(sess);
spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
+
+ if (rc != 0)
+ goto out_term;
return;
+out_term2:
+ if (sess)
+ ha->tgt.tgt_ops->put_sess(sess);
+ spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
out_term:
qlt_send_term_exchange(vha, NULL, &prm->tm_iocb2, 1, 0);
- ha->tgt.tgt_ops->put_sess(sess);
- spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
}
static void qlt_sess_work_fn(struct work_struct *work)
--
1.8.3.1
Re: [REGRESSION v4.10-rc1] blkdev_issue_zeroout() returns -EREMOTEIO on the first call for SCSI device that doesn't support WRITE SAME
> "Jens" == Jens Axboe writes: >> I think we should fix sd.c to only send WRITE SAME if either of the >> variants are explicitly listed as supported through REPORT SUPPORTED >> OPERATION CODES, or maybe through a whitelist if there are important >> enough devices. Jens> Yep I hate it too. But the reason it's assumed on is that there is essentially no heuristic that works. Just like we assume that READ always works. Out of the ~200 devices I have access to in the lab: - 100% of the SAS/FC disk drives and SSDs support WRITE SAME - Only 2 out of about 50 different drive models support RSOC - About half of the arrays support WRITE SAME(10/16) - None of the arrays I have support RSOC So even if we were to entertain using RSOC for "enterprise" transport classes (which I concur would be nice for other reasons), it wouldn't solve the WRITE SAME problem... -- Martin K. Petersen Oracle Linux Engineering
[PATCH v1 0/8] scsi: ufs: enhancements, bug fixes and debug support
This patch series adds following things: - Gear scaling during clock scaling to save additional power - Make clock scaling independent clock gating so that we can set lower clock gating timeout than clock scaling window - one bug fix to clock scaling - additional debug support Gilad Broner (3): scsi: ufs: skip request abort task when previous aborts failed scsi: ufs: reduce printout for aborted requests scsi: ufs: add host state prints in failure cases Subhash Jadavani (3): scsi: ufs: add load based scaling of UFS gear scsi: ufs: don't suspend clock scaling during clock gating scsi: ufs: kick start clock scaling only after device detection Venkat Gopalakrishnan (2): scsi: ufs-qcom: dump additional testbus registers scsi: ufs: dump hw regs on link failures drivers/scsi/ufs/ufs-qcom.c | 48 ++- drivers/scsi/ufs/ufs-qcom.h | 1 + drivers/scsi/ufs/ufshcd.c | 863 drivers/scsi/ufs/ufshcd.h | 44 ++- 4 files changed, 726 insertions(+), 230 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v1 1/8] scsi: ufs: skip request abort task when previous aborts failed
From: Gilad Broner
On certain error conditions request abort task itself might fail
when aborting a request. In such case, subsequent request aborts
should skip issuing the abort task as it is expected to fail as well,
and device reset handler will be called next.
Signed-off-by: Gilad Broner
Signed-off-by: Subhash Jadavani
---
drivers/scsi/ufs/ufshcd.c | 20
drivers/scsi/ufs/ufshcd.h | 3 +++
2 files changed, 23 insertions(+)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index a70bf06..61fea17 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1877,6 +1877,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *cmd)
lrbp->task_tag = tag;
lrbp->lun = ufshcd_scsi_to_upiu_lun(cmd->device->lun);
lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba) ? true : false;
+ lrbp->req_abort_skip = false;
ufshcd_comp_scsi_upiu(hba, lrbp);
@@ -4979,6 +4980,17 @@ static int ufshcd_eh_device_reset_handler(struct
scsi_cmnd *cmd)
return err;
}
+static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long
bitmap)
+{
+ struct ufshcd_lrb *lrbp;
+ int tag;
+
+ for_each_set_bit(tag, &bitmap, hba->nutrs) {
+ lrbp = &hba->lrb[tag];
+ lrbp->req_abort_skip = true;
+ }
+}
+
/**
* ufshcd_abort - abort a specific command
* @cmd: SCSI command pointer
@@ -5047,6 +5059,13 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
ufshcd_print_pwr_info(hba);
ufshcd_print_trs(hba, 1 << tag, true);
+
+ /* Skip task abort in case previous aborts failed and report failure */
+ if (lrbp->req_abort_skip) {
+ err = -EIO;
+ goto out;
+ }
+
for (poll_cnt = 100; poll_cnt; poll_cnt--) {
err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
UFS_QUERY_TASK, &resp);
@@ -5120,6 +5139,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
err = SUCCESS;
} else {
dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
+ ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs);
err = FAILED;
}
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 7ffcde2..292fc14 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -165,6 +165,7 @@ struct ufs_pm_lvl_states {
* @lun: LUN of the command
* @intr_cmd: Interrupt command (doesn't participate in interrupt aggregation)
* @issue_time_stamp: time stamp for debug purposes
+ * @req_abort_skip: skip request abort task flag
*/
struct ufshcd_lrb {
struct utp_transfer_req_desc *utr_descriptor_ptr;
@@ -187,6 +188,8 @@ struct ufshcd_lrb {
u8 lun; /* UPIU LUN id field is only 8-bit wide */
bool intr_cmd;
ktime_t issue_time_stamp;
+
+ bool req_abort_skip;
};
/**
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v1 2/8] scsi: ufs: reduce printout for aborted requests
From: Gilad Broner
Details printed for each request that is aborted can overload the
target as there can be several requests that are aborted at once.
This change will print full request details only for the first
aborted request since the last link reset, and minimal details
for other subsequent requests.
Signed-off-by: Gilad Broner
Signed-off-by: Subhash Jadavani
---
drivers/scsi/ufs/ufshcd.c | 46 --
drivers/scsi/ufs/ufshcd.h | 3 +++
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 61fea17..64d619a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -396,6 +396,7 @@ static void ufshcd_print_host_regs(struct ufs_hba *hba)
void ufshcd_print_trs(struct ufs_hba *hba, unsigned long bitmap, bool pr_prdt)
{
struct ufshcd_lrb *lrbp;
+ int prdt_length;
int tag;
for_each_set_bit(tag, &bitmap, hba->nutrs) {
@@ -417,18 +418,17 @@ void ufshcd_print_trs(struct ufs_hba *hba, unsigned long
bitmap, bool pr_prdt)
(u64)lrbp->ucd_rsp_dma_addr);
ufshcd_hex_dump("UPIU RSP: ", lrbp->ucd_rsp_ptr,
sizeof(struct utp_upiu_rsp));
- if (pr_prdt) {
- int prdt_length = le16_to_cpu(
- lrbp->utr_descriptor_ptr->prd_table_length);
- dev_err(hba->dev,
- "UPIU[%d] - PRDT - %d entries phys@0x%llx\n",
- tag, prdt_length,
- (u64)lrbp->ucd_prdt_dma_addr);
+ prdt_length = le16_to_cpu(
+ lrbp->utr_descriptor_ptr->prd_table_length);
+ dev_err(hba->dev,
+ "UPIU[%d] - PRDT - %d entries phys@0x%llx\n",
+ tag, prdt_length,
+ (u64)lrbp->ucd_prdt_dma_addr);
+
+ if (pr_prdt)
ufshcd_hex_dump("UPIU PRDT: ", lrbp->ucd_prdt_ptr,
- sizeof(struct ufshcd_sg_entry) *
- prdt_length);
- }
+ sizeof(struct ufshcd_sg_entry) * prdt_length);
}
}
@@ -1848,6 +1848,8 @@ static int ufshcd_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *cmd)
}
spin_unlock_irqrestore(hba->host->host_lock, flags);
+ hba->req_abort_count = 0;
+
/* acquire the tag to make sure device cmds don't use it */
if (test_and_set_bit_lock(tag, &hba->lrb_in_use)) {
/*
@@ -4970,7 +4972,9 @@ static int ufshcd_eh_device_reset_handler(struct
scsi_cmnd *cmd)
spin_lock_irqsave(host->host_lock, flags);
ufshcd_transfer_req_compl(hba);
spin_unlock_irqrestore(host->host_lock, flags);
+
out:
+ hba->req_abort_count = 0;
if (!err) {
err = SUCCESS;
} else {
@@ -5054,11 +5058,23 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
/* Print Transfer Request of aborted task */
dev_err(hba->dev, "%s: Device abort task at tag %d\n", __func__, tag);
- scsi_print_command(hba->lrb[tag].cmd);
- ufshcd_print_host_regs(hba);
- ufshcd_print_pwr_info(hba);
- ufshcd_print_trs(hba, 1 << tag, true);
+ /*
+* Print detailed info about aborted request.
+* As more than one request might get aborted at the same time,
+* print full information only for the first aborted request in order
+* to reduce repeated printouts. For other aborted requests only print
+* basic details.
+*/
+ scsi_print_command(hba->lrb[tag].cmd);
+ if (!hba->req_abort_count) {
+ ufshcd_print_host_regs(hba);
+ ufshcd_print_pwr_info(hba);
+ ufshcd_print_trs(hba, 1 << tag, true);
+ } else {
+ ufshcd_print_trs(hba, 1 << tag, false);
+ }
+ hba->req_abort_count++;
/* Skip task abort in case previous aborts failed and report failure */
if (lrbp->req_abort_skip) {
@@ -5707,6 +5723,8 @@ static void ufshcd_clear_dbg_ufs_stats(struct ufs_hba
*hba)
memset(&hba->ufs_stats.nl_err, 0, err_reg_hist_size);
memset(&hba->ufs_stats.tl_err, 0, err_reg_hist_size);
memset(&hba->ufs_stats.dme_err, 0, err_reg_hist_size);
+
+ hba->req_abort_count = 0;
}
/**
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 292fc14..b7ce129 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -594,6 +594,9 @@ struct ufs_hba {
bool wlun_dev_clr_ua;
+ /* Number of requests aborts */
+ int req_abort_count;
+
/* Number of lanes available (1 or 2) for Rx/Tx */
u32 lanes_per_direction;
struct ufs_pa_layer_attr pwr_info;
--
The Qualcomm Innovation Center, Inc.
[PATCH v1 3/8] scsi: ufs: add load based scaling of UFS gear
UFS driver's load based clock scaling feature scales down the ufs related
clocks in order to allow low power modes of chipsets. UniPro 1.6 supports
maximum gear up to HS-G3 (High Speed Gear3) and some of the chipsets
low power modes may not be allowed in HS-G3 hence this change adds support
to scale gear between HS-G3 and HS-G1 based on same existing load based
clock scaling logic.
Signed-off-by: Subhash Jadavani
---
drivers/scsi/ufs/ufshcd.c | 594 --
drivers/scsi/ufs/ufshcd.h | 9 +
2 files changed, 428 insertions(+), 175 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 64d619a..e75e50d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -867,6 +867,396 @@ static bool ufshcd_is_unipro_pa_params_tuning_req(struct
ufs_hba *hba)
return false;
}
+static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up)
+{
+ int ret = 0;
+ struct ufs_clk_info *clki;
+ struct list_head *head = &hba->clk_list_head;
+ ktime_t start = ktime_get();
+ bool clk_state_changed = false;
+
+ if (!head || list_empty(head))
+ goto out;
+
+ ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE);
+ if (ret)
+ return ret;
+
+ list_for_each_entry(clki, head, list) {
+ if (!IS_ERR_OR_NULL(clki->clk)) {
+ if (scale_up && clki->max_freq) {
+ if (clki->curr_freq == clki->max_freq)
+ continue;
+
+ clk_state_changed = true;
+ ret = clk_set_rate(clki->clk, clki->max_freq);
+ if (ret) {
+ dev_err(hba->dev, "%s: %s clk set
rate(%dHz) failed, %d\n",
+ __func__, clki->name,
+ clki->max_freq, ret);
+ break;
+ }
+ trace_ufshcd_clk_scaling(dev_name(hba->dev),
+ "scaled up", clki->name,
+ clki->curr_freq,
+ clki->max_freq);
+
+ clki->curr_freq = clki->max_freq;
+
+ } else if (!scale_up && clki->min_freq) {
+ if (clki->curr_freq == clki->min_freq)
+ continue;
+
+ clk_state_changed = true;
+ ret = clk_set_rate(clki->clk, clki->min_freq);
+ if (ret) {
+ dev_err(hba->dev, "%s: %s clk set
rate(%dHz) failed, %d\n",
+ __func__, clki->name,
+ clki->min_freq, ret);
+ break;
+ }
+ trace_ufshcd_clk_scaling(dev_name(hba->dev),
+ "scaled down", clki->name,
+ clki->curr_freq,
+ clki->min_freq);
+ clki->curr_freq = clki->min_freq;
+ }
+ }
+ dev_dbg(hba->dev, "%s: clk: %s, rate: %lu\n", __func__,
+ clki->name, clk_get_rate(clki->clk));
+ }
+
+ ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE);
+
+out:
+ if (clk_state_changed)
+ trace_ufshcd_profile_clk_scaling(dev_name(hba->dev),
+ (scale_up ? "up" : "down"),
+ ktime_to_us(ktime_sub(ktime_get(), start)), ret);
+ return ret;
+}
+
+/**
+ * ufshcd_is_devfreq_scaling_required - check if scaling is required or not
+ * @hba: per adapter instance
+ * @scale_up: True if scaling up and false if scaling down
+ *
+ * Returns true if scaling is required, false otherwise.
+ */
+static bool ufshcd_is_devfreq_scaling_required(struct ufs_hba *hba,
+ bool scale_up)
+{
+ struct ufs_clk_info *clki;
+ struct list_head *head = &hba->clk_list_head;
+
+ if (!head || list_empty(head))
+ return false;
+
+ list_for_each_entry(clki, head, list) {
+ if (!IS_ERR_OR_NULL(clki->clk)) {
+ if (scale_up && clki->max_freq) {
+ if (clki->curr_freq == clki->max_freq)
+ continue;
+ return true;
+ } else if (!scale_up && clki->min_freq) {
+ if (clki->curr_fre
[PATCH v1 5/8] scsi: ufs: don't suspend clock scaling during clock gating
Currently we are suspending clock scaling during clock gating which doesn't
allow us to have clock gating timeout lower than clock scaling polling
window. If clock gating timeout is smaller than the clock scaling polling
window then we will mostly suspend the clock scaling before clock scaling
polling window expires and we might get stuck in same state (scaled down
or scaled up) for quite a long time. And for this reason, we have clock
gating timeout (150ms) greater than clock scaling polling window (100ms).
We would like to have aggressive clock gating timeout even lower than the
clock scaling polling window hence this change is decoupling the clock
scaling suspend/resume from clock gate/ungate. We will not suspend the
clock scaling as part of clock gating instead clock scaling context will
schedule scaling suspend work if there are no more pending transfer
requests.
Signed-off-by: Subhash Jadavani
---
drivers/scsi/ufs/ufshcd.c | 185 --
drivers/scsi/ufs/ufshcd.h | 31 +++-
2 files changed, 171 insertions(+), 45 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 056e912..4f6ba24 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -248,6 +248,7 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool
on,
static int ufshcd_host_reset_and_restore(struct ufs_hba *hba);
static void ufshcd_resume_clkscaling(struct ufs_hba *hba);
static void ufshcd_suspend_clkscaling(struct ufs_hba *hba);
+static void __ufshcd_suspend_clkscaling(struct ufs_hba *hba);
static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up);
static irqreturn_t ufshcd_intr(int irq, void *__hba);
static int ufshcd_config_pwr_mode(struct ufs_hba *hba,
@@ -1135,6 +1136,9 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba, bool
scale_up)
{
int ret = 0;
+ /* let's not get into low power until clock scaling is completed */
+ ufshcd_hold(hba, false);
+
ret = ufshcd_clock_scaling_prepare(hba);
if (ret)
return ret;
@@ -1166,16 +1170,51 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba,
bool scale_up)
out:
ufshcd_clock_scaling_unprepare(hba);
+ ufshcd_release(hba);
return ret;
}
+static void ufshcd_clk_scaling_suspend_work(struct work_struct *work)
+{
+ struct ufs_hba *hba = container_of(work, struct ufs_hba,
+ clk_scaling.suspend_work);
+ unsigned long irq_flags;
+
+ spin_lock_irqsave(hba->host->host_lock, irq_flags);
+ if (hba->clk_scaling.active_reqs || hba->clk_scaling.is_suspended) {
+ spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+ return;
+ }
+ hba->clk_scaling.is_suspended = true;
+ spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+
+ __ufshcd_suspend_clkscaling(hba);
+}
+
+static void ufshcd_clk_scaling_resume_work(struct work_struct *work)
+{
+ struct ufs_hba *hba = container_of(work, struct ufs_hba,
+ clk_scaling.resume_work);
+ unsigned long irq_flags;
+
+ spin_lock_irqsave(hba->host->host_lock, irq_flags);
+ if (!hba->clk_scaling.is_suspended) {
+ spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+ return;
+ }
+ hba->clk_scaling.is_suspended = false;
+ spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+
+ devfreq_resume_device(hba->devfreq);
+}
+
static int ufshcd_devfreq_target(struct device *dev,
unsigned long *freq, u32 flags)
{
int ret = 0;
struct ufs_hba *hba = dev_get_drvdata(dev);
ktime_t start;
- bool scale_up, release_clk_hold = false;
+ bool scale_up, sched_clk_scaling_suspend_work = false;
unsigned long irq_flags;
if (!ufshcd_is_clkscaling_supported(hba))
@@ -1186,50 +1225,35 @@ static int ufshcd_devfreq_target(struct device *dev,
return -EINVAL;
}
- scale_up = (*freq == UINT_MAX) ? true : false;
- if (!ufshcd_is_devfreq_scaling_required(hba, scale_up))
- return 0; /* no state change required */
-
spin_lock_irqsave(hba->host->host_lock, irq_flags);
if (ufshcd_eh_in_progress(hba)) {
spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
return 0;
}
- if (ufshcd_is_clkgating_allowed(hba)) {
- if (cancel_delayed_work(&hba->clk_gating.gate_work) ||
- (hba->clk_gating.state == CLKS_ON)) {
- /* hold the vote until the scaling work is completed */
- hba->clk_gating.active_reqs++;
- release_clk_hold = true;
- if (hba->clk_gating.state != CLKS_ON) {
- hba->clk_gating.state = CLKS_ON;
-
[PATCH v1 4/8] scsi: ufs: add host state prints in failure cases
From: Gilad Broner
Whenever some UFS failure occurs the driver prints the UFS
registers in order to help with analysis of the failure.
However this may not be sufficient in some cases, so having
the host controller state as it is represented and managed in
the driver will contribute to analysis efforts.
Added prints of various fields in the hba struct which may be
of interest.
Signed-off-by: Gilad Broner
Signed-off-by: Subhash Jadavani
---
drivers/scsi/ufs/ufshcd.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index e75e50d..056e912 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -453,6 +453,28 @@ static void ufshcd_print_tmrs(struct ufs_hba *hba,
unsigned long bitmap)
}
}
+static void ufshcd_print_host_state(struct ufs_hba *hba)
+{
+ dev_err(hba->dev, "UFS Host state=%d\n", hba->ufshcd_state);
+ dev_err(hba->dev, "lrb in use=0x%lx, outstanding reqs=0x%lx
tasks=0x%lx\n",
+ hba->lrb_in_use, hba->outstanding_tasks, hba->outstanding_reqs);
+ dev_err(hba->dev, "saved_err=0x%x, saved_uic_err=0x%x\n",
+ hba->saved_err, hba->saved_uic_err);
+ dev_err(hba->dev, "Device power mode=%d, UIC link state=%d\n",
+ hba->curr_dev_pwr_mode, hba->uic_link_state);
+ dev_err(hba->dev, "PM in progress=%d, sys. suspended=%d\n",
+ hba->pm_op_in_progress, hba->is_sys_suspended);
+ dev_err(hba->dev, "Auto BKOPS=%d, Host self-block=%d\n",
+ hba->auto_bkops_enabled, hba->host->host_self_blocked);
+ dev_err(hba->dev, "Clk gate=%d\n", hba->clk_gating.state);
+ dev_err(hba->dev, "error handling flags=0x%x, req. abort count=%d\n",
+ hba->eh_flags, hba->req_abort_count);
+ dev_err(hba->dev, "Host capabilities=0x%x, caps=0x%x\n",
+ hba->capabilities, hba->caps);
+ dev_err(hba->dev, "quirks=0x%x, dev. quirks=0x%x\n", hba->quirks,
+ hba->dev_quirks);
+}
+
/**
* ufshcd_print_pwr_info - print power params as saved in hba
* power info
@@ -4426,6 +4448,7 @@ static int ufshcd_task_req_compl(struct ufs_hba *hba, u32
index, u8 *resp)
"OCS error from controller = %x for tag %d\n",
ocs, lrbp->task_tag);
ufshcd_print_host_regs(hba);
+ ufshcd_print_host_state(hba);
break;
} /* end of switch */
@@ -5477,6 +5500,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
scsi_print_command(hba->lrb[tag].cmd);
if (!hba->req_abort_count) {
ufshcd_print_host_regs(hba);
+ ufshcd_print_host_state(hba);
ufshcd_print_pwr_info(hba);
ufshcd_print_trs(hba, 1 << tag, true);
} else {
@@ -7738,6 +7762,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem
*mmio_base, unsigned int irq)
if (err) {
dev_err(hba->dev, "Host controller enable failed\n");
ufshcd_print_host_regs(hba);
+ ufshcd_print_host_state(hba);
goto out_remove_scsi_host;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v1 8/8] scsi: ufs: dump hw regs on link failures
From: Venkat Gopalakrishnan
Dump host state, power info and host/vendor specific registers
on link failures. This provides useful info to debug the failures.
Signed-off-by: Venkat Gopalakrishnan
Signed-off-by: Subhash Jadavani
---
drivers/scsi/ufs/ufshcd.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index a99a673..8b721f4 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3554,6 +3554,12 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba,
struct uic_command *cmd)
ret = (status != PWR_OK) ? status : -1;
}
out:
+ if (ret) {
+ ufshcd_print_host_state(hba);
+ ufshcd_print_pwr_info(hba);
+ ufshcd_print_host_regs(hba);
+ }
+
spin_lock_irqsave(hba->host->host_lock, flags);
hba->active_uic_cmd = NULL;
hba->uic_async_done = NULL;
@@ -4146,8 +4152,12 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
ret = ufshcd_make_hba_operational(hba);
out:
- if (ret)
+ if (ret) {
dev_err(hba->dev, "link startup failed %d\n", ret);
+ ufshcd_print_host_state(hba);
+ ufshcd_print_pwr_info(hba);
+ ufshcd_print_host_regs(hba);
+ }
return ret;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v1 6/8] scsi: ufs: kick start clock scaling only after device detection
UFS clock scaling might start kicking in even before the device
is running at the fastest interface speed which is undesirable.
This change moves the clock scaling kick start only after the
device is detected and running at the fastest interface speed.
Signed-off-by: Subhash Jadavani
---
drivers/scsi/ufs/ufshcd.c | 45 +++--
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 4f6ba24..a99a673 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6330,19 +6330,31 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
if (ufshcd_scsi_add_wlus(hba))
goto out;
+ /* Initialize devfreq after UFS device is detected */
+ if (ufshcd_is_clkscaling_supported(hba)) {
+ memcpy(&hba->clk_scaling.saved_pwr_info.info,
+ &hba->pwr_info,
+ sizeof(struct ufs_pa_layer_attr));
+ hba->clk_scaling.saved_pwr_info.is_valid = true;
+ if (!hba->devfreq) {
+ hba->devfreq = devm_devfreq_add_device(hba->dev,
+ &ufs_devfreq_profile,
+ "simple_ondemand",
+ NULL);
+ if (IS_ERR(hba->devfreq)) {
+ ret = PTR_ERR(hba->devfreq);
+ dev_err(hba->dev, "Unable to register
with devfreq %d\n",
+ ret);
+ goto out;
+ }
+ }
+ hba->clk_scaling.is_allowed = true;
+ }
+
scsi_scan_host(hba->host);
pm_runtime_put_sync(hba->dev);
}
- /* Resume devfreq after UFS device is detected */
- if (ufshcd_is_clkscaling_supported(hba)) {
- memcpy(&hba->clk_scaling.saved_pwr_info.info, &hba->pwr_info,
- sizeof(struct ufs_pa_layer_attr));
- hba->clk_scaling.saved_pwr_info.is_valid = true;
- ufshcd_resume_clkscaling(hba);
- hba->clk_scaling.is_allowed = true;
- }
-
if (!hba->is_init_prefetch)
hba->is_init_prefetch = true;
@@ -6865,7 +6877,8 @@ static void ufshcd_hba_exit(struct ufs_hba *hba)
ufshcd_setup_vreg(hba, false);
ufshcd_suspend_clkscaling(hba);
if (ufshcd_is_clkscaling_supported(hba)) {
- ufshcd_suspend_clkscaling(hba);
+ if (hba->devfreq)
+ ufshcd_suspend_clkscaling(hba);
destroy_workqueue(hba->clk_scaling.workq);
}
ufshcd_setup_clocks(hba, false);
@@ -7859,16 +7872,6 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem
*mmio_base, unsigned int irq)
if (ufshcd_is_clkscaling_supported(hba)) {
char wq_name[sizeof("ufs_clkscaling_00")];
- hba->devfreq = devm_devfreq_add_device(dev,
&ufs_devfreq_profile,
- "simple_ondemand", NULL);
- if (IS_ERR(hba->devfreq)) {
- dev_err(hba->dev, "Unable to register with devfreq
%ld\n",
- PTR_ERR(hba->devfreq));
- err = PTR_ERR(hba->devfreq);
- goto out_remove_scsi_host;
- }
- hba->clk_scaling.is_suspended = false;
-
INIT_WORK(&hba->clk_scaling.suspend_work,
ufshcd_clk_scaling_suspend_work);
INIT_WORK(&hba->clk_scaling.resume_work,
@@ -7878,8 +7881,6 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem
*mmio_base, unsigned int irq)
host->host_no);
hba->clk_scaling.workq = create_singlethread_workqueue(wq_name);
- /* Suspend devfreq until the UFS device is detected */
- ufshcd_suspend_clkscaling(hba);
ufshcd_clkscaling_init_sysfs(hba);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v1 7/8] scsi: ufs-qcom: dump additional testbus registers
From: Venkat Gopalakrishnan
Change testbus default config, dump additional testbus registers along
with other debug vendor specific registers. These additional info are
useful in debugging link related failures.
Signed-off-by: Venkat Gopalakrishnan
Signed-off-by: Subhash Jadavani
---
drivers/scsi/ufs/ufs-qcom.c | 48 +++--
drivers/scsi/ufs/ufs-qcom.h | 1 +
2 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 5ff8a6b..ce5d023 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1497,17 +1497,21 @@ static void ufs_qcom_print_hw_debug_reg_all(struct
ufs_hba *hba,
static void ufs_qcom_enable_test_bus(struct ufs_qcom_host *host)
{
- if (host->dbg_print_en & UFS_QCOM_DBG_PRINT_TEST_BUS_EN)
+ if (host->dbg_print_en & UFS_QCOM_DBG_PRINT_TEST_BUS_EN) {
+ ufshcd_rmwl(host->hba, UFS_REG_TEST_BUS_EN,
+ UFS_REG_TEST_BUS_EN, REG_UFS_CFG1);
ufshcd_rmwl(host->hba, TEST_BUS_EN, TEST_BUS_EN, REG_UFS_CFG1);
- else
+ } else {
+ ufshcd_rmwl(host->hba, UFS_REG_TEST_BUS_EN, 0, REG_UFS_CFG1);
ufshcd_rmwl(host->hba, TEST_BUS_EN, 0, REG_UFS_CFG1);
+ }
}
static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host)
{
/* provide a legal default configuration */
- host->testbus.select_major = TSTBUS_UAWM;
- host->testbus.select_minor = 1;
+ host->testbus.select_major = TSTBUS_UNIPRO;
+ host->testbus.select_minor = 37;
}
static bool ufs_qcom_testbus_cfg_is_ok(struct ufs_qcom_host *host)
@@ -1524,7 +1528,7 @@ static bool ufs_qcom_testbus_cfg_is_ok(struct
ufs_qcom_host *host)
* mappings of select_minor, since there is no harm in
* configuring a non-existent select_minor
*/
- if (host->testbus.select_minor > 0x1F) {
+ if (host->testbus.select_minor > 0xFF) {
dev_err(host->hba->dev,
"%s: 0x%05X is not a legal testbus option\n",
__func__, host->testbus.select_minor);
@@ -1593,7 +1597,8 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host *host)
break;
case TSTBUS_UNIPRO:
reg = UFS_UNIPRO_CFG;
- offset = 1;
+ offset = 20;
+ mask = 0xFFF;
break;
/*
* No need for a default case, since
@@ -1612,6 +1617,11 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host *host)
(u32)host->testbus.select_minor << offset,
reg);
ufs_qcom_enable_test_bus(host);
+ /*
+* Make sure the test bus configuration is
+* committed before returning.
+*/
+ mb();
ufshcd_release(host->hba);
pm_runtime_put_sync(host->hba->dev);
@@ -1623,13 +1633,39 @@ static void ufs_qcom_testbus_read(struct ufs_hba *hba)
ufs_qcom_dump_regs(hba, UFS_TEST_BUS, 1, "UFS_TEST_BUS ");
}
+static void ufs_qcom_print_unipro_testbus(struct ufs_hba *hba)
+{
+ struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+ u32 *testbus = NULL;
+ int i, nminor = 256, testbus_len = nminor * sizeof(u32);
+
+ testbus = kmalloc(testbus_len, GFP_KERNEL);
+ if (!testbus)
+ return;
+
+ host->testbus.select_major = TSTBUS_UNIPRO;
+ for (i = 0; i < nminor; i++) {
+ host->testbus.select_minor = i;
+ ufs_qcom_testbus_config(host);
+ testbus[i] = ufshcd_readl(hba, UFS_TEST_BUS);
+ }
+ print_hex_dump(KERN_ERR, "UNIPRO_TEST_BUS ", DUMP_PREFIX_OFFSET,
+ 16, 4, testbus, testbus_len, false);
+ kfree(testbus);
+}
+
static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)
{
ufs_qcom_dump_regs(hba, REG_UFS_SYS1CLK_1US, 16,
"HCI Vendor Specific Registers ");
+ /* sleep a bit intermittently as we are dumping too much data */
ufs_qcom_print_hw_debug_reg_all(hba, NULL, ufs_qcom_dump_regs_wrapper);
+ usleep_range(1000, 1100);
ufs_qcom_testbus_read(hba);
+ usleep_range(1000, 1100);
+ ufs_qcom_print_unipro_testbus(hba);
+ usleep_range(1000, 1100);
}
/**
diff --git a/drivers/scsi/ufs/ufs-qcom.h b/drivers/scsi/ufs/ufs-qcom.h
index fe517cd..076f528 100644
--- a/drivers/scsi/ufs/ufs-qcom.h
+++ b/drivers/scsi/ufs/ufs-qcom.h
@@ -95,6 +95,7 @@ enum {
#define QUNIPRO_SELUFS_BIT(0)
#define TEST_BUS_ENBIT(18)
#define TEST_BUS_SEL GENMASK(22, 19)
+#define UFS_REG_TEST_BUS_ENBIT(30)
/* bit definitions for REG_UFS_CFG2 register */
#define UAWM_HW_CGC_EN (1 << 0)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [REGRESSION v4.10-rc1] blkdev_issue_zeroout() returns -EREMOTEIO on the first call for SCSI device that doesn't support WRITE SAME
On 02/03/2017 03:45 PM, Martin K. Petersen wrote: >> "Jens" == Jens Axboe writes: > >>> I think we should fix sd.c to only send WRITE SAME if either of the >>> variants are explicitly listed as supported through REPORT SUPPORTED >>> OPERATION CODES, or maybe through a whitelist if there are important >>> enough devices. > > Jens> Yep > > I hate it too. But the reason it's assumed on is that there is > essentially no heuristic that works. Just like we assume that READ > always works. > > Out of the ~200 devices I have access to in the lab: > > - 100% of the SAS/FC disk drives and SSDs support WRITE SAME > - Only 2 out of about 50 different drive models support RSOC > - About half of the arrays support WRITE SAME(10/16) > - None of the arrays I have support RSOC > > So even if we were to entertain using RSOC for "enterprise" transport > classes (which I concur would be nice for other reasons), it wouldn't > solve the WRITE SAME problem... We're at (almost) -rc7 time, we have to do more than hand wave about this. What's the plan for 4.10 final? -- Jens Axboe
[PATCH] net-next: treewide use is_vlan_dev() helper function.
This patch makes use of is_vlan_dev() function instead of flag
comparison which is exactly done by is_vlan_dev() helper function.
Signed-off-by: Parav Pandit
Reviewed-by: Daniel Jurgens
---
drivers/infiniband/core/cma.c| 6 ++
drivers/infiniband/sw/rxe/rxe_net.c | 2 +-
drivers/net/ethernet/broadcom/cnic.c | 2 +-
drivers/net/ethernet/chelsio/cxgb3/l2t.c | 2 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 4 ++--
drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c | 8
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 4 ++--
drivers/net/hyperv/netvsc_drv.c | 2 +-
drivers/scsi/bnx2fc/bnx2fc_fcoe.c| 6 +++---
drivers/scsi/cxgbi/libcxgbi.c| 6 +++---
drivers/scsi/fcoe/fcoe.c | 13 ++---
include/rdma/ib_addr.h | 6 ++
net/hsr/hsr_slave.c | 3 ++-
13 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 3e70a9c..4eb5a80 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2467,14 +2467,12 @@ static int iboe_tos_to_sl(struct net_device *ndev, int
tos)
struct net_device *dev;
prio = rt_tos2priority(tos);
- dev = ndev->priv_flags & IFF_802_1Q_VLAN ?
- vlan_dev_real_dev(ndev) : ndev;
-
+ dev = is_vlan_dev(ndev) ? vlan_dev_real_dev(ndev) : ndev;
if (dev->num_tc)
return netdev_get_prio_tc_map(dev, prio);
#if IS_ENABLED(CONFIG_VLAN_8021Q)
- if (ndev->priv_flags & IFF_802_1Q_VLAN)
+ if (is_vlan_dev(ndev))
return (vlan_dev_get_egress_qos_mask(ndev, prio) &
VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
#endif
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
b/drivers/infiniband/sw/rxe/rxe_net.c
index 4abdeb3..d9d1556 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -118,7 +118,7 @@ static struct device *dma_device(struct rxe_dev *rxe)
ndev = rxe->ndev;
- if (ndev->priv_flags & IFF_802_1Q_VLAN)
+ if (is_vlan_dev(ndev))
ndev = vlan_dev_real_dev(ndev);
return ndev->dev.parent;
diff --git a/drivers/net/ethernet/broadcom/cnic.c
b/drivers/net/ethernet/broadcom/cnic.c
index b1d2ac8..cec94bb 100644
--- a/drivers/net/ethernet/broadcom/cnic.c
+++ b/drivers/net/ethernet/broadcom/cnic.c
@@ -3665,7 +3665,7 @@ static int cnic_cm_destroy(struct cnic_sock *csk)
static inline u16 cnic_get_vlan(struct net_device *dev,
struct net_device **vlan_dev)
{
- if (dev->priv_flags & IFF_802_1Q_VLAN) {
+ if (is_vlan_dev(dev)) {
*vlan_dev = vlan_dev_real_dev(dev);
return vlan_dev_vlan_id(dev);
}
diff --git a/drivers/net/ethernet/chelsio/cxgb3/l2t.c
b/drivers/net/ethernet/chelsio/cxgb3/l2t.c
index 5f226ed..5206358 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/l2t.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/l2t.c
@@ -351,7 +351,7 @@ struct l2t_entry *t3_l2t_get(struct t3cdev *cdev, struct
dst_entry *dst,
e->smt_idx = smt_idx;
atomic_set(&e->refcnt, 1);
neigh_replace(e, neigh);
- if (neigh->dev->priv_flags & IFF_802_1Q_VLAN)
+ if (is_vlan_dev(neigh->dev))
e->vlan = vlan_dev_vlan_id(neigh->dev);
else
e->vlan = VLAN_NONE;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index f4f5690..7059014 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -1805,7 +1805,7 @@ static void check_neigh_update(struct neighbour *neigh)
const struct device *parent;
const struct net_device *netdev = neigh->dev;
- if (netdev->priv_flags & IFF_802_1Q_VLAN)
+ if (is_vlan_dev(netdev))
netdev = vlan_dev_real_dev(netdev);
parent = netdev->dev.parent;
if (parent && parent->driver == &cxgb4_driver.driver)
@@ -2111,7 +2111,7 @@ static int cxgb4_inet6addr_handler(struct notifier_block
*this,
#if IS_ENABLED(CONFIG_BONDING)
struct adapter *adap;
#endif
- if (event_dev->priv_flags & IFF_802_1Q_VLAN)
+ if (is_vlan_dev(event_dev))
event_dev = vlan_dev_real_dev(event_dev);
#if IS_ENABLED(CONFIG_BONDING)
if (event_dev->flags & IFF_MASTER) {
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index 0cf8a37..3b5d7cf 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -3264,7 +3264,7 @@ static ssize_t
Re: [PATCH] net-next: treewide use is_vlan_dev() helper function.
On Fri, 2017-02-03 at 22:26 -0600, Parav Pandit wrote: > This patch makes use of is_vlan_dev() function instead of flag > comparison which is exactly done by is_vlan_dev() helper function. Thanks. btw: after applying this patch, there is one left $ git grep -E -n "&\s*IFF_802_1Q_VLAN\b" -- "*.c" drivers/net/ethernet/chelsio/cxgb4/l2t.c:435: if (neigh->dev->priv_flags & IFF_802_1Q_VLAN)
