Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1757043Ab3H2ShF (ORCPT <rfc822;w@1wt.eu>);
	Thu, 29 Aug 2013 14:37:05 -0400
Received: from bedivere.hansenpartnership.com ([66.63.167.143]:47906 "EHLO
	bedivere.hansenpartnership.com" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1756720Ab3H2ShC (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 29 Aug 2013 14:37:02 -0400
Message-ID: <1377801416.13031.30.camel@dabdike>
Subject: Re: [RFC/PATCH 2/2] scsi: ufs: requests completion handling
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Yaniv Gardi <ygardi@codeaurora.org>
Cc: "'Raviv Shvili'" <rshvili@codeaurora.org>, scsi-misc@vger.kernel.org,
        linux-arm-msm@vger.kernel.org,
        "'open list:SCSI SUBSYSTEM'" <linux-scsi@vger.kernel.org>,
        "'open list'" <linux-kernel@vger.kernel.org>, merez@codeaurora.org
Date: Thu, 29 Aug 2013 22:36:56 +0400
In-Reply-To: <06be01cea4de$77b79ce0$6726d6a0$@codeaurora.org>
References: <1377766493-5269-1-git-send-email-rshvili@codeaurora.org>
	 <1377768462.2223.70.camel@dabdike>
	 <06be01cea4de$77b79ce0$6726d6a0$@codeaurora.org>
Content-Type: text/plain; charset="ISO-8859-15"
X-Mailer: Evolution 3.8.5 
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 4224
Lines: 102

On Thu, 2013-08-29 at 20:37 +0300, Yaniv Gardi wrote:
> Hi James,
> 
> See reply inline
> 
> Thanks,
> Yaniv
> 
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of James Bottomley
> Sent: Thursday, August 29, 2013 12:28 PM
> To: Raviv Shvili
> Cc: scsi-misc@vger.kernel.org; linux-arm-msm@vger.kernel.org; open list:SCSI
> SUBSYSTEM; open list
> Subject: Re: [RFC/PATCH 2/2] scsi: ufs: requests completion handling
> 
> On Thu, 2013-08-29 at 11:54 +0300, Raviv Shvili wrote:
> > The patch solves the request completion report order. At the current 
> > implementation, when multiple requests end at the same interrupt call, 
> > the requests reported as completed according to a bitmap scan from the 
> > lowest tags to the highest, regardless the requests priority. That 
> > cause to a priority unfairness and starvation of requests with a high
> tags.
> 
> It does?  Why?  What seems to happen is that you loop over all the pending
> requests and call done for them.  The way SCSI handles done commands is that
> it queues them to the softirq, so there doesn't look to be any real
> unfairness problem here.
> 
> <yaniv> The unfairness is that currently the loop goes over the tags
> from 0
> to NUTRS(i.e 31), and calls done() at this order, regardless of the
> task_attribute they hold.
> Also, the benefit in performance is that instead of going over NUTRS
> (32)
> iteration, we simple call done() only for the exact of completed
> request
> (and according to their task_attribute priority). 

Yes, I know that.  But all done does is queue the completion to the
softirq.  All you get with this is a reordering of that queue.  If you
can actually measure the performance impact of that, I'd be very
surprised.  We're talking under a microsecond in a round trip activity
that takes tens to hundreds of miliseconds to issue and complete.

> Scenario: a new HEAD_OF_QUEUE request that is completed during the
> current
> loop, will be served only in the next interrupt (since the DOORBELL
> will be
> read again only in the next interrupt), and saying it is a high tag,
> it will
> be completed lastly. This patch will fix it, as I see that.

It fixes something that isn't a problem.  The softirq won't even be
activated until all pending interrupts are serviced, so a command
arriving in the middle of processing gets immediately serviced on the
next interrupt before the softirq activates.

James

> > SCSI Architecture Model 5 defines 3 task-attributes that are part of 
> > each SCSI command, and integrated into each Command UPIU. The 
> > task-attribute is for the device usage, it determines the order in 
> > which the device prioritizes the requests.
> > The task-attributes according to their priority are (from high to low):
> > HEAD OF QUEUE, ORDERED and SIMPLE. There is a queue per task-attribute.
> > Each request is assigned to one of the above sw queues according to 
> > its task attribute field.
> > Requests which are not SCSI commands (native UFS) will be assigned to 
> > the lowest priority queue, since there is no much difference between 
> > completing it first or last..
> > 
> > When request is completed, we go over the queues (from the queue's 
> > highest priority to the lowest) and report the completion.
> > 
> > Requests are removed from the queue in case of command completion or 
> > when aborting pending command.
> 
> Since we never use anything other than SIMPLE attributes, this rather looks
> like a solution in search of a problem.
> 
> James
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
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/