Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755292AbbKWWSU (ORCPT ); Mon, 23 Nov 2015 17:18:20 -0500 Received: from quartz.orcorp.ca ([184.70.90.242]:47587 "EHLO quartz.orcorp.ca" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755117AbbKWWSQ (ORCPT ); Mon, 23 Nov 2015 17:18:16 -0500 Date: Mon, 23 Nov 2015 15:18:06 -0700 From: Jason Gunthorpe To: Bart Van Assche Cc: Christoph Hellwig , "linux-rdma@vger.kernel.org" , "sagig@dev.mellanox.co.il" , "axboe@fb.com" , "linux-scsi@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH 2/9] IB: add a proper completion queue abstraction Message-ID: <20151123221806.GA7152@obsidianresearch.com> References: <1447422410-20891-1-git-send-email-hch@lst.de> <1447422410-20891-3-git-send-email-hch@lst.de> <20151113182513.GB21808@obsidianresearch.com> <564640C4.3000603@sandisk.com> <20151113220636.GA32133@obsidianresearch.com> <20151114071344.GE27738@lst.de> <20151123203712.GB5640@obsidianresearch.com> <56537F59.4080708@sandisk.com> <20151123212822.GE6062@obsidianresearch.com> <56538AFD.9080103@sandisk.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56538AFD.9080103@sandisk.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Broken-Reverse-DNS: no host name found for IP address 10.0.0.160 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1388 Lines: 35 On Mon, Nov 23, 2015 at 01:54:05PM -0800, Bart Van Assche wrote: > Not really ... Please have a look at the SRP initiator source code. What the > SRP initiator does is to poll the send queue before sending a new > SCSI I see that. What I don't see is how SRP handles things when the sendq fills up, ie the case where __srp_get_tx_iu() == NULL. It looks like the driver starts to panic and generates printks. I can't tell if it can survive that, but it doesn't look very good.. It would be a lot better if this wasn't allowed to happen, the polling loop can run until a sendq becomes available, and never return null from __srp_get_tx_iu(). Ie, __srp_get_tx_iu should look more like ib_poll_cq_until(..., !list_empty(&ch->free_tx)); Which would be a fairly sane core API for this direct usage.. Ideally the core code would sleep if possible and not just spin. Maybe also protect it with a timer. > command to the target system starts. I think this approach could also be > used in other ULP drivers if the send queue poll frequency is such that no > send queue overflow occurs. Yes, I agree, but it has to be done properly :) Jason -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/