> -----Original Message-----
> From: Leon Romanovsky <leon@xxxxxxxxxx>
> Sent: Friday, August 30, 2019 4:43 PM
> To: Parav Pandit <parav@xxxxxxxxxxxx>
> Cc: Doug Ledford <dledford@xxxxxxxxxx>; Jason Gunthorpe
> <jgg@xxxxxxxxxxxx>; RDMA mailing list <linux-rdma@xxxxxxxxxxxxxxx>; Erez
> Alfasi <ereza@xxxxxxxxxxxx>
> Subject: Re: [PATCH rdma-next v1 3/4] RDMA/nldev: Provide MR statistics
>
> On Fri, Aug 30, 2019 at 10:18:35AM +0000, Parav Pandit wrote:
> >
> >
> > > -----Original Message-----
> > > From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma-
> > > owner@xxxxxxxxxxxxxxx> On Behalf Of Leon Romanovsky
> > > Sent: Friday, August 30, 2019 1:46 PM
> > > To: Doug Ledford <dledford@xxxxxxxxxx>; Jason Gunthorpe
> > > <jgg@xxxxxxxxxxxx>
> > > Cc: Leon Romanovsky <leonro@xxxxxxxxxxxx>; RDMA mailing list <linux-
> > > rdma@xxxxxxxxxxxxxxx>; Erez Alfasi <ereza@xxxxxxxxxxxx>
> > > Subject: [PATCH rdma-next v1 3/4] RDMA/nldev: Provide MR statistics
> > >
> > > From: Erez Alfasi <ereza@xxxxxxxxxxxx>
> > >
> > > Add RDMA nldev netlink interface for dumping MR statistics information.
> > >
> > > Output example:
> > > ereza@dev~$: ./ibv_rc_pingpong -o -P -s 500000000
> > > local address: LID 0x0001, QPN 0x00008a, PSN 0xf81096, GID ::
> > >
> > > ereza@dev~$: rdma stat show mr
> > > dev mlx5_0 mrn 2 page_faults 122071 page_invalidations 0
> > > prefetched_pages 122071
> > >
> > > Signed-off-by: Erez Alfasi <ereza@xxxxxxxxxxxx>
> > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > ---
>
> <...>
>
> > > struct ib_device_ops {
> > > */
> > > int (*counter_update_stats)(struct rdma_counter *counter);
> > >
> > > + /**
> > > + * fill_odp_stats - Fill MR ODP stats into a given
> > > + * ib_odp_counters struct.
> > > + * Return value - true in case counters has been filled,
> > > + * false otherwise (if its non-ODP registered MR for example).
> > > + */
> > > + bool (*fill_odp_stats)(struct ib_mr *mr, struct ib_odp_counters
> > > *cnt);
> > > +
> > Requesting ODP stats on non-ODP MR is an error.
> > Instead of returning bool, please return int = -EINVAL as an error for non ODP
> MRs.
>
> We don't want to add extra checks in the drivers for something that is supposed
> to be checked by upper layer. If caller asks for ODP statistics, he will check that
> MR is ODP backed. Especially if the need to have ODP MR is embedded into the
> function name.
>
That make sense. In that case return type should be void.
Comment says - "false otherwise (if its non-ODP registered MR for example)."
And with that logic,
Below code should be in upper layer instead of mlx5/main.c
+ if (!is_odp_mr(mr))
+ return false;
