linux-kernel - Re: [RFC/PATCH 2/2] scsi: ufs: requests completion handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1377801416.13031.30.camel@dabdike>
Date:	Thu, 29 Aug 2013 22:36:56 +0400
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	Yaniv Gardi <ygardi@...eaurora.org>
Cc:	'Raviv Shvili' <rshvili@...eaurora.org>, scsi-misc@...r.kernel.org,
	linux-arm-msm@...r.kernel.org,
	"'open list:SCSI SUBSYSTEM'" <linux-scsi@...r.kernel.org>,
	'open list' <linux-kernel@...r.kernel.org>,
	merez@...eaurora.org
Subject: Re: [RFC/PATCH 2/2] scsi: ufs: requests completion handling

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@...r.kernel.org
> [mailto:linux-scsi-owner@...r.kernel.org] On Behalf Of James Bottomley
> Sent: Thursday, August 29, 2013 12:28 PM
> To: Raviv Shvili
> Cc: scsi-misc@...r.kernel.org; linux-arm-msm@...r.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@...r.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@...r.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/