All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>,
	bharrosh@panasas.com, axboe@kernel.dk,
	linux-kernel@vger.kernel.org, jeff@garzik.org,
	linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,
	bzolnier@gmail.com, petkovbb@googlemail.com,
	sshtylyov@ru.mvista.com, mike.miller@hp.com, Eric.Moore@lsi.com,
	stern@rowland.harvard.edu, zaitcev@redhat.com,
	Geert.Uytterhoeven@sonycom.com, sfr@canb.auug.org.au,
	grant.likely@secretlab.ca, paul.clements@steeleye.com,
	tim@cyberelk.net, jeremy@xensource.com, adrian@mcmen.demon.co.uk,
	oakad@yahoo.com, dwmw2@infradead.org, schwidefsky@de.ibm.com,
	ballabio_dario@emc.com, davem@davemloft.net,
	rusty@rustcorp.com.au, Markus.Lidel@shadowconnect.com,
	dgilbert@interlog.com, djwong@us.ibm.com
Subject: Re: [PATCH 03/11] block: add rq->resid_len
Date: Wed, 13 May 2009 00:17:01 +0900	[thread overview]
Message-ID: <4A0992ED.4090200@kernel.org> (raw)
In-Reply-To: <1242137329.3308.6.camel@mulgrave.int.hansenpartnership.com>

Hello, James.

James Bottomley wrote:
>> I think it's more like how many bytes are valid where the validity is
>> defined as the number of meaningful bytes on dev -> host commands and
>> the number of bytes the device actually consumed on the other
>> direction.  Please note that this is different from the number of
>> bytes transferred due to padding or under other error conditions.
> 
> For failed commands we don't have that information.  All we know is how
> many bytes were actually transferred (because the HBA keeps a count), so
> it's the actual transfer count we use to construct the residual.  No
> imputation of validity or otherwise.  It just says I transferred this
> amount, based on the error make of it what you will.

Hmmm... so two separate meaning depending on command success/failure?
I doubt high level code or user application can make much use out of
raw transfer byte count.  It doesn't mean all that much as it varies
depending on specific transport protocol in use.

>>>> * What about corner values?  What does 0 or full resid count on
>>>>   failure mean?
>>> 0 means everything transferred, full residual means nothing did.
>> Yeap, I was wondering about the combination 0 resid count + failure.
>> What would it mean?  All bytes are valid but the command failed?
> 
> Well, there are certain SCSI conditions called deferred errors and
> the like where we return Check Condition but everything's OK,
> redisual count should be zero, same goes for recovered errors
> ... there are actually lots of things we can get back as an "error"
> which means I'm warning you of something, but the transfer was OK.

Shouldn't those be request successful w/ sense data?  Please note that
the term "error" in this context means failure of block layer request
not SCSI layer CHECK SENSE.

> Likewise we get unit attentions (essentialy AENs) which mean I'm
> telling you something before you start, so please try again.  Here
> residual would be the full transfer.

And these are request failures.

> Also, we have the nasty USB case where no error return but an actual
> residual tells you something really went wrong.

This is something to be interpreted by lld, be it usb or scsi layer,
from block layer POV, if it's partial completion, it's success w/
residue count.  If nothing really happened, it's request failure.

>> The behavior wasn't guaranteed before the change in paths including
>> SG_IO fast fail one.  libata and ide have been and are completely
>> funky about residual counts anyway so I highly doubt anyone has been
>> depending on it.
>>
>> There's nothing wrong with keeping the original behavior in itself but
>> to me it looks like it would be a bad precedence when no one should
>> depend on the behavior.
> 
> OK, that's what we'll do then, thanks.

I'm still reluctant to do it because...

* Its definition still isn't clear (well, at least to me) and if it's
  defined as the number of valid bytes on request success and the
  number of bytes HBA transferred on request failure, I don't think
  it's all that useful.

* Seen from userland, residue count on request failure has never been
  guaranteed and there doesn't seem to be any valid in kernel user.

* It would be extra code explicitly setting the residue count to full
  on failure path.  If it's something necessary, full residue count on
  failure needs to be made default.  If not, it will only add more
  confusion.

Thanks.

