linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sujit Reddy Thumma <sthumma@codeaurora.org>
To: Santosh Y <santoshsy@gmail.com>
Cc: Dolev Raviv <draviv@codeaurora.org>,
	linux-scsi@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
Date: Fri, 14 Jun 2013 13:10:38 +0530	[thread overview]
Message-ID: <51BAC8F6.5010601@codeaurora.org> (raw)
In-Reply-To: <51B94BAF.2070203@codeaurora.org>

On 6/13/2013 10:03 AM, Sujit Reddy Thumma wrote:
>>   static struct scsi_host_template ufshcd_driver_template = {
>> @@ -1771,8 +2064,8 @@ int ufshcd_init(struct device *dev, struct
>> ufs_hba **hba_handle,
>>          /* Configure LRB */
>>          ufshcd_host_memory_configure(hba);
>>
>> -       host->can_queue = hba->nutrs;
>> -       host->cmd_per_lun = hba->nutrs;
>> +       host->can_queue = SCSI_CMD_QUEUE_SIZE;
>
>
> I don't think this is appropriate. Reserving a slot exclusively for
> query/DM requests is not optimal.  can_queue should be changed
> dynamically, scsi_adjust_queue_depth() maybe?

The motivation to change the design for this patch  is that routing
query command through SCSI layer raised problems when we are trying to
improve the fatal error handling as we need to block the SCSI layer and
recover the link. Hence, the need to have the query/DM command send as
internal commands. Which probably makes sense.

One possible option was to look for a free command slot whenever we
try to send an internal command, but getting a free slot is not always 
guaranteed.

Even if we get hold of a tag, there is no way we can explain this to
SCSI/block layer that particular tag is in use internally (case where
normal query requests are sent in tandem with SCSI requests). In which
case, other option is to use tag[31] as you have said on need basis
and change the queue depth to 31. This again has problems - one
changing queue depth doesn't take effect immediately but for the next
command. Second, if the command in tag[31] is the root cause of the
fatal error and is stuck, then the internal command has to wait forever
(until scsi timesout) to plan recovery. Considering, all these factors
it is better to reserve a tag and use it for internal commands instead 
of playing around with the tags internally without/partially informing 
SCSI/block layers.

I am open for discussion, if there are any suggestions to handle such 
LLD requirements in the SCSI layer optimally.

Coming to how optimal is to work with 31 slots instead of h/w defined 32 
is something which we can answer when we have true multi queueing. 
AFAIK, there may not exist real-world applications which utilize full 32 
tags at particular instant. SATA AHCI controller driver which is 
ubiquitous in modern systems also reserves a slot for internal commands 
which is used only in case of error handling and AFAIK, no one has ever 
reported performance problems with this (its about 7 years the commit to 
reserve a slot is merged into Linux tree).

I hope this explains the intent. Please let me know what do you think.

-- 
Regards,
Sujit

  reply	other threads:[~2013-06-14  7:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-10 14:05 [PATCH 0/2] Dolev Raviv
2013-06-10 14:05 ` [PATCH 1/2] scsi: ufs: Add support for sending NOP OUT UPIU Dolev Raviv
2013-06-12  5:30   ` Santosh Y
2013-06-13  4:33     ` Sujit Reddy Thumma
2013-06-14  7:40       ` Sujit Reddy Thumma [this message]
2013-06-14 13:47         ` Santosh Y
2013-06-10 14:05 ` [PATCH 2/2] scsi: ufs: Set fDeviceInit flag to initiate device initialization Dolev Raviv
2013-06-12  5:34   ` Santosh Y
2013-06-13  4:36     ` Sujit Reddy Thumma
2013-06-14 13:55       ` Santosh Y
  -- strict thread matches above, loose matches on Subject: below --
2013-06-10 14:12 [PATCH 0/2] Add suport for internal request (NOP and Query Request) Dolev Raviv
2013-06-10 14:12 ` [PATCH 1/2] scsi: ufs: Add support for sending NOP OUT UPIU Dolev Raviv

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51BAC8F6.5010601@codeaurora.org \
    --to=sthumma@codeaurora.org \
    --cc=draviv@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=santoshsy@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).