From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mikhail Skorzhinskii <mskorzhinskiy@solarflare.com>
Subject: Re: [PATCH v2 14/14] nvme-tcp: add NVMe over TCP host driver
Date: Wed, 21 Nov 2018 15:01:11 +0300
Message-ID: <8736rufxmg.fsf@solarflare.com>
References: <20181120030019.31738-1-sagi@grimberg.me> <20181120030019.31738-16-sagi@grimberg.me>
Mime-Version: 1.0
Content-Type: text/plain
Cc: <linux-nvme@lists.infradead.org>, <linux-block@vger.kernel.org>,
        <netdev@vger.kernel.org>, "David S. Miller" <davem@davemloft.net>,
        "Keith Busch" <keith.busch@intel.com>,
        Christoph Hellwig <hch@lst.de>
To: Sagi Grimberg <sagi@grimberg.me>
Return-path: <netdev-owner@vger.kernel.org>
Received: from dispatch1-us1.ppe-hosted.com ([67.231.154.164]:33052 "EHLO
        dispatch1-us1.ppe-hosted.com" rhost-flags-OK-OK-OK-OK)
        by vger.kernel.org with ESMTP id S1727046AbeKVALN (ORCPT
        <rfc822;netdev@vger.kernel.org>); Wed, 21 Nov 2018 19:11:13 -0500
In-Reply-To: <20181120030019.31738-16-sagi@grimberg.me>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Sagi Grimberg <sagi@grimberg.me> writes:
 > +static inline void nvme_tcp_queue_request(struct nvme_tcp_request *req)
 > +{
 > +	struct nvme_tcp_queue *queue = req->queue;
 > +
 > +	spin_lock_bh(&queue->lock);
 > +	list_add_tail(&req->entry, &queue->send_list);
 > +	spin_unlock_bh(&queue->lock);
 > +
 > +	queue_work_on(queue->io_cpu, nvme_tcp_wq, &queue->io_work);
 > +}

May be I missing something, but why bother with bottom half version of
locking?

There are few places where this lock could be accessed:

 (1) From ->queue_rq() call;
 (2) From submitting new AEN request;
 (3) From receiving new R2T;

Which one if these originates from bottom half? Not 100% about queue_rq
data path, but (2) and (3) looks perfectly safe for me.

Possibly just a relic of some previous iterations of experimenting?

Mikhail Skorzhinskii