-- 
tejun

  parent reply	other threads:[~2009-05-12 15:21 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-04  7:58 [GIT PATCH] block,scsi,ide: unify sector and data_len, take#2 Tejun Heo
2009-05-04  7:58 ` Tejun Heo
2009-05-04  7:58 ` [PATCH 01/11] nbd: don't clear rq->sector and nr_sectors unnecessarily Tejun Heo
2009-05-04  7:58   ` Tejun Heo
2009-05-04  7:58 ` [PATCH 02/11] ide-tape: don't initialize rq->sector for rw requests Tejun Heo
2009-05-04  7:58   ` Tejun Heo
2009-05-04  7:58 ` [PATCH 03/11] block: add rq->resid_len Tejun Heo
2009-05-04  7:58   ` Tejun Heo
2009-05-04 12:08   ` Sergei Shtylyov
2009-05-05  3:41     ` Tejun Heo
2009-05-07 10:23   ` FUJITA Tomonori
2009-05-10 14:07   ` Boaz Harrosh
2009-05-10 23:48     ` Tejun Heo
2009-05-11  5:49       ` FUJITA Tomonori
2009-05-11 14:18         ` James Bottomley
2009-05-11 15:03           ` FUJITA Tomonori
2009-05-11 15:13             ` James Bottomley
2009-05-11 23:47               ` FUJITA Tomonori
2009-05-12  0:19           ` Tejun Heo
2009-05-12  3:43             ` James Bottomley
2009-05-12  6:04               ` Tejun Heo
2009-05-12 14:08                 ` James Bottomley
2009-05-12 14:34                   ` Alan Stern
2009-05-12 14:34                     ` Alan Stern
2009-05-12 15:17                   ` Tejun Heo [this message]
2009-05-12 15:45                     ` James Bottomley
2009-05-13  6:30                       ` Tejun Heo
2009-05-11 11:31       ` Boaz Harrosh
2009-05-11 14:59         ` FUJITA Tomonori
2009-05-12  8:58           ` Boaz Harrosh
2009-05-12 15:00             ` FUJITA Tomonori
2009-05-12 15:08               ` Boaz Harrosh
2009-05-12 15:16               ` FUJITA Tomonori
2009-05-12  0:27         ` Tejun Heo
2009-05-12  8:46           ` Boaz Harrosh
2009-05-12  9:07             ` Tejun Heo
2009-05-12  9:10               ` Tejun Heo
2009-05-12  9:52               ` Boaz Harrosh
2009-05-12 10:06                 ` Tejun Heo
2009-05-12 11:08                   ` Boaz Harrosh
2009-05-12 15:20                     ` Tejun Heo
2009-05-12 15:53                       ` Boaz Harrosh
2009-05-04  7:58 ` [PATCH 04/11] block: implement blk_rq_pos/[cur_]sectors() and convert obvious ones Tejun Heo
2009-05-04  7:58   ` Tejun Heo
2009-05-04 13:45   ` Sergei Shtylyov
2009-05-05  3:42     ` Tejun Heo
2009-05-04  7:58 ` [PATCH 05/11] block: convert to pos and nr_sectors accessors Tejun Heo
2009-05-04  7:58   ` Tejun Heo
2009-05-04 19:48   ` Adrian McMenamin
2009-05-05  3:42     ` Tejun Heo
2009-05-04  7:58 ` [PATCH 06/11] ide: convert to rq " Tejun Heo
2009-05-04  7:58   ` Tejun Heo
2009-05-04  7:58 ` [PATCH 07/11] block: drop request->hard_* and *nr_sectors Tejun Heo
2009-05-04  7:58   ` Tejun Heo
2009-05-04  7:58 ` [PATCH 08/11] block: cleanup rq->data_len usages Tejun Heo
2009-05-04  7:58   ` Tejun Heo
2009-05-04 14:41   ` Sergei Shtylyov
2009-05-11 12:02   ` Boaz Harrosh
2009-05-04  7:58 ` [PATCH 09/11] ide: " Tejun Heo
2009-05-04  7:58   ` Tejun Heo
2009-05-04  7:58 ` [PATCH 10/11] block: hide request sector and data_len Tejun Heo
2009-05-04  7:58   ` Tejun Heo
2009-05-04  7:58 ` [PATCH 11/11] block: blk_rq_[cur_]_{sectors|bytes}() usage cleanup Tejun Heo
2009-05-04  7:58   ` Tejun Heo
2009-05-05  3:59 ` [GIT PATCH] block,scsi,ide: unify sector and data_len, take#2 Tejun Heo
2009-05-05  3:59   ` Tejun Heo
2009-05-07  2:48   ` Tejun Heo
2009-05-07  2:48     ` Tejun Heo
2009-05-07 10:23 ` FUJITA Tomonori
2009-05-08  2:06   ` FUJITA Tomonori
2009-05-08  9:11     ` Tejun Heo
2009-05-11 12:06 ` Boaz Harrosh
2009-05-12  0:49   ` Tejun Heo

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=4A0992ED.4090200@kernel.org \
    --to=tj@kernel.org \
    --cc=Eric.Moore@lsi.com \
    --cc=Geert.Uytterhoeven@sonycom.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=Markus.Lidel@shadowconnect.com \
    --cc=adrian@mcmen.demon.co.uk \
    --cc=axboe@kernel.dk \
    --cc=ballabio_dario@emc.com \
    --cc=bharrosh@panasas.com \
    --cc=bzolnier@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dgilbert@interlog.com \
    --cc=djwong@us.ibm.com \
    --cc=dwmw2@infradead.org \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=grant.likely@secretlab.ca \
    --cc=jeff@garzik.org \
    --cc=jeremy@xensource.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=mike.miller@hp.com \
    --cc=oakad@yahoo.com \
    --cc=paul.clements@steeleye.com \
    --cc=petkovbb@googlemail.com \
    --cc=rusty@rustcorp.com.au \
    --cc=schwidefsky@de.ibm.com \
    --cc=sfr@canb.auug.org.au \
    --cc=sshtylyov@ru.mvista.com \
    --cc=stern@rowland.harvard.edu \
    --cc=tim@cyberelk.net \
    --cc=zaitcev@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.