Re: [PATCH rdma-core] srp_daemon: Remove duplicate sync_res shutdown

Bart Van Assche
2017-12-19T21:21:05Z

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1267 bytes --]

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@mellanox.com>
> ---
> 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
> --- a/srp_daemon/srp_daemon.c
> +++ 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?

Thanks,

Bart.N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dÊ‡Ú™ë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš