| Message ID | 20190608092714.GE28890@mwanda (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | b417c0879db72f810ca81d88b719e70d20566857 |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | RDMA/hns: Fix an error code in hns_roce_set_user_sq_size() | expand |
On Sat, Jun 08, 2019 at 12:27:14PM +0300, Dan Carpenter wrote: > This function is supposed to return negative kernel error codes but here > it returns CMD_RST_PRC_EBUSY (2). The error code eventually gets passed > to IS_ERR() and since it's not an error pointer it leads to an Oops in > hns_roce_v1_rsv_lp_qp() > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > Static analysis. Not tested. > > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > index ac017c24b200..018ff302ab9e 100644 > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > @@ -1098,7 +1098,7 @@ static int hns_roce_cmq_send(struct hns_roce_dev *hr_dev, > if (ret == CMD_RST_PRC_SUCCESS) > return 0; > if (ret == CMD_RST_PRC_EBUSY) The better fix will be to remove CMD_RST_PRC_* definitions in favor of normal errno. Thanks
On Wed, Jun 12, 2019 at 08:23:16PM +0300, Leon Romanovsky wrote: > On Sat, Jun 08, 2019 at 12:27:14PM +0300, Dan Carpenter wrote: > > This function is supposed to return negative kernel error codes but here > > it returns CMD_RST_PRC_EBUSY (2). The error code eventually gets passed > > to IS_ERR() and since it's not an error pointer it leads to an Oops in > > hns_roce_v1_rsv_lp_qp() > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > Static analysis. Not tested. > > > > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > index ac017c24b200..018ff302ab9e 100644 > > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > @@ -1098,7 +1098,7 @@ static int hns_roce_cmq_send(struct hns_roce_dev *hr_dev, > > if (ret == CMD_RST_PRC_SUCCESS) > > return 0; > > if (ret == CMD_RST_PRC_EBUSY) > > The better fix will be to remove CMD_RST_PRC_* definitions in favor of > normal errno. > Yes. I've looked at that idea and I would almost feel like it's easy enough to send a patch like that without testing it at all. But it would be better if the people with the hardware sent it. I reported this bug months ago... regards, dan carpenter
在 2019/6/13 1:23, Leon Romanovsky 写道: > On Sat, Jun 08, 2019 at 12:27:14PM +0300, Dan Carpenter wrote: >> This function is supposed to return negative kernel error codes but here >> it returns CMD_RST_PRC_EBUSY (2). The error code eventually gets passed >> to IS_ERR() and since it's not an error pointer it leads to an Oops in >> hns_roce_v1_rsv_lp_qp() >> >> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >> --- >> Static analysis. Not tested. >> >> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >> index ac017c24b200..018ff302ab9e 100644 >> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >> @@ -1098,7 +1098,7 @@ static int hns_roce_cmq_send(struct hns_roce_dev *hr_dev, >> if (ret == CMD_RST_PRC_SUCCESS) >> return 0; >> if (ret == CMD_RST_PRC_EBUSY) > The better fix will be to remove CMD_RST_PRC_* definitions in favor of > normal errno. > > Thanks > > . > Sorry, Our guys are analyzing his influence. So the response is a bit late.
On Wed, Jun 12, 2019 at 11:05:17PM -0700, Dan Carpenter wrote: > On Wed, Jun 12, 2019 at 08:23:16PM +0300, Leon Romanovsky wrote: > > On Sat, Jun 08, 2019 at 12:27:14PM +0300, Dan Carpenter wrote: > > > This function is supposed to return negative kernel error codes but here > > > it returns CMD_RST_PRC_EBUSY (2). The error code eventually gets passed > > > to IS_ERR() and since it's not an error pointer it leads to an Oops in > > > hns_roce_v1_rsv_lp_qp() > > > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > --- > > > Static analysis. Not tested. > > > > > > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > > index ac017c24b200..018ff302ab9e 100644 > > > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > > @@ -1098,7 +1098,7 @@ static int hns_roce_cmq_send(struct hns_roce_dev *hr_dev, > > > if (ret == CMD_RST_PRC_SUCCESS) > > > return 0; > > > if (ret == CMD_RST_PRC_EBUSY) > > > > The better fix will be to remove CMD_RST_PRC_* definitions in favor of > > normal errno. > > > > Yes. > > I've looked at that idea and I would almost feel like it's easy enough > to send a patch like that without testing it at all. But it would be > better if the people with the hardware sent it. I reported this bug > months ago... Feel free to send, we will give time to respond. thanks > > regards, > dan carpenter
在 2019/6/13 14:05, Dan Carpenter 写道: > On Wed, Jun 12, 2019 at 08:23:16PM +0300, Leon Romanovsky wrote: >> On Sat, Jun 08, 2019 at 12:27:14PM +0300, Dan Carpenter wrote: >>> This function is supposed to return negative kernel error codes but here >>> it returns CMD_RST_PRC_EBUSY (2). The error code eventually gets passed >>> to IS_ERR() and since it's not an error pointer it leads to an Oops in >>> hns_roce_v1_rsv_lp_qp() >>> >>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >>> --- >>> Static analysis. Not tested. >>> >>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >>> index ac017c24b200..018ff302ab9e 100644 >>> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >>> @@ -1098,7 +1098,7 @@ static int hns_roce_cmq_send(struct hns_roce_dev *hr_dev, >>> if (ret == CMD_RST_PRC_SUCCESS) >>> return 0; >>> if (ret == CMD_RST_PRC_EBUSY) >> The better fix will be to remove CMD_RST_PRC_* definitions in favor of >> normal errno. >> > Yes. > > I've looked at that idea and I would almost feel like it's easy enough > to send a patch like that without testing it at all. But it would be > better if the people with the hardware sent it. I reported this bug > months ago... > > regards, > dan carpenter > > . Hi, dan carpenter Sorry to reply slowly. I have noticed before months ago when I see your email. But we are sorting through our entire process and haven't had time to deal with it yet. We are agree with your modifications after analysis. For leon's advice, We are considering. If consider to use normal errno, it is not easy to review the entire reset for others. We will give a patch for testing and send it. Thanks. Lijun Ou
On Sat, Jun 08, 2019 at 12:27:14PM +0300, Dan Carpenter wrote: > This function is supposed to return negative kernel error codes but here > it returns CMD_RST_PRC_EBUSY (2). The error code eventually gets passed > to IS_ERR() and since it's not an error pointer it leads to an Oops in > hns_roce_v1_rsv_lp_qp() > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > Static analysis. Not tested. Applied to for-next, thanks Jason
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c index ac017c24b200..018ff302ab9e 100644 --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c @@ -1098,7 +1098,7 @@ static int hns_roce_cmq_send(struct hns_roce_dev *hr_dev, if (ret == CMD_RST_PRC_SUCCESS) return 0; if (ret == CMD_RST_PRC_EBUSY) - return ret; + return -EBUSY; ret = __hns_roce_cmq_send(hr_dev, desc, num); if (ret) { @@ -1106,7 +1106,7 @@ static int hns_roce_cmq_send(struct hns_roce_dev *hr_dev, if (retval == CMD_RST_PRC_SUCCESS) return 0; else if (retval == CMD_RST_PRC_EBUSY) - return retval; + return -EBUSY; } return ret;
This function is supposed to return negative kernel error codes but here it returns CMD_RST_PRC_EBUSY (2). The error code eventually gets passed to IS_ERR() and since it's not an error pointer it leads to an Oops in hns_roce_v1_rsv_lp_qp() Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- Static analysis. Not tested. drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)