On Tue, Dec 19, 2017 at 09:21:05PM +0000, Bart Van Assche wrote:
> On Tue, 2017-12-19 at 14:16 -0700, Jason Gunthorpe wrote:
> > The very first thing free_res does is the same lock/signal/unlock
> > sequence, so there is no reason to open code it before calling
> > free_res.
> >
> > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> > srp_daemon/srp_daemon.c | 4 ----
> > 1 file changed, 4 deletions(-)
> >
> > Noticed while looking at honli's patch.. Didn't make a PR.
> >
> > diff --git a/srp_daemon/srp_daemon.c b/srp_daemon/srp_daemon.c
> > index cec36db2e0f12e..a1798558373490 100644
> > +++ b/srp_daemon/srp_daemon.c
> > @@ -2074,10 +2074,6 @@ static int ibsrpdm(int argc, char *argv[])
> > pr_err("Querying SRP targets failed\n");
> >
> > assert(res->sync_res);
> > - pthread_mutex_lock(&res->sync_res->retry_mutex);
> > - res->sync_res->stop_threads = 1;
> > - pthread_cond_signal(&res->sync_res->retry_cond);
> > - pthread_mutex_unlock(&res->sync_res->retry_mutex);
>
> Hello Jason,
>
> Since this patch removes all code that dereferences res->sync_res from
> the ibsrpdm() function, have you considered to remove assert(res->sync_res)
> too?
Well, that would be a behavior change.. free_res works OK if sync_res
is NULL, so I'm guessing that the assert had some meaning to show that
the reconnect_thread was always started for ibsrpd?
Or maybe we shouldn't care about that?
Happy to drop it if you think it makes sense.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
