| Message ID | 20171219215102.GA28841@ziepe.ca (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Leon Romanovsky |
| Headers | show |
On Tue, 2017-12-19 at 14:51 -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. > > res->sync_res always exists at this point so remove the assert too. Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
On Tue, Dec 19, 2017 at 02:51:02PM -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. > > res->sync_res always exists at this point so remove the assert too. > > Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> > --- > srp_daemon/srp_daemon.c | 6 ------ > 1 file changed, 6 deletions(-) > > v3: > - Delete stray edit > > Bart, Yes, must have been a stray editor edit, I did not even look at v2 in > the mailer before sending it, since the one line change was so simple :( > This one compiles. > > diff --git a/srp_daemon/srp_daemon.c b/srp_daemon/srp_daemon.c > index cec36db2e0f12e..5995e6a6173046 100644 > --- a/srp_daemon/srp_daemon.c > +++ b/srp_daemon/srp_daemon.c > @@ -2073,12 +2073,6 @@ static int ibsrpdm(int argc, char *argv[]) > if (ret) > 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); > - > free_res(res); > umad_done: > umad_done(); Reviewed-by: Honggang Li <honli@redhat.com> thanks -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 19, 2017 at 02:51:02PM -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. > > res->sync_res always exists at this point so remove the assert too. > > Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> > --- > srp_daemon/srp_daemon.c | 6 ------ > 1 file changed, 6 deletions(-) > Thanks, applied
diff --git a/srp_daemon/srp_daemon.c b/srp_daemon/srp_daemon.c index cec36db2e0f12e..5995e6a6173046 100644 --- a/srp_daemon/srp_daemon.c +++ b/srp_daemon/srp_daemon.c @@ -2073,12 +2073,6 @@ static int ibsrpdm(int argc, char *argv[]) if (ret) 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); - free_res(res); umad_done: umad_done();
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. res->sync_res always exists at this point so remove the assert too. Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> --- srp_daemon/srp_daemon.c | 6 ------ 1 file changed, 6 deletions(-) v3: - Delete stray edit Bart, Yes, must have been a stray editor edit, I did not even look at v2 in the mailer before sending it, since the one line change was so simple :( This one compiles.