* [PATCH for-next 0/3] EFA statistics minor updates
@ 2020-04-20 6:22 Gal Pressman
2020-04-20 6:22 ` [PATCH for-next 1/3] RDMA/efa: Report create CQ error counter Gal Pressman
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Gal Pressman @ 2020-04-20 6:22 UTC (permalink / raw)
To: Jason Gunthorpe, Doug Ledford
Cc: linux-rdma, Alexander Matushevsky, Gal Pressman
This series contains three small patches to collect a few counters we
found helpful when encountering different issues.
Regards,
Gal
Gal Pressman (3):
RDMA/efa: Report create CQ error counter
RDMA/efa: Count mmap failures
RDMA/efa: Count admin commands errors
drivers/infiniband/hw/efa/efa.h | 3 ++-
drivers/infiniband/hw/efa/efa_com.c | 5 ++++-
drivers/infiniband/hw/efa/efa_com.h | 3 ++-
drivers/infiniband/hw/efa/efa_verbs.c | 13 +++++++++++--
4 files changed, 19 insertions(+), 5 deletions(-)
base-commit: 8f3d9f354286745c751374f5f1fcafee6b3f3136
--
2.26.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH for-next 1/3] RDMA/efa: Report create CQ error counter 2020-04-20 6:22 [PATCH for-next 0/3] EFA statistics minor updates Gal Pressman @ 2020-04-20 6:22 ` Gal Pressman 2020-04-20 6:22 ` [PATCH for-next 2/3] RDMA/efa: Count mmap failures Gal Pressman ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: Gal Pressman @ 2020-04-20 6:22 UTC (permalink / raw) To: Jason Gunthorpe, Doug Ledford Cc: linux-rdma, Alexander Matushevsky, Gal Pressman, Firas JahJah, Yossi Leybovich Create CQ errors are already being counted, report them along all other counters. Reviewed-by: Firas JahJah <firasj@amazon.com> Reviewed-by: Yossi Leybovich <sleybo@amazon.com> Signed-off-by: Gal Pressman <galpress@amazon.com> --- drivers/infiniband/hw/efa/efa_verbs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c index 5c57098a4aee..b555845d6c14 100644 --- a/drivers/infiniband/hw/efa/efa_verbs.c +++ b/drivers/infiniband/hw/efa/efa_verbs.c @@ -41,6 +41,7 @@ struct efa_user_mmap_entry { op(EFA_KEEP_ALIVE_RCVD, "keep_alive_rcvd") \ op(EFA_ALLOC_PD_ERR, "alloc_pd_err") \ op(EFA_CREATE_QP_ERR, "create_qp_err") \ + op(EFA_CREATE_CQ_ERR, "create_cq_err") \ op(EFA_REG_MR_ERR, "reg_mr_err") \ op(EFA_ALLOC_UCONTEXT_ERR, "alloc_ucontext_err") \ op(EFA_CREATE_AH_ERR, "create_ah_err") @@ -1753,6 +1754,7 @@ int efa_get_hw_stats(struct ib_device *ibdev, struct rdma_hw_stats *stats, stats->value[EFA_KEEP_ALIVE_RCVD] = atomic64_read(&s->keep_alive_rcvd); stats->value[EFA_ALLOC_PD_ERR] = atomic64_read(&s->sw_stats.alloc_pd_err); stats->value[EFA_CREATE_QP_ERR] = atomic64_read(&s->sw_stats.create_qp_err); + stats->value[EFA_CREATE_CQ_ERR] = atomic64_read(&s->sw_stats.create_cq_err); stats->value[EFA_REG_MR_ERR] = atomic64_read(&s->sw_stats.reg_mr_err); stats->value[EFA_ALLOC_UCONTEXT_ERR] = atomic64_read(&s->sw_stats.alloc_ucontext_err); stats->value[EFA_CREATE_AH_ERR] = atomic64_read(&s->sw_stats.create_ah_err); -- 2.26.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH for-next 2/3] RDMA/efa: Count mmap failures 2020-04-20 6:22 [PATCH for-next 0/3] EFA statistics minor updates Gal Pressman 2020-04-20 6:22 ` [PATCH for-next 1/3] RDMA/efa: Report create CQ error counter Gal Pressman @ 2020-04-20 6:22 ` Gal Pressman 2020-04-24 14:59 ` Jason Gunthorpe 2020-04-20 6:22 ` [PATCH for-next 3/3] RDMA/efa: Count admin commands errors Gal Pressman 2020-05-02 23:33 ` [PATCH for-next 0/3] EFA statistics minor updates Jason Gunthorpe 3 siblings, 1 reply; 11+ messages in thread From: Gal Pressman @ 2020-04-20 6:22 UTC (permalink / raw) To: Jason Gunthorpe, Doug Ledford Cc: linux-rdma, Alexander Matushevsky, Gal Pressman, Firas JahJah, Yossi Leybovich Add a new stat that counts mmap failures, which might help when debugging different issues. Reviewed-by: Firas JahJah <firasj@amazon.com> Reviewed-by: Yossi Leybovich <sleybo@amazon.com> Signed-off-by: Gal Pressman <galpress@amazon.com> --- drivers/infiniband/hw/efa/efa.h | 3 ++- drivers/infiniband/hw/efa/efa_verbs.c | 9 +++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/infiniband/hw/efa/efa.h b/drivers/infiniband/hw/efa/efa.h index aa7396a1588a..77c9ff798117 100644 --- a/drivers/infiniband/hw/efa/efa.h +++ b/drivers/infiniband/hw/efa/efa.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */ /* - * Copyright 2018-2019 Amazon.com, Inc. or its affiliates. All rights reserved. + * Copyright 2018-2020 Amazon.com, Inc. or its affiliates. All rights reserved. */ #ifndef _EFA_H_ @@ -40,6 +40,7 @@ struct efa_sw_stats { atomic64_t reg_mr_err; atomic64_t alloc_ucontext_err; atomic64_t create_ah_err; + atomic64_t mmap_err; }; /* Don't use anything other than atomic64 */ diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c index b555845d6c14..75eef1ec2474 100644 --- a/drivers/infiniband/hw/efa/efa_verbs.c +++ b/drivers/infiniband/hw/efa/efa_verbs.c @@ -44,7 +44,8 @@ struct efa_user_mmap_entry { op(EFA_CREATE_CQ_ERR, "create_cq_err") \ op(EFA_REG_MR_ERR, "reg_mr_err") \ op(EFA_ALLOC_UCONTEXT_ERR, "alloc_ucontext_err") \ - op(EFA_CREATE_AH_ERR, "create_ah_err") + op(EFA_CREATE_AH_ERR, "create_ah_err") \ + op(EFA_MMAP_ERR, "mmap_err") #define EFA_STATS_ENUM(ename, name) ename, #define EFA_STATS_STR(ename, name) [ename] = name, @@ -1569,6 +1570,7 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, ibdev_dbg(&dev->ibdev, "pgoff[%#lx] does not have valid entry\n", vma->vm_pgoff); + atomic64_inc(&dev->stats.sw_stats.mmap_err); return -EINVAL; } entry = to_emmap(rdma_entry); @@ -1604,12 +1606,14 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, err = -EINVAL; } - if (err) + if (err) { ibdev_dbg( &dev->ibdev, "Couldn't mmap address[%#llx] length[%#zx] mmap_flag[%d] err[%d]\n", entry->address, rdma_entry->npages * PAGE_SIZE, entry->mmap_flag, err); + atomic64_inc(&dev->stats.sw_stats.mmap_err); + } rdma_user_mmap_entry_put(rdma_entry); return err; @@ -1758,6 +1762,7 @@ int efa_get_hw_stats(struct ib_device *ibdev, struct rdma_hw_stats *stats, stats->value[EFA_REG_MR_ERR] = atomic64_read(&s->sw_stats.reg_mr_err); stats->value[EFA_ALLOC_UCONTEXT_ERR] = atomic64_read(&s->sw_stats.alloc_ucontext_err); stats->value[EFA_CREATE_AH_ERR] = atomic64_read(&s->sw_stats.create_ah_err); + stats->value[EFA_MMAP_ERR] = atomic64_read(&s->sw_stats.mmap_err); return ARRAY_SIZE(efa_stats_names); } -- 2.26.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH for-next 2/3] RDMA/efa: Count mmap failures 2020-04-20 6:22 ` [PATCH for-next 2/3] RDMA/efa: Count mmap failures Gal Pressman @ 2020-04-24 14:59 ` Jason Gunthorpe 2020-04-24 15:25 ` Gal Pressman 0 siblings, 1 reply; 11+ messages in thread From: Jason Gunthorpe @ 2020-04-24 14:59 UTC (permalink / raw) To: Gal Pressman Cc: Doug Ledford, linux-rdma, Alexander Matushevsky, Firas JahJah, Yossi Leybovich On Mon, Apr 20, 2020 at 09:22:12AM +0300, Gal Pressman wrote: > Add a new stat that counts mmap failures, which might help when > debugging different issues. > > Reviewed-by: Firas JahJah <firasj@amazon.com> > Reviewed-by: Yossi Leybovich <sleybo@amazon.com> > Signed-off-by: Gal Pressman <galpress@amazon.com> > drivers/infiniband/hw/efa/efa.h | 3 ++- > drivers/infiniband/hw/efa/efa_verbs.c | 9 +++++++-- > 2 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/infiniband/hw/efa/efa.h b/drivers/infiniband/hw/efa/efa.h > index aa7396a1588a..77c9ff798117 100644 > +++ b/drivers/infiniband/hw/efa/efa.h > @@ -1,6 +1,6 @@ > /* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */ > /* > - * Copyright 2018-2019 Amazon.com, Inc. or its affiliates. All rights reserved. > + * Copyright 2018-2020 Amazon.com, Inc. or its affiliates. All rights reserved. > */ > > #ifndef _EFA_H_ > @@ -40,6 +40,7 @@ struct efa_sw_stats { > atomic64_t reg_mr_err; > atomic64_t alloc_ucontext_err; > atomic64_t create_ah_err; > + atomic64_t mmap_err; > }; > > /* Don't use anything other than atomic64 */ > diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c > index b555845d6c14..75eef1ec2474 100644 > +++ b/drivers/infiniband/hw/efa/efa_verbs.c > @@ -44,7 +44,8 @@ struct efa_user_mmap_entry { > op(EFA_CREATE_CQ_ERR, "create_cq_err") \ > op(EFA_REG_MR_ERR, "reg_mr_err") \ > op(EFA_ALLOC_UCONTEXT_ERR, "alloc_ucontext_err") \ > - op(EFA_CREATE_AH_ERR, "create_ah_err") > + op(EFA_CREATE_AH_ERR, "create_ah_err") \ > + op(EFA_MMAP_ERR, "mmap_err") > > #define EFA_STATS_ENUM(ename, name) ename, > #define EFA_STATS_STR(ename, name) [ename] = name, > @@ -1569,6 +1570,7 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, > ibdev_dbg(&dev->ibdev, > "pgoff[%#lx] does not have valid entry\n", > vma->vm_pgoff); > + atomic64_inc(&dev->stats.sw_stats.mmap_err); > return -EINVAL; > } > entry = to_emmap(rdma_entry); > @@ -1604,12 +1606,14 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, > err = -EINVAL; > } > > - if (err) > + if (err) { > ibdev_dbg( > &dev->ibdev, > "Couldn't mmap address[%#llx] length[%#zx] mmap_flag[%d] err[%d]\n", > entry->address, rdma_entry->npages * PAGE_SIZE, > entry->mmap_flag, err); > + atomic64_inc(&dev->stats.sw_stats.mmap_err); Really? Isn't this something that is only possible with a buggy rdma-core provider? Why count it? Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-next 2/3] RDMA/efa: Count mmap failures 2020-04-24 14:59 ` Jason Gunthorpe @ 2020-04-24 15:25 ` Gal Pressman 2020-04-24 18:26 ` Jason Gunthorpe 0 siblings, 1 reply; 11+ messages in thread From: Gal Pressman @ 2020-04-24 15:25 UTC (permalink / raw) To: Jason Gunthorpe Cc: Doug Ledford, linux-rdma, Alexander Matushevsky, Firas JahJah, Yossi Leybovich On 24/04/2020 17:59, Jason Gunthorpe wrote: > On Mon, Apr 20, 2020 at 09:22:12AM +0300, Gal Pressman wrote: >> Add a new stat that counts mmap failures, which might help when >> debugging different issues. >> >> Reviewed-by: Firas JahJah <firasj@amazon.com> >> Reviewed-by: Yossi Leybovich <sleybo@amazon.com> >> Signed-off-by: Gal Pressman <galpress@amazon.com> >> drivers/infiniband/hw/efa/efa.h | 3 ++- >> drivers/infiniband/hw/efa/efa_verbs.c | 9 +++++++-- >> 2 files changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/infiniband/hw/efa/efa.h b/drivers/infiniband/hw/efa/efa.h >> index aa7396a1588a..77c9ff798117 100644 >> +++ b/drivers/infiniband/hw/efa/efa.h >> @@ -1,6 +1,6 @@ >> /* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */ >> /* >> - * Copyright 2018-2019 Amazon.com, Inc. or its affiliates. All rights reserved. >> + * Copyright 2018-2020 Amazon.com, Inc. or its affiliates. All rights reserved. >> */ >> >> #ifndef _EFA_H_ >> @@ -40,6 +40,7 @@ struct efa_sw_stats { >> atomic64_t reg_mr_err; >> atomic64_t alloc_ucontext_err; >> atomic64_t create_ah_err; >> + atomic64_t mmap_err; >> }; >> >> /* Don't use anything other than atomic64 */ >> diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c >> index b555845d6c14..75eef1ec2474 100644 >> +++ b/drivers/infiniband/hw/efa/efa_verbs.c >> @@ -44,7 +44,8 @@ struct efa_user_mmap_entry { >> op(EFA_CREATE_CQ_ERR, "create_cq_err") \ >> op(EFA_REG_MR_ERR, "reg_mr_err") \ >> op(EFA_ALLOC_UCONTEXT_ERR, "alloc_ucontext_err") \ >> - op(EFA_CREATE_AH_ERR, "create_ah_err") >> + op(EFA_CREATE_AH_ERR, "create_ah_err") \ >> + op(EFA_MMAP_ERR, "mmap_err") >> >> #define EFA_STATS_ENUM(ename, name) ename, >> #define EFA_STATS_STR(ename, name) [ename] = name, >> @@ -1569,6 +1570,7 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, >> ibdev_dbg(&dev->ibdev, >> "pgoff[%#lx] does not have valid entry\n", >> vma->vm_pgoff); >> + atomic64_inc(&dev->stats.sw_stats.mmap_err); >> return -EINVAL; >> } >> entry = to_emmap(rdma_entry); >> @@ -1604,12 +1606,14 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, >> err = -EINVAL; >> } >> >> - if (err) >> + if (err) { >> ibdev_dbg( >> &dev->ibdev, >> "Couldn't mmap address[%#llx] length[%#zx] mmap_flag[%d] err[%d]\n", >> entry->address, rdma_entry->npages * PAGE_SIZE, >> entry->mmap_flag, err); >> + atomic64_inc(&dev->stats.sw_stats.mmap_err); > > Really? Isn't this something that is only possible with a buggy > rdma-core provider? Why count it? Though unlikely, it could happen, otherwise this error flow wouldn't exist in the first place. If for some reason a customer app steps on a bug we're not aware of, this counter could serve as a red flag. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-next 2/3] RDMA/efa: Count mmap failures 2020-04-24 15:25 ` Gal Pressman @ 2020-04-24 18:26 ` Jason Gunthorpe 2020-04-26 6:42 ` Gal Pressman 0 siblings, 1 reply; 11+ messages in thread From: Jason Gunthorpe @ 2020-04-24 18:26 UTC (permalink / raw) To: Gal Pressman Cc: Doug Ledford, linux-rdma, Alexander Matushevsky, Firas JahJah, Yossi Leybovich On Fri, Apr 24, 2020 at 06:25:54PM +0300, Gal Pressman wrote: > On 24/04/2020 17:59, Jason Gunthorpe wrote: > > On Mon, Apr 20, 2020 at 09:22:12AM +0300, Gal Pressman wrote: > >> Add a new stat that counts mmap failures, which might help when > >> debugging different issues. > >> > >> Reviewed-by: Firas JahJah <firasj@amazon.com> > >> Reviewed-by: Yossi Leybovich <sleybo@amazon.com> > >> Signed-off-by: Gal Pressman <galpress@amazon.com> > >> drivers/infiniband/hw/efa/efa.h | 3 ++- > >> drivers/infiniband/hw/efa/efa_verbs.c | 9 +++++++-- > >> 2 files changed, 9 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/infiniband/hw/efa/efa.h b/drivers/infiniband/hw/efa/efa.h > >> index aa7396a1588a..77c9ff798117 100644 > >> +++ b/drivers/infiniband/hw/efa/efa.h > >> @@ -1,6 +1,6 @@ > >> /* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */ > >> /* > >> - * Copyright 2018-2019 Amazon.com, Inc. or its affiliates. All rights reserved. > >> + * Copyright 2018-2020 Amazon.com, Inc. or its affiliates. All rights reserved. > >> */ > >> > >> #ifndef _EFA_H_ > >> @@ -40,6 +40,7 @@ struct efa_sw_stats { > >> atomic64_t reg_mr_err; > >> atomic64_t alloc_ucontext_err; > >> atomic64_t create_ah_err; > >> + atomic64_t mmap_err; > >> }; > >> > >> /* Don't use anything other than atomic64 */ > >> diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c > >> index b555845d6c14..75eef1ec2474 100644 > >> +++ b/drivers/infiniband/hw/efa/efa_verbs.c > >> @@ -44,7 +44,8 @@ struct efa_user_mmap_entry { > >> op(EFA_CREATE_CQ_ERR, "create_cq_err") \ > >> op(EFA_REG_MR_ERR, "reg_mr_err") \ > >> op(EFA_ALLOC_UCONTEXT_ERR, "alloc_ucontext_err") \ > >> - op(EFA_CREATE_AH_ERR, "create_ah_err") > >> + op(EFA_CREATE_AH_ERR, "create_ah_err") \ > >> + op(EFA_MMAP_ERR, "mmap_err") > >> > >> #define EFA_STATS_ENUM(ename, name) ename, > >> #define EFA_STATS_STR(ename, name) [ename] = name, > >> @@ -1569,6 +1570,7 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, > >> ibdev_dbg(&dev->ibdev, > >> "pgoff[%#lx] does not have valid entry\n", > >> vma->vm_pgoff); > >> + atomic64_inc(&dev->stats.sw_stats.mmap_err); > >> return -EINVAL; > >> } > >> entry = to_emmap(rdma_entry); > >> @@ -1604,12 +1606,14 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, > >> err = -EINVAL; > >> } > >> > >> - if (err) > >> + if (err) { > >> ibdev_dbg( > >> &dev->ibdev, > >> "Couldn't mmap address[%#llx] length[%#zx] mmap_flag[%d] err[%d]\n", > >> entry->address, rdma_entry->npages * PAGE_SIZE, > >> entry->mmap_flag, err); > >> + atomic64_inc(&dev->stats.sw_stats.mmap_err); > > > > Really? Isn't this something that is only possible with a buggy > > rdma-core provider? Why count it? > > Though unlikely, it could happen, otherwise this error flow wouldn't exist in > the first place. > > If for some reason a customer app steps on a bug we're not aware of, this > counter could serve as a red flag. But there are lots of cases where a buggy provider can cause error exits, why choose this one to count against all the others? Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-next 2/3] RDMA/efa: Count mmap failures 2020-04-24 18:26 ` Jason Gunthorpe @ 2020-04-26 6:42 ` Gal Pressman 2020-04-26 13:30 ` Jason Gunthorpe 0 siblings, 1 reply; 11+ messages in thread From: Gal Pressman @ 2020-04-26 6:42 UTC (permalink / raw) To: Jason Gunthorpe Cc: Doug Ledford, linux-rdma, Alexander Matushevsky, Firas JahJah, Yossi Leybovich On 24/04/2020 21:26, Jason Gunthorpe wrote: > On Fri, Apr 24, 2020 at 06:25:54PM +0300, Gal Pressman wrote: >> On 24/04/2020 17:59, Jason Gunthorpe wrote: >>> On Mon, Apr 20, 2020 at 09:22:12AM +0300, Gal Pressman wrote: >>>> Add a new stat that counts mmap failures, which might help when >>>> debugging different issues. >>>> >>>> Reviewed-by: Firas JahJah <firasj@amazon.com> >>>> Reviewed-by: Yossi Leybovich <sleybo@amazon.com> >>>> Signed-off-by: Gal Pressman <galpress@amazon.com> >>>> drivers/infiniband/hw/efa/efa.h | 3 ++- >>>> drivers/infiniband/hw/efa/efa_verbs.c | 9 +++++++-- >>>> 2 files changed, 9 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/infiniband/hw/efa/efa.h b/drivers/infiniband/hw/efa/efa.h >>>> index aa7396a1588a..77c9ff798117 100644 >>>> +++ b/drivers/infiniband/hw/efa/efa.h >>>> @@ -1,6 +1,6 @@ >>>> /* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */ >>>> /* >>>> - * Copyright 2018-2019 Amazon.com, Inc. or its affiliates. All rights reserved. >>>> + * Copyright 2018-2020 Amazon.com, Inc. or its affiliates. All rights reserved. >>>> */ >>>> >>>> #ifndef _EFA_H_ >>>> @@ -40,6 +40,7 @@ struct efa_sw_stats { >>>> atomic64_t reg_mr_err; >>>> atomic64_t alloc_ucontext_err; >>>> atomic64_t create_ah_err; >>>> + atomic64_t mmap_err; >>>> }; >>>> >>>> /* Don't use anything other than atomic64 */ >>>> diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c >>>> index b555845d6c14..75eef1ec2474 100644 >>>> +++ b/drivers/infiniband/hw/efa/efa_verbs.c >>>> @@ -44,7 +44,8 @@ struct efa_user_mmap_entry { >>>> op(EFA_CREATE_CQ_ERR, "create_cq_err") \ >>>> op(EFA_REG_MR_ERR, "reg_mr_err") \ >>>> op(EFA_ALLOC_UCONTEXT_ERR, "alloc_ucontext_err") \ >>>> - op(EFA_CREATE_AH_ERR, "create_ah_err") >>>> + op(EFA_CREATE_AH_ERR, "create_ah_err") \ >>>> + op(EFA_MMAP_ERR, "mmap_err") >>>> >>>> #define EFA_STATS_ENUM(ename, name) ename, >>>> #define EFA_STATS_STR(ename, name) [ename] = name, >>>> @@ -1569,6 +1570,7 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, >>>> ibdev_dbg(&dev->ibdev, >>>> "pgoff[%#lx] does not have valid entry\n", >>>> vma->vm_pgoff); >>>> + atomic64_inc(&dev->stats.sw_stats.mmap_err); >>>> return -EINVAL; >>>> } >>>> entry = to_emmap(rdma_entry); >>>> @@ -1604,12 +1606,14 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, >>>> err = -EINVAL; >>>> } >>>> >>>> - if (err) >>>> + if (err) { >>>> ibdev_dbg( >>>> &dev->ibdev, >>>> "Couldn't mmap address[%#llx] length[%#zx] mmap_flag[%d] err[%d]\n", >>>> entry->address, rdma_entry->npages * PAGE_SIZE, >>>> entry->mmap_flag, err); >>>> + atomic64_inc(&dev->stats.sw_stats.mmap_err); >>> >>> Really? Isn't this something that is only possible with a buggy >>> rdma-core provider? Why count it? >> >> Though unlikely, it could happen, otherwise this error flow wouldn't exist in >> the first place. >> >> If for some reason a customer app steps on a bug we're not aware of, this >> counter could serve as a red flag. > > But there are lots of cases where a buggy provider can cause error > exits, why choose this one to count against all the others? It's not one against all others, most if not all of our userspace facing API error flows have a similar counter. And TBH, I think that the mmap flow is quite convoluted with the cookie response from the crate verb, so it deserves a counter IMO. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-next 2/3] RDMA/efa: Count mmap failures 2020-04-26 6:42 ` Gal Pressman @ 2020-04-26 13:30 ` Jason Gunthorpe 2020-04-26 14:17 ` Gal Pressman 0 siblings, 1 reply; 11+ messages in thread From: Jason Gunthorpe @ 2020-04-26 13:30 UTC (permalink / raw) To: Gal Pressman Cc: Doug Ledford, linux-rdma, Alexander Matushevsky, Firas JahJah, Yossi Leybovich On Sun, Apr 26, 2020 at 09:42:27AM +0300, Gal Pressman wrote: > On 24/04/2020 21:26, Jason Gunthorpe wrote: > > On Fri, Apr 24, 2020 at 06:25:54PM +0300, Gal Pressman wrote: > >> On 24/04/2020 17:59, Jason Gunthorpe wrote: > >>> On Mon, Apr 20, 2020 at 09:22:12AM +0300, Gal Pressman wrote: > >>>> Add a new stat that counts mmap failures, which might help when > >>>> debugging different issues. > >>>> > >>>> Reviewed-by: Firas JahJah <firasj@amazon.com> > >>>> Reviewed-by: Yossi Leybovich <sleybo@amazon.com> > >>>> Signed-off-by: Gal Pressman <galpress@amazon.com> > >>>> drivers/infiniband/hw/efa/efa.h | 3 ++- > >>>> drivers/infiniband/hw/efa/efa_verbs.c | 9 +++++++-- > >>>> 2 files changed, 9 insertions(+), 3 deletions(-) > >>>> > >>>> diff --git a/drivers/infiniband/hw/efa/efa.h b/drivers/infiniband/hw/efa/efa.h > >>>> index aa7396a1588a..77c9ff798117 100644 > >>>> +++ b/drivers/infiniband/hw/efa/efa.h > >>>> @@ -1,6 +1,6 @@ > >>>> /* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */ > >>>> /* > >>>> - * Copyright 2018-2019 Amazon.com, Inc. or its affiliates. All rights reserved. > >>>> + * Copyright 2018-2020 Amazon.com, Inc. or its affiliates. All rights reserved. > >>>> */ > >>>> > >>>> #ifndef _EFA_H_ > >>>> @@ -40,6 +40,7 @@ struct efa_sw_stats { > >>>> atomic64_t reg_mr_err; > >>>> atomic64_t alloc_ucontext_err; > >>>> atomic64_t create_ah_err; > >>>> + atomic64_t mmap_err; > >>>> }; > >>>> > >>>> /* Don't use anything other than atomic64 */ > >>>> diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c > >>>> index b555845d6c14..75eef1ec2474 100644 > >>>> +++ b/drivers/infiniband/hw/efa/efa_verbs.c > >>>> @@ -44,7 +44,8 @@ struct efa_user_mmap_entry { > >>>> op(EFA_CREATE_CQ_ERR, "create_cq_err") \ > >>>> op(EFA_REG_MR_ERR, "reg_mr_err") \ > >>>> op(EFA_ALLOC_UCONTEXT_ERR, "alloc_ucontext_err") \ > >>>> - op(EFA_CREATE_AH_ERR, "create_ah_err") > >>>> + op(EFA_CREATE_AH_ERR, "create_ah_err") \ > >>>> + op(EFA_MMAP_ERR, "mmap_err") > >>>> > >>>> #define EFA_STATS_ENUM(ename, name) ename, > >>>> #define EFA_STATS_STR(ename, name) [ename] = name, > >>>> @@ -1569,6 +1570,7 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, > >>>> ibdev_dbg(&dev->ibdev, > >>>> "pgoff[%#lx] does not have valid entry\n", > >>>> vma->vm_pgoff); > >>>> + atomic64_inc(&dev->stats.sw_stats.mmap_err); > >>>> return -EINVAL; > >>>> } > >>>> entry = to_emmap(rdma_entry); > >>>> @@ -1604,12 +1606,14 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, > >>>> err = -EINVAL; > >>>> } > >>>> > >>>> - if (err) > >>>> + if (err) { > >>>> ibdev_dbg( > >>>> &dev->ibdev, > >>>> "Couldn't mmap address[%#llx] length[%#zx] mmap_flag[%d] err[%d]\n", > >>>> entry->address, rdma_entry->npages * PAGE_SIZE, > >>>> entry->mmap_flag, err); > >>>> + atomic64_inc(&dev->stats.sw_stats.mmap_err); > >>> > >>> Really? Isn't this something that is only possible with a buggy > >>> rdma-core provider? Why count it? > >> > >> Though unlikely, it could happen, otherwise this error flow wouldn't exist in > >> the first place. > >> > >> If for some reason a customer app steps on a bug we're not aware of, this > >> counter could serve as a red flag. > > > > But there are lots of cases where a buggy provider can cause error > > exits, why choose this one to count against all the others? > > It's not one against all others, most if not all of our userspace facing API > error flows have a similar counter. Hurm, seems a bit strange, but OK > And TBH, I think that the mmap flow is quite convoluted with the cookie response > from the crate verb, so it deserves a counter IMO. How so? Userspace takes the u64 from the command and pass it to mmap, what is convoluted? Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-next 2/3] RDMA/efa: Count mmap failures 2020-04-26 13:30 ` Jason Gunthorpe @ 2020-04-26 14:17 ` Gal Pressman 0 siblings, 0 replies; 11+ messages in thread From: Gal Pressman @ 2020-04-26 14:17 UTC (permalink / raw) To: Jason Gunthorpe Cc: Doug Ledford, linux-rdma, Alexander Matushevsky, Firas JahJah, Yossi Leybovich On 26/04/2020 16:30, Jason Gunthorpe wrote: > On Sun, Apr 26, 2020 at 09:42:27AM +0300, Gal Pressman wrote: >> On 24/04/2020 21:26, Jason Gunthorpe wrote: >>> On Fri, Apr 24, 2020 at 06:25:54PM +0300, Gal Pressman wrote: >>>> On 24/04/2020 17:59, Jason Gunthorpe wrote: >>>>> On Mon, Apr 20, 2020 at 09:22:12AM +0300, Gal Pressman wrote: >>>>>> Add a new stat that counts mmap failures, which might help when >>>>>> debugging different issues. >>>>>> >>>>>> Reviewed-by: Firas JahJah <firasj@amazon.com> >>>>>> Reviewed-by: Yossi Leybovich <sleybo@amazon.com> >>>>>> Signed-off-by: Gal Pressman <galpress@amazon.com> >>>>>> drivers/infiniband/hw/efa/efa.h | 3 ++- >>>>>> drivers/infiniband/hw/efa/efa_verbs.c | 9 +++++++-- >>>>>> 2 files changed, 9 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/drivers/infiniband/hw/efa/efa.h b/drivers/infiniband/hw/efa/efa.h >>>>>> index aa7396a1588a..77c9ff798117 100644 >>>>>> +++ b/drivers/infiniband/hw/efa/efa.h >>>>>> @@ -1,6 +1,6 @@ >>>>>> /* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */ >>>>>> /* >>>>>> - * Copyright 2018-2019 Amazon.com, Inc. or its affiliates. All rights reserved. >>>>>> + * Copyright 2018-2020 Amazon.com, Inc. or its affiliates. All rights reserved. >>>>>> */ >>>>>> >>>>>> #ifndef _EFA_H_ >>>>>> @@ -40,6 +40,7 @@ struct efa_sw_stats { >>>>>> atomic64_t reg_mr_err; >>>>>> atomic64_t alloc_ucontext_err; >>>>>> atomic64_t create_ah_err; >>>>>> + atomic64_t mmap_err; >>>>>> }; >>>>>> >>>>>> /* Don't use anything other than atomic64 */ >>>>>> diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c >>>>>> index b555845d6c14..75eef1ec2474 100644 >>>>>> +++ b/drivers/infiniband/hw/efa/efa_verbs.c >>>>>> @@ -44,7 +44,8 @@ struct efa_user_mmap_entry { >>>>>> op(EFA_CREATE_CQ_ERR, "create_cq_err") \ >>>>>> op(EFA_REG_MR_ERR, "reg_mr_err") \ >>>>>> op(EFA_ALLOC_UCONTEXT_ERR, "alloc_ucontext_err") \ >>>>>> - op(EFA_CREATE_AH_ERR, "create_ah_err") >>>>>> + op(EFA_CREATE_AH_ERR, "create_ah_err") \ >>>>>> + op(EFA_MMAP_ERR, "mmap_err") >>>>>> >>>>>> #define EFA_STATS_ENUM(ename, name) ename, >>>>>> #define EFA_STATS_STR(ename, name) [ename] = name, >>>>>> @@ -1569,6 +1570,7 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, >>>>>> ibdev_dbg(&dev->ibdev, >>>>>> "pgoff[%#lx] does not have valid entry\n", >>>>>> vma->vm_pgoff); >>>>>> + atomic64_inc(&dev->stats.sw_stats.mmap_err); >>>>>> return -EINVAL; >>>>>> } >>>>>> entry = to_emmap(rdma_entry); >>>>>> @@ -1604,12 +1606,14 @@ static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext, >>>>>> err = -EINVAL; >>>>>> } >>>>>> >>>>>> - if (err) >>>>>> + if (err) { >>>>>> ibdev_dbg( >>>>>> &dev->ibdev, >>>>>> "Couldn't mmap address[%#llx] length[%#zx] mmap_flag[%d] err[%d]\n", >>>>>> entry->address, rdma_entry->npages * PAGE_SIZE, >>>>>> entry->mmap_flag, err); >>>>>> + atomic64_inc(&dev->stats.sw_stats.mmap_err); >>>>> >>>>> Really? Isn't this something that is only possible with a buggy >>>>> rdma-core provider? Why count it? >>>> >>>> Though unlikely, it could happen, otherwise this error flow wouldn't exist in >>>> the first place. >>>> >>>> If for some reason a customer app steps on a bug we're not aware of, this >>>> counter could serve as a red flag. >>> >>> But there are lots of cases where a buggy provider can cause error >>> exits, why choose this one to count against all the others? >> >> It's not one against all others, most if not all of our userspace facing API >> error flows have a similar counter. > > Hurm, seems a bit strange, but OK > >> And TBH, I think that the mmap flow is quite convoluted with the cookie response >> from the crate verb, so it deserves a counter IMO. > > How so? Userspace takes the u64 from the command and pass it to mmap, > what is convoluted? It's the only flow that involves two phases/userspace calls and not a single command-response call, so it's a bit more error prone. Convoluted was a bit harsh :). ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH for-next 3/3] RDMA/efa: Count admin commands errors 2020-04-20 6:22 [PATCH for-next 0/3] EFA statistics minor updates Gal Pressman 2020-04-20 6:22 ` [PATCH for-next 1/3] RDMA/efa: Report create CQ error counter Gal Pressman 2020-04-20 6:22 ` [PATCH for-next 2/3] RDMA/efa: Count mmap failures Gal Pressman @ 2020-04-20 6:22 ` Gal Pressman 2020-05-02 23:33 ` [PATCH for-next 0/3] EFA statistics minor updates Jason Gunthorpe 3 siblings, 0 replies; 11+ messages in thread From: Gal Pressman @ 2020-04-20 6:22 UTC (permalink / raw) To: Jason Gunthorpe, Doug Ledford Cc: linux-rdma, Alexander Matushevsky, Gal Pressman, Daniel Kranzdorf, Yossi Leybovich Add a new stat that counts admin commands failures, which might help when debugging different issues. Reviewed-by: Daniel Kranzdorf <dkkranzd@amazon.com> Reviewed-by: Yossi Leybovich <sleybo@amazon.com> Signed-off-by: Gal Pressman <galpress@amazon.com> --- drivers/infiniband/hw/efa/efa_com.c | 5 ++++- drivers/infiniband/hw/efa/efa_com.h | 3 ++- drivers/infiniband/hw/efa/efa_verbs.c | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/hw/efa/efa_com.c b/drivers/infiniband/hw/efa/efa_com.c index 7fce69f5568f..336bc2c57bb1 100644 --- a/drivers/infiniband/hw/efa/efa_com.c +++ b/drivers/infiniband/hw/efa/efa_com.c @@ -631,17 +631,20 @@ int efa_com_cmd_exec(struct efa_com_admin_queue *aq, cmd->aq_common_descriptor.opcode, PTR_ERR(comp_ctx)); up(&aq->avail_cmds); + atomic64_inc(&aq->stats.cmd_err); return PTR_ERR(comp_ctx); } err = efa_com_wait_and_process_admin_cq(comp_ctx, aq); - if (err) + if (err) { ibdev_err_ratelimited( aq->efa_dev, "Failed to process command %s (opcode %u) comp_status %d err %d\n", efa_com_cmd_str(cmd->aq_common_descriptor.opcode), cmd->aq_common_descriptor.opcode, comp_ctx->comp_status, err); + atomic64_inc(&aq->stats.cmd_err); + } up(&aq->avail_cmds); diff --git a/drivers/infiniband/hw/efa/efa_com.h b/drivers/infiniband/hw/efa/efa_com.h index c67dd8109d1c..5e4c88877ddb 100644 --- a/drivers/infiniband/hw/efa/efa_com.h +++ b/drivers/infiniband/hw/efa/efa_com.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */ /* - * Copyright 2018-2019 Amazon.com, Inc. or its affiliates. All rights reserved. + * Copyright 2018-2020 Amazon.com, Inc. or its affiliates. All rights reserved. */ #ifndef _EFA_COM_H_ @@ -47,6 +47,7 @@ struct efa_com_admin_sq { struct efa_com_stats_admin { atomic64_t submitted_cmd; atomic64_t completed_cmd; + atomic64_t cmd_err; atomic64_t no_completion; }; diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c index 75eef1ec2474..ce7a3ead1ba7 100644 --- a/drivers/infiniband/hw/efa/efa_verbs.c +++ b/drivers/infiniband/hw/efa/efa_verbs.c @@ -37,6 +37,7 @@ struct efa_user_mmap_entry { op(EFA_RX_DROPS, "rx_drops") \ op(EFA_SUBMITTED_CMDS, "submitted_cmds") \ op(EFA_COMPLETED_CMDS, "completed_cmds") \ + op(EFA_CMDS_ERR, "cmds_err") \ op(EFA_NO_COMPLETION_CMDS, "no_completion_cmds") \ op(EFA_KEEP_ALIVE_RCVD, "keep_alive_rcvd") \ op(EFA_ALLOC_PD_ERR, "alloc_pd_err") \ @@ -1752,6 +1753,7 @@ int efa_get_hw_stats(struct ib_device *ibdev, struct rdma_hw_stats *stats, as = &dev->edev.aq.stats; stats->value[EFA_SUBMITTED_CMDS] = atomic64_read(&as->submitted_cmd); stats->value[EFA_COMPLETED_CMDS] = atomic64_read(&as->completed_cmd); + stats->value[EFA_CMDS_ERR] = atomic64_read(&as->cmd_err); stats->value[EFA_NO_COMPLETION_CMDS] = atomic64_read(&as->no_completion); s = &dev->stats; -- 2.26.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH for-next 0/3] EFA statistics minor updates 2020-04-20 6:22 [PATCH for-next 0/3] EFA statistics minor updates Gal Pressman ` (2 preceding siblings ...) 2020-04-20 6:22 ` [PATCH for-next 3/3] RDMA/efa: Count admin commands errors Gal Pressman @ 2020-05-02 23:33 ` Jason Gunthorpe 3 siblings, 0 replies; 11+ messages in thread From: Jason Gunthorpe @ 2020-05-02 23:33 UTC (permalink / raw) To: Gal Pressman; +Cc: Doug Ledford, linux-rdma, Alexander Matushevsky On Mon, Apr 20, 2020 at 09:22:10AM +0300, Gal Pressman wrote: > This series contains three small patches to collect a few counters we > found helpful when encountering different issues. Applied to for-next, thanks Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-05-02 23:33 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-20 6:22 [PATCH for-next 0/3] EFA statistics minor updates Gal Pressman 2020-04-20 6:22 ` [PATCH for-next 1/3] RDMA/efa: Report create CQ error counter Gal Pressman 2020-04-20 6:22 ` [PATCH for-next 2/3] RDMA/efa: Count mmap failures Gal Pressman 2020-04-24 14:59 ` Jason Gunthorpe 2020-04-24 15:25 ` Gal Pressman 2020-04-24 18:26 ` Jason Gunthorpe 2020-04-26 6:42 ` Gal Pressman 2020-04-26 13:30 ` Jason Gunthorpe 2020-04-26 14:17 ` Gal Pressman 2020-04-20 6:22 ` [PATCH for-next 3/3] RDMA/efa: Count admin commands errors Gal Pressman 2020-05-02 23:33 ` [PATCH for-next 0/3] EFA statistics minor updates Jason Gunthorpe
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.