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]

Date:	Mon, 11 May 2009 09:18:20 -0500
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
Cc:	tj@...nel.org, 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 Mon, 2009-05-11 at 14:49 +0900, FUJITA Tomonori wrote:
> On Mon, 11 May 2009 08:48:53 +0900
> Tejun Heo <tj@...nel.org> wrote:
> 
> > Hello, Boaz.
> > 
> > Boaz Harrosh wrote:
> > 
> > > Hi Tejun, I've carefully reviewed these files which I know more
> > > about.  The drivers/block files I've skipped, since I'm not familiar
> > > with this code.
> > > 
> > > Except a small fallout, it looks very good. See some comments plus
> > > Ack/review below
> > 
> > Thanks a lot for reviewing it closely.  It's really nice to have
> > careful extra pair of eyes on the changes.  :-)
> > 
> > >> --- a/drivers/message/fusion/mptsas.c
> > >> +++ b/drivers/message/fusion/mptsas.c
> > >> @@ -1357,8 +1357,7 @@ static int mptsas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
> > >>  		smprep = (SmpPassthroughReply_t *)ioc->sas_mgmt.reply;
> > >>  		memcpy(req->sense, smprep, sizeof(*smprep));
> > >>  		req->sense_len = sizeof(*smprep);
> > >> -		req->data_len = 0;
> > >> -		rsp->data_len -= smprep->ResponseDataLength;
> > >> +		rsp->resid_len = rsp->data_len - smprep->ResponseDataLength;
> > >>  	} else {
> > >>  		printk(MYIOC_s_ERR_FMT "%s: smp passthru reply failed to be returned\n",
> > >>  		    ioc->name, __func__);
> > > 
> > > I think original code was assuming full residual count on the else side
> > > (not MPT_IOCTL_STATUS_RF_VALID). So maybe add:
> > > 
> > > +		rsp->resid_len = rsp->data_len;
> > 
> > Does resid_len make any sense w/ failed requests?  I think we would be
> > better off with declaring residual count to be undefined on request
> > failure.  Is there any place which depends on it?
> 
> IIRC, I wrote the code. I think that this doesn't matter but it's
> better not to change the behavior unless Eric ack on this change
> (maybe LSI has some management binary that assume this behavior though
> it's unlikely).

Actually, yes it does, for many possible reasons.

The first being if the device is too stupid to report an actual sector
location the next best way of determining where the error occurred is
from the residual.  We don't make use of this in kernel (perhaps we
should?) but some of the user space programs for CD/DVD burning do.

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/