Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751892AbcKRG5h (ORCPT ); Fri, 18 Nov 2016 01:57:37 -0500 Received: from mail-ua0-f180.google.com ([209.85.217.180]:34092 "EHLO mail-ua0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751243AbcKRG5e (ORCPT ); Fri, 18 Nov 2016 01:57:34 -0500 MIME-Version: 1.0 In-Reply-To: References: <1477551554-30349-1-git-send-email-binoy.jayan@linaro.org> <1477551554-30349-6-git-send-email-binoy.jayan@linaro.org> From: Binoy Jayan Date: Fri, 18 Nov 2016 12:27:32 +0530 Message-ID: Subject: Re: [PATCH v4 05/10] IB/isert: Replace semaphore sem with completion To: Sagi Grimberg Cc: Doug Ledford , Sean Hefty , Hal Rosenstock , Arnd Bergmann , linux-rdma@vger.kernel.org, Linux kernel mailing list Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1926 Lines: 45 Hi Sagi, On 31 October 2016 at 02:42, Sagi Grimberg wrote: >> The semaphore 'sem' in isert_device is used as completion, so convert >> it to struct completion. Semaphores are going away in the future. > > > Umm, this is 100% *not* true. np->sem is designed as a counting to > sync the iscsi login thread with the connect requests coming from the > initiators. So this is actually a reliable bug insertion :( > > NAK from me... Sorry for the late reply as I was held up in other activities. I converted this to a wait_event() implementation but as I was doing it, I was wondering how it would have been different if it was a completion and not a semaphore. File: drivers/infiniband/ulp/isert/ib_isert.c If isert_connected_handler() is called multiple times, adding an entry to the list, and if that happens while we use completion, 'done' (part of struct completion) would be incremented by 1 each time 'complete' is called from isert_connected_handler. After 'n' iterations, done will be equal to 'n'. If we call wait_for_completion now from isert_accept_np, it would just decrement 'done' by one and continue without blocking, consuming one node at a time from the list 'isert_np->pending'. Alternatively if "done" becomes zero, and the next time wait_for_completion is called, the API would add a node at the end of the wait queue 'wait' in 'struct completion' and block until "done" is nonzero. (Ref: do_wait_for_common) It exists the wait when a call to 'complete' turns 'done' back to 1. But if there are multiple waits called before calling complete, all the tasks calling the wait gets queued up and they will all would see "done" set to zero. When complete is called now, done turns 1 again and the first task in the queue is woken up as it is serialized as FIFO. Now the first wait returns and the done is decremented by 1 just before the return. Am I missing something here? Thanks, Binoy