Followup on patches as571 and as572

Followup on patches as571 and as572

[Alan Stern]

James:

I haven't heard anything back about these two patches:

http://marc.theaimsgroup.com/?l=linux-scsi&m=112801815229091&w=2
http://marc.theaimsgroup.com/?l=linux-scsi&m=112801965207763&w=2

They were submitted a couple of weeks ago, and they don't appear in your 
scsi-misc git repository.

Alan Stern

Re: Followup on patches as571 and as572

[James Bottomley]

On Fri, 2005-10-14 at 11:09 -0400, Alan Stern wrote:
> I haven't heard anything back about these two patches:
> 
> http://marc.theaimsgroup.com/?l=linux-scsi&m=112801815229091&w=2
> http://marc.theaimsgroup.com/?l=linux-scsi&m=112801965207763&w=2
> 
> They were submitted a couple of weeks ago, and they don't appear in your 
> scsi-misc git repository.

I was waiting for Jens to comment since he's supposed to be the sr
maintainer.

To me, the first looks fine.  The second alters the behaviour of error
completion.  Previously we report the number of good bytes up to the
first bad block in linux filesystem terms.  Now we report it up to the
first bad block in hardsect terms.  I'm not sure if anything actually
cares, but I don't really see a reason to change it.

James

Re: Followup on patches as571 and as572

[Alan Stern]

On Fri, 14 Oct 2005, James Bottomley wrote:

> On Fri, 2005-10-14 at 11:09 -0400, Alan Stern wrote:
> > I haven't heard anything back about these two patches:
> > 
> > http://marc.theaimsgroup.com/?l=linux-scsi&m=112801815229091&w=2
> > http://marc.theaimsgroup.com/?l=linux-scsi&m=112801965207763&w=2
> > 
> > They were submitted a couple of weeks ago, and they don't appear in your 
> > scsi-misc git repository.
> 
> I was waiting for Jens to comment since he's supposed to be the sr
> maintainer.
> 
> To me, the first looks fine.  The second alters the behaviour of error
> completion.  Previously we report the number of good bytes up to the
> first bad block in linux filesystem terms.  Now we report it up to the
> first bad block in hardsect terms.  I'm not sure if anything actually
> cares, but I don't really see a reason to change it.

Thanks for responding so quickly.

I'm not sure I understand your comment on the second patch.  The patch 
doesn't affect the value of good_bytes except in two respects:

    (a) When the device does not provide valid information in
	the Information field, the code will now realize it and
	fall back on the Residue.

    (b) The bizarre line that said

		error_sector &= ~(block_sectors - 1);

	is now gone.  Do you know what that was intended to do?

Otherwise the value should be exactly the same as before.  The new 
calc_good_bytes routine does most of its work using 2048-byte hardware 
blocks instead of 512-byte sectors, but the end result is unchanged.

When the Information value is used, the good region must end on a hardware
block boundary because the value is given in units of blocks.  When using
the Residue, in principle the region could end at any byte.  Would you
like me to change the patch so that the Residue-based calculation goes up
to a 512-byte sector boundary instead of a block boundary?

Or were you referring to something else entirely?

Alan Stern

Re: Followup on patches as571 and as572

[James Bottomley]

On Fri, 2005-10-14 at 12:58 -0400, Alan Stern wrote:
>     (b) The bizarre line that said
> 
> 		error_sector &= ~(block_sectors - 1);
> 
> 	is now gone.  Do you know what that was intended to do?

It's not a bizzare line ... it's rounding down to our current set block
size.  That was the difference I noticed in the reporting.  The bio
layer maintains a separate sector size from the actual device, so if the
error occurs in the middle of a block sector (because block sectors >
hard sectors) then we report as though it occurred at the beginning of
the block sector.

James

Re: Followup on patches as571 and as572

[Alan Stern]

On Fri, 14 Oct 2005, James Bottomley wrote:

> On Fri, 2005-10-14 at 12:58 -0400, Alan Stern wrote:
> >     (b) The bizarre line that said
> > 
> > 		error_sector &= ~(block_sectors - 1);
> > 
> > 	is now gone.  Do you know what that was intended to do?
> 
> It's not a bizzare line ... it's rounding down to our current set block
> size.  That was the difference I noticed in the reporting.  The bio
> layer maintains a separate sector size from the actual device, so if the
> error occurs in the middle of a block sector (because block sectors >
> hard sectors) then we report as though it occurred at the beginning of
> the block sector.

I see.  I didn't understand the relation between bio->bi_size and anything
else.  It wasn't clear that this size is always equal to a block-layer
sector size, as opposed to an arbitrary memory buffer size.

I will send a revised patch shortly.

Alan Stern