| Message ID | 20220908094548.4115-4-guoqing.jiang@linux.dev (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | misc changes for rtrs | expand |
On Thu, Sep 8, 2022 at 11:46 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote: > > Let's call try_cmpxchg directly for the same purpose. > > Acked-by: Md Haris Iqbal <haris.iqbal@ionos.com> > Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev> > Reported-by: kernel test robot <lkp@intel.com> I am not sure whats the correct way of using this. But technically, this change was NOT done due to a report from the "kernel test robot". It only pointed out the problem in the original patch. To the branch maintainers, if its okay to keep this in this scenario, then ignore this comment. Thanks. > --- > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c > index c29eccdb4fd2..2428bf5d07e3 100644 > --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c > @@ -2220,17 +2220,6 @@ static void rtrs_clt_stop_and_destroy_conns(struct rtrs_clt_path *clt_path) > } > } > > -static inline bool xchg_paths(struct rtrs_clt_path __rcu **rcu_ppcpu_path, > - struct rtrs_clt_path *clt_path, > - struct rtrs_clt_path *next) > -{ > - struct rtrs_clt_path **ppcpu_path; > - > - /* Call cmpxchg() without sparse warnings */ > - ppcpu_path = (typeof(ppcpu_path))rcu_ppcpu_path; > - return clt_path == cmpxchg(ppcpu_path, clt_path, next); > -} > - > static void rtrs_clt_remove_path_from_arr(struct rtrs_clt_path *clt_path) > { > struct rtrs_clt_sess *clt = clt_path->clt; > @@ -2305,7 +2294,8 @@ static void rtrs_clt_remove_path_from_arr(struct rtrs_clt_path *clt_path) > * We race with IO code path, which also changes pointer, > * thus we have to be careful not to overwrite it. > */ > - if (xchg_paths(ppcpu_path, clt_path, next)) > + if (try_cmpxchg((struct rtrs_clt_path **)ppcpu_path, > + &clt_path, next)) > /* > * @ppcpu_path was successfully replaced with @next, > * that means that someone could also pick up the > -- > 2.31.1 >
On 9/8/22 5:55 PM, Haris Iqbal wrote: > On Thu, Sep 8, 2022 at 11:46 AM Guoqing Jiang<guoqing.jiang@linux.dev> wrote: >> Let's call try_cmpxchg directly for the same purpose. >> >> Acked-by: Md Haris Iqbal<haris.iqbal@ionos.com> >> Signed-off-by: Guoqing Jiang<guoqing.jiang@linux.dev> >> Reported-by: kernel test robot<lkp@intel.com> > I am not sure whats the correct way of using this. But technically, > this change was NOT done due to a report from the "kernel test robot". > It only pointed out the problem in the original patch. To the branch > maintainers, if its okay to keep this in this scenario, then ignore > this comment. Me either, just want to give credit to lkp for previous report, but not sure if there is a better tag. Thanks, Guoqing
Looks good. As clt->pcpu_path is of type "struct rtrs_clt_path __rcu **ppcpu_path", using without typecasting with cmpxchg() would fetch sparse warning. I still believe we can use rcu_replace_pointer() instead of cmpxchg(), anyway we are going to allow RCU grace period using synchronize_rcu(). Best Regards, Santosh On Thu, Sep 8, 2022 at 12:06 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote: > > > > On 9/8/22 5:55 PM, Haris Iqbal wrote: > > On Thu, Sep 8, 2022 at 11:46 AM Guoqing Jiang<guoqing.jiang@linux.dev> wrote: > >> Let's call try_cmpxchg directly for the same purpose. > >> > >> Acked-by: Md Haris Iqbal<haris.iqbal@ionos.com> > >> Signed-off-by: Guoqing Jiang<guoqing.jiang@linux.dev> > >> Reported-by: kernel test robot<lkp@intel.com> > > I am not sure whats the correct way of using this. But technically, > > this change was NOT done due to a report from the "kernel test robot". > > It only pointed out the problem in the original patch. To the branch > > maintainers, if its okay to keep this in this scenario, then ignore > > this comment. > > Me either, just want to give credit to lkp for previous report, but not sure > if there is a better tag. > > Thanks, > Guoqing
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c index c29eccdb4fd2..2428bf5d07e3 100644 --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c @@ -2220,17 +2220,6 @@ static void rtrs_clt_stop_and_destroy_conns(struct rtrs_clt_path *clt_path) } } -static inline bool xchg_paths(struct rtrs_clt_path __rcu **rcu_ppcpu_path, - struct rtrs_clt_path *clt_path, - struct rtrs_clt_path *next) -{ - struct rtrs_clt_path **ppcpu_path; - - /* Call cmpxchg() without sparse warnings */ - ppcpu_path = (typeof(ppcpu_path))rcu_ppcpu_path; - return clt_path == cmpxchg(ppcpu_path, clt_path, next); -} - static void rtrs_clt_remove_path_from_arr(struct rtrs_clt_path *clt_path) { struct rtrs_clt_sess *clt = clt_path->clt; @@ -2305,7 +2294,8 @@ static void rtrs_clt_remove_path_from_arr(struct rtrs_clt_path *clt_path) * We race with IO code path, which also changes pointer, * thus we have to be careful not to overwrite it. */ - if (xchg_paths(ppcpu_path, clt_path, next)) + if (try_cmpxchg((struct rtrs_clt_path **)ppcpu_path, + &clt_path, next)) /* * @ppcpu_path was successfully replaced with @next, * that means that someone could also pick up the