linux-kernel - Re: [PATCH 03/11] block: add rq->resid

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] [thread-next>] [day] [month] [year] [list]
Message-Id: <1242137329.3308.6.camel@mulgrave.int.hansenpartnership.com>
Date:	Tue, 12 May 2009 09:08:49 -0500
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>,
	bharrosh@...asas.com, axboe@...nel.dk,
	linux-kernel@...r.kernel.org, jeff@...zik.org,
	linux-ide@...r.kernel.org, linux-scsi@...r.kernel.org,
	bzolnier@...il.com, petkovbb@...glemail.com,
	sshtylyov@...mvista.com, mike.miller@...com, Eric.Moore@....com,
	stern@...land.harvard.edu, zaitcev@...hat.com,
	Geert.Uytterhoeven@...ycom.com, sfr@...b.auug.org.au,
	grant.likely@...retlab.ca, paul.clements@...eleye.com,
	tim@...erelk.net, jeremy@...source.com, adrian@...en.demon.co.uk,
	oakad@...oo.com, dwmw2@...radead.org, schwidefsky@...ibm.com,
	ballabio_dario@....com, davem@...emloft.net, rusty@...tcorp.com.au,
	Markus.Lidel@...dowconnect.com, dgilbert@...erlog.com,
	djwong@...ibm.com
Subject: Re: [PATCH 03/11] block: add rq->resid_len

On Tue, 2009-05-12 at 15:04 +0900, Tejun Heo wrote:
> Hello, James.
> 
> James Bottomley wrote:
> >> * Not well defined.  What does it mean really?  It can't indicate
> >>   successful partial transfer.  If the request partially succeeded,
> >>   the required behavior is to successfully complete the request
> >>   partially with residual count and then fail the latter part when
> >>   issued again.  If the failure applies to the whole request but
> >>   location information is useful, it should be carried in the sense
> >>   data.
> > 
> > The definition is the amount of data transfer requested less the actual
> > that went over the wire ... that's certainly a well defined quantity;
> > although, one could argue about what this means in the device.
> > Certainly I agree that just because the data was transferred to or from
> > the device is no guarantee that the device did anything with it (or
> > transferred it accurately).
> 
> 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.

> >> * 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.  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.   Also,
we have the nasty USB case where no error return but an actual residual
tells you something really went wrong.

> >> * Different layers of failing.  In SG_IO interface, a request may fail
> >>   with -EIO way before it reaches block layer.  Residual count can't
> >>   be set to any meaningful value in these cases.  We can set it to
> >>   full count for these fast fail paths, but do we really wanna go
> >>   there?  Another problem is when a driver is missing SG_IO
> >>   capability.  Who's responsible for setting resid count in that case?
> >>   How is upper layer gonna determine a SG_IO failed because lower
> >>   level driver didn't support it or it genuinely failed?
> > 
> > Well, I prefer the concept of transfer length, which would be
> > initialised to zero ... however, residuals should be initialised to
> > the actual transfer count.
> > 
> >> I think it's just silly to give any meaning to resid count when the
> >> request fails.  It's best to leave the field unmodified or just
> >> declare it undefined.
> > 
> > It's current behaviour.  Technically that makes it part of the SG_IO
> > ABI ... although it could be deprecated if someone can verify there are
> > no current users.
> 
> 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.

James


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