* [PATCH v2 0/7] Fix hfi1_ioctl locking
@ 2015-11-16 22:32 ira.weiny-ral2JQCrhuEAvxtiuMwx3w
[not found] ` <1447713160-24858-1-git-send-email-ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: ira.weiny-ral2JQCrhuEAvxtiuMwx3w @ 2015-11-16 22:32 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, Ira Weiny
From: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
It was identified that hfi1_ioctl may sleep with a spin lock held.
This was identified publicly here:
http://www.spinics.net/lists/linux-rdma/msg29926.html
As well as by our internal development.
This series cleans up the code and parameter checks, as well as fixing the
locking.
---
Changes from v1
Alter switch to just ignore set link state and default to return -ENOTTY
Remove patch which introduced an intermediate bug and squash it in the final patch
Series is now 7 patches rather than 8
Dennis Dalessandro (2):
staging/rdma/hfi1: Reduce snoop locking scope in IOCTL handler.
staging/rdma/hfi1: Return immediately on error
Ira Weiny (4):
staging/rdma/hfi1: Fix camel case variables
staging/rdma/hfi1: Return early from hfi1_ioctl parameter errors
staging/rdma/hfi1: hfi1_ioctl remove setlink state
staging/rdma/hfi1: Further clean up hfi1_ioctl parameter checks
Jubin John (1):
staging/rdma/hfi1: diag.c Correct code style issues
drivers/staging/rdma/hfi1/diag.c | 461 +++++++++++++++++++--------------------
1 file changed, 219 insertions(+), 242 deletions(-)
--
1.8.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues
[not found] ` <1447713160-24858-1-git-send-email-ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2015-11-16 22:32 ` ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-17 16:47 ` Sudip Mukherjee
2015-11-16 22:32 ` [PATCH v2 2/7] staging/rdma/hfi1: Fix camel case variables ira.weiny-ral2JQCrhuEAvxtiuMwx3w
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: ira.weiny-ral2JQCrhuEAvxtiuMwx3w @ 2015-11-16 22:32 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, Jubin John, Ira Weiny
From: Jubin John <jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Correct the checks on diag.c with the latest checkpatch
Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Jubin John <jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/staging/rdma/hfi1/diag.c | 81 ++++++++++++++++++----------------------
1 file changed, 37 insertions(+), 44 deletions(-)
diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c
index 0aaad7412842..c5cc2db1b559 100644
--- a/drivers/staging/rdma/hfi1/diag.c
+++ b/drivers/staging/rdma/hfi1/diag.c
@@ -78,8 +78,8 @@
hfi1_cdbg(SNOOP, fmt, ##__VA_ARGS__)
/* Snoop option mask */
-#define SNOOP_DROP_SEND (1 << 0)
-#define SNOOP_USE_METADATA (1 << 1)
+#define SNOOP_DROP_SEND BIT(0)
+#define SNOOP_USE_METADATA BIT(1)
static u8 snoop_flags;
@@ -135,7 +135,7 @@ static struct cdev diagpkt_cdev;
static struct device *diagpkt_device;
static ssize_t diagpkt_write(struct file *fp, const char __user *data,
- size_t count, loff_t *off);
+ size_t count, loff_t *off);
static const struct file_operations diagpkt_file_ops = {
.owner = THIS_MODULE,
@@ -177,37 +177,37 @@ struct hfi1_link_info {
#define HFI1_SNOOP_IOCGETLINKSTATE \
_IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ)
#define HFI1_SNOOP_IOCSETLINKSTATE \
- _IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ+1)
+ _IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ + 1)
#define HFI1_SNOOP_IOCCLEARQUEUE \
- _IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ+2)
+ _IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ + 2)
#define HFI1_SNOOP_IOCCLEARFILTER \
- _IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ+3)
+ _IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ + 3)
#define HFI1_SNOOP_IOCSETFILTER \
- _IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ+4)
+ _IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ + 4)
#define HFI1_SNOOP_IOCGETVERSION \
- _IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ+5)
+ _IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ + 5)
#define HFI1_SNOOP_IOCSET_OPTS \
- _IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ+6)
+ _IO(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ + 6)
/*
* These offsets +6/+7 could change, but these are already known and used
* IOCTL numbers so don't change them without a good reason.
*/
#define HFI1_SNOOP_IOCGETLINKSTATE_EXTRA \
- _IOWR(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ+6, \
+ _IOWR(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ + 6, \
struct hfi1_link_info)
#define HFI1_SNOOP_IOCSETLINKSTATE_EXTRA \
- _IOWR(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ+7, \
+ _IOWR(HFI1_SNOOP_IOC_MAGIC, HFI1_SNOOP_IOC_BASE_SEQ + 7, \
struct hfi1_link_info)
static int hfi1_snoop_open(struct inode *in, struct file *fp);
static ssize_t hfi1_snoop_read(struct file *fp, char __user *data,
- size_t pkt_len, loff_t *off);
+ size_t pkt_len, loff_t *off);
static ssize_t hfi1_snoop_write(struct file *fp, const char __user *data,
- size_t count, loff_t *off);
+ size_t count, loff_t *off);
static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
static unsigned int hfi1_snoop_poll(struct file *fp,
- struct poll_table_struct *wait);
+ struct poll_table_struct *wait);
static int hfi1_snoop_release(struct inode *in, struct file *fp);
struct hfi1_packet_filter_command {
@@ -323,14 +323,12 @@ static void hfi1_snoop_remove(struct hfi1_devdata *dd)
void hfi1_diag_remove(struct hfi1_devdata *dd)
{
-
hfi1_snoop_remove(dd);
if (atomic_dec_and_test(&diagpkt_count))
hfi1_cdev_cleanup(&diagpkt_cdev, &diagpkt_device);
hfi1_cdev_cleanup(&dd->diag_cdev, &dd->diag_device);
}
-
/*
* Allocated structure shared between the credit return mechanism and
* diagpkt_send().
@@ -393,7 +391,7 @@ static ssize_t diagpkt_send(struct diag_pkt *dp)
if (dp->version != _DIAG_PKT_VERS) {
dd_dev_err(dd, "Invalid version %u for diagpkt_write\n",
- dp->version);
+ dp->version);
ret = -EINVAL;
goto bail;
}
@@ -440,7 +438,7 @@ static ssize_t diagpkt_send(struct diag_pkt *dp)
}
if (copy_from_user(tmpbuf,
- (const void __user *) (unsigned long) dp->data,
+ (const void __user *)(unsigned long)dp->data,
dp->len)) {
ret = -EFAULT;
goto bail;
@@ -530,9 +528,9 @@ static ssize_t diagpkt_send(struct diag_pkt *dp)
* NOTE: PRC_FILL_ERR is at best informational and cannot
* be depended on.
*/
- if (!ret && (((wait->code & PRC_STATUS_ERR)
- || (wait->code & PRC_FILL_ERR)
- || (wait->code & PRC_SC_DISABLE))))
+ if (!ret && (((wait->code & PRC_STATUS_ERR) ||
+ (wait->code & PRC_FILL_ERR) ||
+ (wait->code & PRC_SC_DISABLE))))
ret = -EIO;
put_diagpkt_wait(wait); /* finished with the structure */
@@ -545,7 +543,7 @@ bail:
}
static ssize_t diagpkt_write(struct file *fp, const char __user *data,
- size_t count, loff_t *off)
+ size_t count, loff_t *off)
{
struct hfi1_devdata *dd;
struct send_context *sc;
@@ -565,7 +563,7 @@ static ssize_t diagpkt_write(struct file *fp, const char __user *data,
*/
if (dp.pbc) {
dd = hfi1_lookup(dp.unit);
- if (dd == NULL)
+ if (!dd)
return -ENODEV;
vl = (dp.pbc >> PBC_VL_SHIFT) & PBC_VL_MASK;
sc = dd->vld[vl].sc;
@@ -598,7 +596,7 @@ static int hfi1_snoop_add(struct hfi1_devdata *dd, const char *name)
if (ret) {
dd_dev_err(dd, "Couldn't create %s device: %d", name, ret);
hfi1_cdev_cleanup(&dd->hfi1_snoop.cdev,
- &dd->hfi1_snoop.class_dev);
+ &dd->hfi1_snoop.class_dev);
}
return ret;
@@ -611,7 +609,6 @@ static struct hfi1_devdata *hfi1_dd_from_sc_inode(struct inode *in)
dd = hfi1_lookup(unit);
return dd;
-
}
/* clear or restore send context integrity checks */
@@ -652,7 +649,7 @@ static int hfi1_snoop_open(struct inode *in, struct file *fp)
mutex_lock(&hfi1_mutex);
dd = hfi1_dd_from_sc_inode(in);
- if (dd == NULL) {
+ if (!dd) {
ret = -ENODEV;
goto bail;
}
@@ -739,7 +736,7 @@ static int hfi1_snoop_release(struct inode *in, struct file *fp)
int mode_flag;
dd = hfi1_dd_from_sc_inode(in);
- if (dd == NULL)
+ if (!dd)
return -ENODEV;
spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
@@ -794,7 +791,7 @@ static unsigned int hfi1_snoop_poll(struct file *fp,
struct hfi1_devdata *dd;
dd = hfi1_dd_from_sc_inode(fp->f_inode);
- if (dd == NULL)
+ if (!dd)
return -ENODEV;
spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
@@ -805,7 +802,6 @@ static unsigned int hfi1_snoop_poll(struct file *fp,
spin_unlock_irqrestore(&dd->hfi1_snoop.snoop_lock, flags);
return ret;
-
}
static ssize_t hfi1_snoop_write(struct file *fp, const char __user *data,
@@ -822,7 +818,7 @@ static ssize_t hfi1_snoop_write(struct file *fp, const char __user *data,
struct hfi1_pportdata *ppd;
dd = hfi1_dd_from_sc_inode(fp->f_inode);
- if (dd == NULL)
+ if (!dd)
return -ENODEV;
ppd = dd->pport;
@@ -847,7 +843,7 @@ static ssize_t hfi1_snoop_write(struct file *fp, const char __user *data,
if (copy_from_user(&byte_one, data, 1))
return -EINVAL;
- if (copy_from_user(&byte_two, data+1, 1))
+ if (copy_from_user(&byte_two, data + 1, 1))
return -EINVAL;
sc4 = (byte_one >> 4) & 0xf;
@@ -920,7 +916,7 @@ static ssize_t hfi1_snoop_read(struct file *fp, char __user *data,
struct hfi1_devdata *dd;
dd = hfi1_dd_from_sc_inode(fp->f_inode);
- if (dd == NULL)
+ if (!dd)
return -ENODEV;
spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
@@ -946,16 +942,18 @@ static ssize_t hfi1_snoop_read(struct file *fp, char __user *data,
spin_unlock_irqrestore(&dd->hfi1_snoop.snoop_lock, flags);
if (pkt_len >= packet->total_len) {
if (copy_to_user(data, packet->data,
- packet->total_len))
+ packet->total_len))
ret = -EFAULT;
else
ret = packet->total_len;
- } else
+ } else {
ret = -EINVAL;
+ }
kfree(packet);
- } else
+ } else {
spin_unlock_irqrestore(&dd->hfi1_snoop.snoop_lock, flags);
+ }
return ret;
}
@@ -1321,7 +1319,6 @@ static int hfi1_filter_mad_mgmt_class(void *ibhdr, void *packet_data,
static int hfi1_filter_qp_number(void *ibhdr, void *packet_data, void *value)
{
-
struct hfi1_ib_header *hdr;
struct hfi1_other_headers *ohdr = NULL;
int ret;
@@ -1404,7 +1401,6 @@ static int hfi1_filter_ib_service_level(void *ibhdr, void *packet_data,
static int hfi1_filter_ib_pkey(void *ibhdr, void *packet_data, void *value)
{
-
u32 lnh = 0;
struct hfi1_ib_header *hdr;
struct hfi1_other_headers *ohdr = NULL;
@@ -1479,13 +1475,12 @@ static struct snoop_packet *allocate_snoop_packet(u32 hdr_len,
struct snoop_packet *packet;
- packet = kzalloc(sizeof(struct snoop_packet) + hdr_len + data_len
+ packet = kzalloc(sizeof(*packet) + hdr_len + data_len
+ md_len,
GFP_ATOMIC | __GFP_NOWARN);
if (likely(packet))
INIT_LIST_HEAD(&packet->list);
-
return packet;
}
@@ -1542,12 +1537,11 @@ int snoop_recv_handler(struct hfi1_packet *packet)
unlikely(snoop_flags & SNOOP_USE_METADATA))
md_len = sizeof(struct capture_md);
-
s_packet = allocate_snoop_packet(header_size,
tlen - header_size,
md_len);
- if (unlikely(s_packet == NULL)) {
+ if (unlikely(!s_packet)) {
dd_dev_warn_ratelimited(ppd->dd, "Unable to allocate snoop/capture packet\n");
break;
}
@@ -1668,7 +1662,7 @@ int snoop_send_pio_handler(struct hfi1_qp *qp, struct hfi1_pkt_state *ps,
/* not using ss->total_len as arg 2 b/c that does not count CRC */
s_packet = allocate_snoop_packet(hdr_len, tlen - hdr_len, md_len);
- if (unlikely(s_packet == NULL)) {
+ if (unlikely(!s_packet)) {
dd_dev_warn_ratelimited(ppd->dd, "Unable to allocate snoop/capture packet\n");
goto out;
}
@@ -1837,7 +1831,7 @@ void snoop_inline_pio_send(struct hfi1_devdata *dd, struct pio_buf *pbuf,
s_packet = allocate_snoop_packet(packet_len, 0, md_len);
- if (unlikely(s_packet == NULL)) {
+ if (unlikely(!s_packet)) {
dd_dev_warn_ratelimited(dd, "Unable to allocate snoop/capture packet\n");
goto inline_pio_out;
}
@@ -1869,5 +1863,4 @@ void snoop_inline_pio_send(struct hfi1_devdata *dd, struct pio_buf *pbuf,
inline_pio_out:
pio_copy(dd, pbuf, pbc, from, count);
-
}
--
1.8.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/7] staging/rdma/hfi1: Fix camel case variables
[not found] ` <1447713160-24858-1-git-send-email-ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2015-11-16 22:32 ` [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues ira.weiny-ral2JQCrhuEAvxtiuMwx3w
@ 2015-11-16 22:32 ` ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-16 22:32 ` [PATCH v2 3/7] staging/rdma/hfi1: Return early from hfi1_ioctl parameter errors ira.weiny-ral2JQCrhuEAvxtiuMwx3w
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: ira.weiny-ral2JQCrhuEAvxtiuMwx3w @ 2015-11-16 22:32 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, Ira Weiny,
Dennis Dalessandro
From: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
physState, linkState, and devState should be phys_state, link_state, and
dev_state
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/staging/rdma/hfi1/diag.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c
index c5cc2db1b559..c2839473f983 100644
--- a/drivers/staging/rdma/hfi1/diag.c
+++ b/drivers/staging/rdma/hfi1/diag.c
@@ -964,9 +964,9 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
void *filter_value = NULL;
long ret = 0;
int value = 0;
- u8 physState = 0;
- u8 linkState = 0;
- u16 devState = 0;
+ u8 phys_state = 0;
+ u8 link_state = 0;
+ u16 dev_state = 0;
unsigned long flags = 0;
unsigned long *argp = NULL;
struct hfi1_packet_filter_command filter_cmd = {0};
@@ -1030,31 +1030,31 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
}
/* What we want to transition to */
- physState = (value >> 4) & 0xF;
- linkState = value & 0xF;
+ phys_state = (value >> 4) & 0xF;
+ link_state = value & 0xF;
snoop_dbg("Setting link state 0x%x", value);
- switch (linkState) {
+ switch (link_state) {
case IB_PORT_NOP:
- if (physState == 0)
+ if (phys_state == 0)
break;
/* fall through */
case IB_PORT_DOWN:
- switch (physState) {
+ switch (phys_state) {
case 0:
- devState = HLS_DN_DOWNDEF;
+ dev_state = HLS_DN_DOWNDEF;
break;
case 2:
- devState = HLS_DN_POLL;
+ dev_state = HLS_DN_POLL;
break;
case 3:
- devState = HLS_DN_DISABLE;
+ dev_state = HLS_DN_DISABLE;
break;
default:
ret = -EINVAL;
goto done;
}
- ret = set_link_state(ppd, devState);
+ ret = set_link_state(ppd, dev_state);
break;
case IB_PORT_ARMED:
ret = set_link_state(ppd, HLS_UP_ARMED);
--
1.8.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/7] staging/rdma/hfi1: Return early from hfi1_ioctl parameter errors
[not found] ` <1447713160-24858-1-git-send-email-ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2015-11-16 22:32 ` [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-16 22:32 ` [PATCH v2 2/7] staging/rdma/hfi1: Fix camel case variables ira.weiny-ral2JQCrhuEAvxtiuMwx3w
@ 2015-11-16 22:32 ` ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-16 22:32 ` [PATCH v2 4/7] staging/rdma/hfi1: hfi1_ioctl remove setlink state ira.weiny-ral2JQCrhuEAvxtiuMwx3w
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: ira.weiny-ral2JQCrhuEAvxtiuMwx3w @ 2015-11-16 22:32 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, Ira Weiny,
Dennis Dalessandro
From: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Rather than have a switch in a large else clause make the parameter checks
return immediately.
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/staging/rdma/hfi1/diag.c | 348 +++++++++++++++++++--------------------
1 file changed, 173 insertions(+), 175 deletions(-)
diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c
index c2839473f983..2bb857b2a103 100644
--- a/drivers/staging/rdma/hfi1/diag.c
+++ b/drivers/staging/rdma/hfi1/diag.c
@@ -979,17 +979,15 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
if (dd == NULL)
return -ENODEV;
- spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
-
mode_flag = dd->hfi1_snoop.mode_flag;
if (((_IOC_DIR(cmd) & _IOC_READ)
&& !access_ok(VERIFY_WRITE, (void __user *)arg, _IOC_SIZE(cmd)))
|| ((_IOC_DIR(cmd) & _IOC_WRITE)
&& !access_ok(VERIFY_READ, (void __user *)arg, _IOC_SIZE(cmd)))) {
- ret = -EFAULT;
+ return -EFAULT;
} else if (!capable(CAP_SYS_ADMIN)) {
- ret = -EPERM;
+ return -EPERM;
} else if ((mode_flag & HFI1_PORT_CAPTURE_MODE) &&
(cmd != HFI1_SNOOP_IOCCLEARQUEUE) &&
(cmd != HFI1_SNOOP_IOCCLEARFILTER) &&
@@ -1000,208 +998,208 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
* 3.Set capture filter
* Other are invalid.
*/
+ return -EINVAL;
+ }
+
+ spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
+
+ switch (cmd) {
+ case HFI1_SNOOP_IOCSETLINKSTATE:
+ snoop_dbg("HFI1_SNOOP_IOCSETLINKSTATE is not valid");
ret = -EINVAL;
- } else {
- switch (cmd) {
- case HFI1_SNOOP_IOCSETLINKSTATE:
- snoop_dbg("HFI1_SNOOP_IOCSETLINKSTATE is not valid");
+ break;
+
+ case HFI1_SNOOP_IOCSETLINKSTATE_EXTRA:
+ memset(&link_info, 0, sizeof(link_info));
+
+ if (copy_from_user(&link_info,
+ (struct hfi1_link_info __user *)arg,
+ sizeof(link_info)))
+ ret = -EFAULT;
+
+ value = link_info.port_state;
+ index = link_info.port_number;
+ if (index > dd->num_pports - 1) {
ret = -EINVAL;
break;
+ }
- case HFI1_SNOOP_IOCSETLINKSTATE_EXTRA:
- memset(&link_info, 0, sizeof(link_info));
+ ppd = &dd->pport[index];
+ if (!ppd) {
+ ret = -EINVAL;
+ break;
+ }
- if (copy_from_user(&link_info,
- (struct hfi1_link_info __user *)arg,
- sizeof(link_info)))
- ret = -EFAULT;
+ /* What we want to transition to */
+ phys_state = (value >> 4) & 0xF;
+ link_state = value & 0xF;
+ snoop_dbg("Setting link state 0x%x", value);
- value = link_info.port_state;
- index = link_info.port_number;
- if (index > dd->num_pports - 1) {
- ret = -EINVAL;
+ switch (link_state) {
+ case IB_PORT_NOP:
+ if (phys_state == 0)
break;
- }
-
- ppd = &dd->pport[index];
- if (!ppd) {
- ret = -EINVAL;
- break;
- }
-
- /* What we want to transition to */
- phys_state = (value >> 4) & 0xF;
- link_state = value & 0xF;
- snoop_dbg("Setting link state 0x%x", value);
-
- switch (link_state) {
- case IB_PORT_NOP:
- if (phys_state == 0)
- break;
- /* fall through */
- case IB_PORT_DOWN:
- switch (phys_state) {
- case 0:
- dev_state = HLS_DN_DOWNDEF;
- break;
- case 2:
- dev_state = HLS_DN_POLL;
- break;
- case 3:
- dev_state = HLS_DN_DISABLE;
- break;
- default:
- ret = -EINVAL;
- goto done;
- }
- ret = set_link_state(ppd, dev_state);
+ /* fall through */
+ case IB_PORT_DOWN:
+ switch (phys_state) {
+ case 0:
+ dev_state = HLS_DN_DOWNDEF;
break;
- case IB_PORT_ARMED:
- ret = set_link_state(ppd, HLS_UP_ARMED);
- if (!ret)
- send_idle_sma(dd, SMA_IDLE_ARM);
+ case 2:
+ dev_state = HLS_DN_POLL;
break;
- case IB_PORT_ACTIVE:
- ret = set_link_state(ppd, HLS_UP_ACTIVE);
- if (!ret)
- send_idle_sma(dd, SMA_IDLE_ACTIVE);
+ case 3:
+ dev_state = HLS_DN_DISABLE;
break;
default:
ret = -EINVAL;
- break;
- }
-
- if (ret)
- break;
- /* fall through */
- case HFI1_SNOOP_IOCGETLINKSTATE:
- case HFI1_SNOOP_IOCGETLINKSTATE_EXTRA:
- if (cmd == HFI1_SNOOP_IOCGETLINKSTATE_EXTRA) {
- memset(&link_info, 0, sizeof(link_info));
- if (copy_from_user(&link_info,
- (struct hfi1_link_info __user *)arg,
- sizeof(link_info)))
- ret = -EFAULT;
- index = link_info.port_number;
- } else {
- ret = __get_user(index, (int __user *) arg);
- if (ret != 0)
- break;
- }
-
- if (index > dd->num_pports - 1) {
- ret = -EINVAL;
- break;
- }
-
- ppd = &dd->pport[index];
- if (!ppd) {
- ret = -EINVAL;
- break;
- }
- value = hfi1_ibphys_portstate(ppd);
- value <<= 4;
- value |= driver_lstate(ppd);
-
- snoop_dbg("Link port | Link State: %d", value);
-
- if ((cmd == HFI1_SNOOP_IOCGETLINKSTATE_EXTRA) ||
- (cmd == HFI1_SNOOP_IOCSETLINKSTATE_EXTRA)) {
- link_info.port_state = value;
- link_info.node_guid = cpu_to_be64(ppd->guid);
- link_info.link_speed_active =
- ppd->link_speed_active;
- link_info.link_width_active =
- ppd->link_width_active;
- if (copy_to_user(
- (struct hfi1_link_info __user *)arg,
- &link_info, sizeof(link_info)))
- ret = -EFAULT;
- } else {
- ret = __put_user(value, (int __user *)arg);
+ goto done;
}
+ ret = set_link_state(ppd, dev_state);
break;
-
- case HFI1_SNOOP_IOCCLEARQUEUE:
- snoop_dbg("Clearing snoop queue");
- drain_snoop_list(&dd->hfi1_snoop.queue);
+ case IB_PORT_ARMED:
+ ret = set_link_state(ppd, HLS_UP_ARMED);
+ if (!ret)
+ send_idle_sma(dd, SMA_IDLE_ARM);
break;
-
- case HFI1_SNOOP_IOCCLEARFILTER:
- snoop_dbg("Clearing filter");
- if (dd->hfi1_snoop.filter_callback) {
- /* Drain packets first */
- drain_snoop_list(&dd->hfi1_snoop.queue);
- dd->hfi1_snoop.filter_callback = NULL;
- }
- kfree(dd->hfi1_snoop.filter_value);
- dd->hfi1_snoop.filter_value = NULL;
+ case IB_PORT_ACTIVE:
+ ret = set_link_state(ppd, HLS_UP_ACTIVE);
+ if (!ret)
+ send_idle_sma(dd, SMA_IDLE_ACTIVE);
+ break;
+ default:
+ ret = -EINVAL;
break;
+ }
- case HFI1_SNOOP_IOCSETFILTER:
- snoop_dbg("Setting filter");
- /* just copy command structure */
- argp = (unsigned long *)arg;
- if (copy_from_user(&filter_cmd, (void __user *)argp,
- sizeof(filter_cmd))) {
+ if (ret)
+ break;
+ /* fall through */
+ case HFI1_SNOOP_IOCGETLINKSTATE:
+ case HFI1_SNOOP_IOCGETLINKSTATE_EXTRA:
+ if (cmd == HFI1_SNOOP_IOCGETLINKSTATE_EXTRA) {
+ memset(&link_info, 0, sizeof(link_info));
+ if (copy_from_user(&link_info,
+ (struct hfi1_link_info __user *)arg,
+ sizeof(link_info)))
ret = -EFAULT;
+ index = link_info.port_number;
+ } else {
+ ret = __get_user(index, (int __user *)arg);
+ if (ret != 0)
break;
- }
- if (filter_cmd.opcode >= HFI1_MAX_FILTERS) {
- pr_alert("Invalid opcode in request\n");
- ret = -EINVAL;
- break;
- }
+ }
- snoop_dbg("Opcode %d Len %d Ptr %p",
- filter_cmd.opcode, filter_cmd.length,
- filter_cmd.value_ptr);
+ if (index > dd->num_pports - 1) {
+ ret = -EINVAL;
+ break;
+ }
- filter_value = kcalloc(filter_cmd.length, sizeof(u8),
- GFP_KERNEL);
- if (!filter_value) {
- pr_alert("Not enough memory\n");
- ret = -ENOMEM;
- break;
- }
- /* copy remaining data from userspace */
- if (copy_from_user((u8 *)filter_value,
- (void __user *)filter_cmd.value_ptr,
- filter_cmd.length)) {
- kfree(filter_value);
+ ppd = &dd->pport[index];
+ if (!ppd) {
+ ret = -EINVAL;
+ break;
+ }
+ value = hfi1_ibphys_portstate(ppd);
+ value <<= 4;
+ value |= driver_lstate(ppd);
+
+ snoop_dbg("Link port | Link State: %d", value);
+
+ if ((cmd == HFI1_SNOOP_IOCGETLINKSTATE_EXTRA) ||
+ (cmd == HFI1_SNOOP_IOCSETLINKSTATE_EXTRA)) {
+ link_info.port_state = value;
+ link_info.node_guid = cpu_to_be64(ppd->guid);
+ link_info.link_speed_active =
+ ppd->link_speed_active;
+ link_info.link_width_active =
+ ppd->link_width_active;
+ if (copy_to_user((struct hfi1_link_info __user *)arg,
+ &link_info, sizeof(link_info)))
ret = -EFAULT;
- break;
- }
+ } else {
+ ret = __put_user(value, (int __user *)arg);
+ }
+ break;
+
+ case HFI1_SNOOP_IOCCLEARQUEUE:
+ snoop_dbg("Clearing snoop queue");
+ drain_snoop_list(&dd->hfi1_snoop.queue);
+ break;
+
+ case HFI1_SNOOP_IOCCLEARFILTER:
+ snoop_dbg("Clearing filter");
+ if (dd->hfi1_snoop.filter_callback) {
/* Drain packets first */
drain_snoop_list(&dd->hfi1_snoop.queue);
- dd->hfi1_snoop.filter_callback =
- hfi1_filters[filter_cmd.opcode].filter;
- /* just in case we see back to back sets */
- kfree(dd->hfi1_snoop.filter_value);
- dd->hfi1_snoop.filter_value = filter_value;
+ dd->hfi1_snoop.filter_callback = NULL;
+ }
+ kfree(dd->hfi1_snoop.filter_value);
+ dd->hfi1_snoop.filter_value = NULL;
+ break;
+ case HFI1_SNOOP_IOCSETFILTER:
+ snoop_dbg("Setting filter");
+ /* just copy command structure */
+ argp = (unsigned long *)arg;
+ if (copy_from_user(&filter_cmd, (void __user *)argp,
+ sizeof(filter_cmd))) {
+ ret = -EFAULT;
break;
- case HFI1_SNOOP_IOCGETVERSION:
- value = SNOOP_CAPTURE_VERSION;
- snoop_dbg("Getting version: %d", value);
- ret = __put_user(value, (int __user *)arg);
+ }
+ if (filter_cmd.opcode >= HFI1_MAX_FILTERS) {
+ pr_alert("Invalid opcode in request\n");
+ ret = -EINVAL;
break;
- case HFI1_SNOOP_IOCSET_OPTS:
- snoop_flags = 0;
- ret = __get_user(value, (int __user *) arg);
- if (ret != 0)
- break;
+ }
- snoop_dbg("Setting snoop option %d", value);
- if (value & SNOOP_DROP_SEND)
- snoop_flags |= SNOOP_DROP_SEND;
- if (value & SNOOP_USE_METADATA)
- snoop_flags |= SNOOP_USE_METADATA;
+ snoop_dbg("Opcode %d Len %d Ptr %p",
+ filter_cmd.opcode, filter_cmd.length,
+ filter_cmd.value_ptr);
+
+ filter_value = kcalloc(filter_cmd.length, sizeof(u8),
+ GFP_KERNEL);
+ if (!filter_value) {
+ ret = -ENOMEM;
break;
- default:
- ret = -ENOTTY;
+ }
+ /* copy remaining data from userspace */
+ if (copy_from_user((u8 *)filter_value,
+ (void __user *)filter_cmd.value_ptr,
+ filter_cmd.length)) {
+ kfree(filter_value);
+ ret = -EFAULT;
break;
}
+ /* Drain packets first */
+ drain_snoop_list(&dd->hfi1_snoop.queue);
+ dd->hfi1_snoop.filter_callback =
+ hfi1_filters[filter_cmd.opcode].filter;
+ /* just in case we see back to back sets */
+ kfree(dd->hfi1_snoop.filter_value);
+ dd->hfi1_snoop.filter_value = filter_value;
+
+ break;
+ case HFI1_SNOOP_IOCGETVERSION:
+ value = SNOOP_CAPTURE_VERSION;
+ snoop_dbg("Getting version: %d", value);
+ ret = __put_user(value, (int __user *)arg);
+ break;
+ case HFI1_SNOOP_IOCSET_OPTS:
+ snoop_flags = 0;
+ ret = __get_user(value, (int __user *)arg);
+ if (ret != 0)
+ break;
+
+ snoop_dbg("Setting snoop option %d", value);
+ if (value & SNOOP_DROP_SEND)
+ snoop_flags |= SNOOP_DROP_SEND;
+ if (value & SNOOP_USE_METADATA)
+ snoop_flags |= SNOOP_USE_METADATA;
+ break;
+ default:
+ ret = -ENOTTY;
+ break;
}
done:
spin_unlock_irqrestore(&dd->hfi1_snoop.snoop_lock, flags);
--
1.8.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/7] staging/rdma/hfi1: hfi1_ioctl remove setlink state
[not found] ` <1447713160-24858-1-git-send-email-ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
` (2 preceding siblings ...)
2015-11-16 22:32 ` [PATCH v2 3/7] staging/rdma/hfi1: Return early from hfi1_ioctl parameter errors ira.weiny-ral2JQCrhuEAvxtiuMwx3w
@ 2015-11-16 22:32 ` ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-16 22:32 ` [PATCH v2 5/7] staging/rdma/hfi1: Further clean up hfi1_ioctl parameter checks ira.weiny-ral2JQCrhuEAvxtiuMwx3w
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: ira.weiny-ral2JQCrhuEAvxtiuMwx3w @ 2015-11-16 22:32 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, Ira Weiny,
Dennis Dalessandro
From: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Set link state is not supported remove from the switch statement and allow the
default to return -ENOTTY
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
Changes from v1:
Just remove the check for SETLINKSTATE and allow the switch to default
to -ENOTTY
Update commit message to reflect this
drivers/staging/rdma/hfi1/diag.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c
index 2bb857b2a103..979890b40194 100644
--- a/drivers/staging/rdma/hfi1/diag.c
+++ b/drivers/staging/rdma/hfi1/diag.c
@@ -1004,11 +1004,6 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
switch (cmd) {
- case HFI1_SNOOP_IOCSETLINKSTATE:
- snoop_dbg("HFI1_SNOOP_IOCSETLINKSTATE is not valid");
- ret = -EINVAL;
- break;
-
case HFI1_SNOOP_IOCSETLINKSTATE_EXTRA:
memset(&link_info, 0, sizeof(link_info));
--
1.8.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 5/7] staging/rdma/hfi1: Further clean up hfi1_ioctl parameter checks
[not found] ` <1447713160-24858-1-git-send-email-ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
` (3 preceding siblings ...)
2015-11-16 22:32 ` [PATCH v2 4/7] staging/rdma/hfi1: hfi1_ioctl remove setlink state ira.weiny-ral2JQCrhuEAvxtiuMwx3w
@ 2015-11-16 22:32 ` ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-16 22:32 ` [PATCH v2 6/7] staging/rdma/hfi1: Reduce snoop locking scope in IOCTL handler ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-16 22:32 ` [PATCH v2 7/7] staging/rdma/hfi1: Return immediately on error ira.weiny-ral2JQCrhuEAvxtiuMwx3w
6 siblings, 0 replies; 12+ messages in thread
From: ira.weiny-ral2JQCrhuEAvxtiuMwx3w @ 2015-11-16 22:32 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, Ira Weiny,
Dennis Dalessandro
From: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Final clean up of the if/then/else clause for the parameter checks of
hfi1_ioctl
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
Changes from v1:
SETLINKSTATE is removed
drivers/staging/rdma/hfi1/diag.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c
index 979890b40194..2bfb455d3ca4 100644
--- a/drivers/staging/rdma/hfi1/diag.c
+++ b/drivers/staging/rdma/hfi1/diag.c
@@ -974,24 +974,28 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
struct hfi1_pportdata *ppd = NULL;
unsigned int index;
struct hfi1_link_info link_info;
+ int read_cmd, write_cmd, read_ok, write_ok;
dd = hfi1_dd_from_sc_inode(fp->f_inode);
if (dd == NULL)
return -ENODEV;
mode_flag = dd->hfi1_snoop.mode_flag;
+ read_cmd = _IOC_DIR(cmd) & _IOC_READ;
+ write_cmd = _IOC_DIR(cmd) & _IOC_WRITE;
+ write_ok = access_ok(VERIFY_WRITE, (void __user *)arg, _IOC_SIZE(cmd));
+ read_ok = access_ok(VERIFY_READ, (void __user *)arg, _IOC_SIZE(cmd));
- if (((_IOC_DIR(cmd) & _IOC_READ)
- && !access_ok(VERIFY_WRITE, (void __user *)arg, _IOC_SIZE(cmd)))
- || ((_IOC_DIR(cmd) & _IOC_WRITE)
- && !access_ok(VERIFY_READ, (void __user *)arg, _IOC_SIZE(cmd)))) {
+ if ((read_cmd && !write_ok) || (write_cmd && !read_ok))
return -EFAULT;
- } else if (!capable(CAP_SYS_ADMIN)) {
+
+ if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- } else if ((mode_flag & HFI1_PORT_CAPTURE_MODE) &&
- (cmd != HFI1_SNOOP_IOCCLEARQUEUE) &&
- (cmd != HFI1_SNOOP_IOCCLEARFILTER) &&
- (cmd != HFI1_SNOOP_IOCSETFILTER)) {
+
+ if ((mode_flag & HFI1_PORT_CAPTURE_MODE) &&
+ (cmd != HFI1_SNOOP_IOCCLEARQUEUE) &&
+ (cmd != HFI1_SNOOP_IOCCLEARFILTER) &&
+ (cmd != HFI1_SNOOP_IOCSETFILTER))
/* Capture devices are allowed only 3 operations
* 1.Clear capture queue
* 2.Clear capture filter
@@ -999,7 +1003,6 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
* Other are invalid.
*/
return -EINVAL;
- }
spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
--
1.8.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 6/7] staging/rdma/hfi1: Reduce snoop locking scope in IOCTL handler.
[not found] ` <1447713160-24858-1-git-send-email-ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
` (4 preceding siblings ...)
2015-11-16 22:32 ` [PATCH v2 5/7] staging/rdma/hfi1: Further clean up hfi1_ioctl parameter checks ira.weiny-ral2JQCrhuEAvxtiuMwx3w
@ 2015-11-16 22:32 ` ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-16 22:32 ` [PATCH v2 7/7] staging/rdma/hfi1: Return immediately on error ira.weiny-ral2JQCrhuEAvxtiuMwx3w
6 siblings, 0 replies; 12+ messages in thread
From: ira.weiny-ral2JQCrhuEAvxtiuMwx3w @ 2015-11-16 22:32 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, Dennis Dalessandro,
Ira Weiny
From: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
This patch avoids issues while calling into copy from/to user while holding the
lock by only taking the lock when it is absolutely required.
The only commands which require the snoop lock are: *Set Filter *Clear Filter
*Clear Queue
Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
Changes from V1:
Adjust to add the done label which can't be removed until a later patch
Removed SETLINKSTATE
drivers/staging/rdma/hfi1/diag.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c
index 2bfb455d3ca4..a4dd4cbc2bea 100644
--- a/drivers/staging/rdma/hfi1/diag.c
+++ b/drivers/staging/rdma/hfi1/diag.c
@@ -1004,8 +1004,6 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
*/
return -EINVAL;
- spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
-
switch (cmd) {
case HFI1_SNOOP_IOCSETLINKSTATE_EXTRA:
memset(&link_info, 0, sizeof(link_info));
@@ -1122,11 +1120,14 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
case HFI1_SNOOP_IOCCLEARQUEUE:
snoop_dbg("Clearing snoop queue");
+ spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
drain_snoop_list(&dd->hfi1_snoop.queue);
+ spin_unlock_irqrestore(&dd->hfi1_snoop.snoop_lock, flags);
break;
case HFI1_SNOOP_IOCCLEARFILTER:
snoop_dbg("Clearing filter");
+ spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
if (dd->hfi1_snoop.filter_callback) {
/* Drain packets first */
drain_snoop_list(&dd->hfi1_snoop.queue);
@@ -1134,6 +1135,7 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
}
kfree(dd->hfi1_snoop.filter_value);
dd->hfi1_snoop.filter_value = NULL;
+ spin_unlock_irqrestore(&dd->hfi1_snoop.snoop_lock, flags);
break;
case HFI1_SNOOP_IOCSETFILTER:
@@ -1170,13 +1172,14 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
break;
}
/* Drain packets first */
+ spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
drain_snoop_list(&dd->hfi1_snoop.queue);
dd->hfi1_snoop.filter_callback =
hfi1_filters[filter_cmd.opcode].filter;
/* just in case we see back to back sets */
kfree(dd->hfi1_snoop.filter_value);
dd->hfi1_snoop.filter_value = filter_value;
-
+ spin_unlock_irqrestore(&dd->hfi1_snoop.snoop_lock, flags);
break;
case HFI1_SNOOP_IOCGETVERSION:
value = SNOOP_CAPTURE_VERSION;
@@ -1200,7 +1203,6 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
break;
}
done:
- spin_unlock_irqrestore(&dd->hfi1_snoop.snoop_lock, flags);
return ret;
}
--
1.8.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 7/7] staging/rdma/hfi1: Return immediately on error
[not found] ` <1447713160-24858-1-git-send-email-ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
` (5 preceding siblings ...)
2015-11-16 22:32 ` [PATCH v2 6/7] staging/rdma/hfi1: Reduce snoop locking scope in IOCTL handler ira.weiny-ral2JQCrhuEAvxtiuMwx3w
@ 2015-11-16 22:32 ` ira.weiny-ral2JQCrhuEAvxtiuMwx3w
6 siblings, 0 replies; 12+ messages in thread
From: ira.weiny-ral2JQCrhuEAvxtiuMwx3w @ 2015-11-16 22:32 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, Dennis Dalessandro,
Ira Weiny
From: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Now that the spinlock is not taken throughout hfi1_ioctl it is safe to return
early rather than setting a variable and falling through the switch.
Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
Changes from V1:
Squashed removal of goto which is safe after this patch
staging/rdma/hfi1: remove unneeded goto done
drivers/staging/rdma/hfi1/diag.c | 62 ++++++++++++++++------------------------
1 file changed, 24 insertions(+), 38 deletions(-)
diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c
index a4dd4cbc2bea..7cc862062c5e 100644
--- a/drivers/staging/rdma/hfi1/diag.c
+++ b/drivers/staging/rdma/hfi1/diag.c
@@ -1011,20 +1011,16 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
if (copy_from_user(&link_info,
(struct hfi1_link_info __user *)arg,
sizeof(link_info)))
- ret = -EFAULT;
+ return -EFAULT;
value = link_info.port_state;
index = link_info.port_number;
- if (index > dd->num_pports - 1) {
- ret = -EINVAL;
- break;
- }
+ if (index > dd->num_pports - 1)
+ return -EINVAL;
ppd = &dd->pport[index];
- if (!ppd) {
- ret = -EINVAL;
- break;
- }
+ if (!ppd)
+ return -EINVAL;
/* What we want to transition to */
phys_state = (value >> 4) & 0xF;
@@ -1048,8 +1044,7 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
dev_state = HLS_DN_DISABLE;
break;
default:
- ret = -EINVAL;
- goto done;
+ return -EINVAL;
}
ret = set_link_state(ppd, dev_state);
break;
@@ -1064,8 +1059,7 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
send_idle_sma(dd, SMA_IDLE_ACTIVE);
break;
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
if (ret)
@@ -1078,7 +1072,7 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
if (copy_from_user(&link_info,
(struct hfi1_link_info __user *)arg,
sizeof(link_info)))
- ret = -EFAULT;
+ return -EFAULT;
index = link_info.port_number;
} else {
ret = __get_user(index, (int __user *)arg);
@@ -1086,16 +1080,13 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
break;
}
- if (index > dd->num_pports - 1) {
- ret = -EINVAL;
- break;
- }
+ if (index > dd->num_pports - 1)
+ return -EINVAL;
ppd = &dd->pport[index];
- if (!ppd) {
- ret = -EINVAL;
- break;
- }
+ if (!ppd)
+ return -EINVAL;
+
value = hfi1_ibphys_portstate(ppd);
value <<= 4;
value |= driver_lstate(ppd);
@@ -1112,7 +1103,7 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
ppd->link_width_active;
if (copy_to_user((struct hfi1_link_info __user *)arg,
&link_info, sizeof(link_info)))
- ret = -EFAULT;
+ return -EFAULT;
} else {
ret = __put_user(value, (int __user *)arg);
}
@@ -1143,14 +1134,12 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
/* just copy command structure */
argp = (unsigned long *)arg;
if (copy_from_user(&filter_cmd, (void __user *)argp,
- sizeof(filter_cmd))) {
- ret = -EFAULT;
- break;
- }
+ sizeof(filter_cmd)))
+ return -EFAULT;
+
if (filter_cmd.opcode >= HFI1_MAX_FILTERS) {
pr_alert("Invalid opcode in request\n");
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
snoop_dbg("Opcode %d Len %d Ptr %p",
@@ -1159,17 +1148,15 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
filter_value = kcalloc(filter_cmd.length, sizeof(u8),
GFP_KERNEL);
- if (!filter_value) {
- ret = -ENOMEM;
- break;
- }
+ if (!filter_value)
+ return -ENOMEM;
+
/* copy remaining data from userspace */
if (copy_from_user((u8 *)filter_value,
(void __user *)filter_cmd.value_ptr,
filter_cmd.length)) {
kfree(filter_value);
- ret = -EFAULT;
- break;
+ return -EFAULT;
}
/* Drain packets first */
spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
@@ -1199,10 +1186,9 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
snoop_flags |= SNOOP_USE_METADATA;
break;
default:
- ret = -ENOTTY;
- break;
+ return -ENOTTY;
}
-done:
+
return ret;
}
--
1.8.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues
2015-11-16 22:32 ` [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues ira.weiny-ral2JQCrhuEAvxtiuMwx3w
@ 2015-11-17 16:47 ` Sudip Mukherjee
2015-11-17 20:30 ` ira.weiny
0 siblings, 1 reply; 12+ messages in thread
From: Sudip Mukherjee @ 2015-11-17 16:47 UTC (permalink / raw)
To: ira.weiny; +Cc: devel, gregkh, dledford, linux-rdma
On Mon, Nov 16, 2015 at 05:32:34PM -0500, ira.weiny@intel.com wrote:
> From: Jubin John <jubin.john@intel.com>
>
> Correct the checks on diag.c with the latest checkpatch
>
> Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
> Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
> Signed-off-by: Jubin John <jubin.john@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
Different types of changes in a single patch.
regards
sudip
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues
2015-11-17 16:47 ` Sudip Mukherjee
@ 2015-11-17 20:30 ` ira.weiny
[not found] ` <20151117203023.GA31037-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: ira.weiny @ 2015-11-17 20:30 UTC (permalink / raw)
To: Sudip Mukherjee, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dledford-H+wXaHxf7aLQT0dZR+AlfA
On Tue, Nov 17, 2015 at 10:17:26PM +0530, Sudip Mukherjee wrote:
> On Mon, Nov 16, 2015 at 05:32:34PM -0500, ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:
> > From: Jubin John <jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >
> > Correct the checks on diag.c with the latest checkpatch
> >
> > Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Jubin John <jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
>
> Different types of changes in a single patch.
Greg,
Do you want me to resubmit the entire series for this?
Ira
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues
[not found] ` <20151117203023.GA31037-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
@ 2015-11-17 22:27 ` gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
2015-11-23 2:17 ` ira.weiny
0 siblings, 1 reply; 12+ messages in thread
From: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r @ 2015-11-17 22:27 UTC (permalink / raw)
To: ira.weiny
Cc: Sudip Mukherjee, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dledford-H+wXaHxf7aLQT0dZR+AlfA
On Tue, Nov 17, 2015 at 03:30:24PM -0500, ira.weiny wrote:
> On Tue, Nov 17, 2015 at 10:17:26PM +0530, Sudip Mukherjee wrote:
> > On Mon, Nov 16, 2015 at 05:32:34PM -0500, ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:
> > > From: Jubin John <jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > >
> > > Correct the checks on diag.c with the latest checkpatch
> > >
> > > Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > Signed-off-by: Jubin John <jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > Signed-off-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > ---
> >
> > Different types of changes in a single patch.
>
> Greg,
>
> Do you want me to resubmit the entire series for this?
Why wouldn't you? :)
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues
2015-11-17 22:27 ` gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
@ 2015-11-23 2:17 ` ira.weiny
0 siblings, 0 replies; 12+ messages in thread
From: ira.weiny @ 2015-11-23 2:17 UTC (permalink / raw)
To: gregkh; +Cc: devel, linux-rdma, dledford, Sudip Mukherjee
On Tue, Nov 17, 2015 at 02:27:46PM -0800, gregkh@linuxfoundation.org wrote:
> On Tue, Nov 17, 2015 at 03:30:24PM -0500, ira.weiny wrote:
> > On Tue, Nov 17, 2015 at 10:17:26PM +0530, Sudip Mukherjee wrote:
> > > On Mon, Nov 16, 2015 at 05:32:34PM -0500, ira.weiny@intel.com wrote:
> > > > From: Jubin John <jubin.john@intel.com>
> > > >
> > > > Correct the checks on diag.c with the latest checkpatch
> > > >
> > > > Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
> > > > Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
> > > > Signed-off-by: Jubin John <jubin.john@intel.com>
> > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > > > ---
> > >
> > > Different types of changes in a single patch.
> >
> > Greg,
> >
> > Do you want me to resubmit the entire series for this?
>
> Why wouldn't you? :)
>
I meant instead of just replacing this single patch with a new "pre" series.
It is easier to just resend the entire series. I can do that once we settle
whos tree these patches are going to land in.
Ira
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-11-23 2:17 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-16 22:32 [PATCH v2 0/7] Fix hfi1_ioctl locking ira.weiny-ral2JQCrhuEAvxtiuMwx3w
[not found] ` <1447713160-24858-1-git-send-email-ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2015-11-16 22:32 ` [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-17 16:47 ` Sudip Mukherjee
2015-11-17 20:30 ` ira.weiny
[not found] ` <20151117203023.GA31037-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
2015-11-17 22:27 ` gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
2015-11-23 2:17 ` ira.weiny
2015-11-16 22:32 ` [PATCH v2 2/7] staging/rdma/hfi1: Fix camel case variables ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-16 22:32 ` [PATCH v2 3/7] staging/rdma/hfi1: Return early from hfi1_ioctl parameter errors ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-16 22:32 ` [PATCH v2 4/7] staging/rdma/hfi1: hfi1_ioctl remove setlink state ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-16 22:32 ` [PATCH v2 5/7] staging/rdma/hfi1: Further clean up hfi1_ioctl parameter checks ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-16 22:32 ` [PATCH v2 6/7] staging/rdma/hfi1: Reduce snoop locking scope in IOCTL handler ira.weiny-ral2JQCrhuEAvxtiuMwx3w
2015-11-16 22:32 ` [PATCH v2 7/7] staging/rdma/hfi1: Return immediately on error ira.weiny-ral2JQCrhuEAvxtiuMwx3w
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.