All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Tejun Heo <tj@kernel.org>
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: Tue, 12 May 2009 10:45:26 -0500	[thread overview]
Message-ID: <1242143126.3308.20.camel@mulgrave.int.hansenpartnership.com> (raw)
In-Reply-To: <4A0992ED.4090200@kernel.org>

On Wed, 2009-05-13 at 00:17 +0900, Tejun Heo wrote:
> 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.

Well, no, the definition is the same in both cases:  how much data went
over the wire.  In the success case, you assume that means the device
processed it correctly.  In the failure case you need to interpret the
error (and possibly device specific knowledge) to determine what it
means.

> >>>> * 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.

Heh, well, this is where we get into interpretations.  For SG_IO
requests, we have three separate ways of returning error.  The error
return to the block layer, the results return and the sense code.  The
error to block is a somewhat later addition to the layer, so not all
cases are handled right or agreed (for instance we just altered BLOCK_PC
with recovered error from -EIO to no error).  So hopefully we've just
decided that deferred and current but recovered all fall into the no
error to block, but results and sense to the user.

Note that the error to block is basically discarded from SG_IO before we
return to the user, so the user *only* has results and sense to go by,
thus the concept of residual not valid on error to block is something
the user can't check.  That's why a consistent definition in all cases
(i.e. the amount of data the HBA transferred) is the correct one and
allows userspace to make the determination of what it should do based on
the returns it gets.

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

It's not valid bytes in either case ... it's number transferred.  One
can infer from a successful SCSI status code that number transferred ==
valid bytes, but I'd rather we didn't say that.

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

But that's the point ... we don't define for userland what request
failure is very well.

> * 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.

OK, so if what you're asking is that we can regard the residue as
invalid if SG_IO itself returns an error, then I can agree ... but not
if blk_end_request() returns error, because that error gets ignored by
SG_IO.

James



  reply	other threads:[~2009-05-12 15:45 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
2009-05-12 15:45                     ` James Bottomley [this message]
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=1242143126.3308.20.camel@mulgrave.int.hansenpartnership.com \
    --to=james.bottomley@hansenpartnership.com \
    --cc=Eric.Moore@lsi.com \
    --cc=Geert.Uytterhoeven@sonycom.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=tj@kernel.org \
    --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.