Re: dm: Integrity errors should not cause failover

Re: dm: Integrity errors should not cause failover

[Mike Snitzer]

On Tue, Mar 08 2011 at  1:54am -0500,
Martin K. Petersen <martin.petersen@oracle.com> wrote:

> 
> Integrity errors need to be passed to the owner of the integrity
> metadata for processing. Consequently EILSEQ should be passed up the
> stack.
> 
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

Acked-by: Mike Snitzer <snitzer@redhat.com>

Thanks Martin.  But James has already staged a few patches associated
with IO error differentation for 2.6.39.  Your dm-mpath patch will need
to be rebased on that work; best to feed it to James too.

(we were gambling that there wouldn't be additional DM or block changes
related to this work and figured it safe to have James carry all the
SCSI, block and DM changes... ;)

Problem is I'm not seeing a 2.6.39 branch in James' scsi-2.6-misc.git
(even though I know the changes are staged there ;)

See:
http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commit;h=79775567e0439ca47eb9f501e52c4b713d44cf89
http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commit;h=751b2a7d623ead9e55f751a6087efeab454b5659
http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commit;h=63583cca745f440167bf27877182dc13e19d4bcf

James,
Any easy way to get Martin your 2.6.39 tree?  Do you have a branch (just
the tag isn't exposed)?

Mike

> ---
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index f336c69..57f7dd3 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -1215,7 +1215,7 @@ static int do_end_io(struct multipath *m, struct request *clone,
>  	if (!error && !clone->errors)
>  		return 0;	/* I/O complete */
>  
> -	if (error == -EOPNOTSUPP)
> +	if (error == -EOPNOTSUPP || error == -EILSEQ)
>  		return error;
>  
>  	if (mpio->pgpath)
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

Re: dm: Integrity errors should not cause failover

[James Bottomley]

On Tue, 2011-03-08 at 10:14 -0500, Mike Snitzer wrote:
> On Tue, Mar 08 2011 at  1:54am -0500,
> Martin K. Petersen <martin.petersen@oracle.com> wrote:
> 
> > 
> > Integrity errors need to be passed to the owner of the integrity
> > metadata for processing. Consequently EILSEQ should be passed up the
> > stack.
> > 
> > Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> 
> Acked-by: Mike Snitzer <snitzer@redhat.com>
> 
> Thanks Martin.  But James has already staged a few patches associated
> with IO error differentation for 2.6.39.  Your dm-mpath patch will need
> to be rebased on that work; best to feed it to James too.
> 
> (we were gambling that there wouldn't be additional DM or block changes
> related to this work and figured it safe to have James carry all the
> SCSI, block and DM changes... ;)
> 
> Problem is I'm not seeing a 2.6.39 branch in James' scsi-2.6-misc.git
> (even though I know the changes are staged there ;)
> 
> See:
> http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commit;h=79775567e0439ca47eb9f501e52c4b713d44cf89
> http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commit;h=751b2a7d623ead9e55f751a6087efeab454b5659
> http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commit;h=63583cca745f440167bf27877182dc13e19d4bcf
> 
> James,
> Any easy way to get Martin your 2.6.39 tree?  Do you have a branch (just
> the tag isn't exposed)?

Branch for what?  It's a linear tree with all the SCSI patches on
master.

James

Re: dm: Integrity errors should not cause failover

[Martin K. Petersen]

>>>>> "Mike" == Mike Snitzer <snitzer@redhat.com> writes:

Mike> But James has already staged a few patches associated with IO
Mike> error differentation for 2.6.39.  Your dm-mpath patch will need to
Mike> be rebased on that work;

Oh, I didn't see they finally went in. I'll take a look at scsi-misc.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: dm: Integrity errors should not cause failover

[Mike Snitzer]

On Tue, Mar 08 2011 at 10:20am -0500,
James Bottomley <James.Bottomley@suse.de> wrote:

> On Tue, 2011-03-08 at 10:14 -0500, Mike Snitzer wrote:
> > On Tue, Mar 08 2011 at  1:54am -0500,
> > Martin K. Petersen <martin.petersen@oracle.com> wrote:
> > 
> > > 
> > > Integrity errors need to be passed to the owner of the integrity
> > > metadata for processing. Consequently EILSEQ should be passed up the
> > > stack.
> > > 
> > > Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> > 
> > Acked-by: Mike Snitzer <snitzer@redhat.com>
> > 
> > Thanks Martin.  But James has already staged a few patches associated
> > with IO error differentation for 2.6.39.  Your dm-mpath patch will need
> > to be rebased on that work; best to feed it to James too.
> > 
> > (we were gambling that there wouldn't be additional DM or block changes
> > related to this work and figured it safe to have James carry all the
> > SCSI, block and DM changes... ;)
> > 
> > Problem is I'm not seeing a 2.6.39 branch in James' scsi-2.6-misc.git
> > (even though I know the changes are staged there ;)
> > 
> > See:
> > http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commit;h=79775567e0439ca47eb9f501e52c4b713d44cf89
> > http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commit;h=751b2a7d623ead9e55f751a6087efeab454b5659
> > http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commit;h=63583cca745f440167bf27877182dc13e19d4bcf
> > 
> > James,
> > Any easy way to get Martin your 2.6.39 tree?  Do you have a branch (just
> > the tag isn't exposed)?
> 
> Branch for what?  It's a linear tree with all the SCSI patches on
> master.

Ah.. good to know.  I hadn't looked that closely ;)

But now that I look, you've been quite busy!  The above commits are
buried pretty deep by now.

Mike

Re: dm: Integrity errors should not cause failover

[Mike Snitzer]

On Wed, May 18 2011 at  8:11am -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Wed, May 18 2011 at 12:49am -0400,
> Martin K. Petersen <martin.petersen@oracle.com> wrote:
> 
> > 
> > Integrity errors need to be passed to the owner of the integrity
> > metadata for processing. Consequently EILSEQ should be passed up the
> > stack.
> > 
> > Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> > Acked-by: Mike Snitzer <snitzer@redhat.com>
> > 
> > ---
> > 
> > Ping on this one?
> 
> It has been on my personal queue but I didn't push the issue with
> Alasdair (for 2.6.39).
> 
> At this point seems we should be targetting this fix for 2.6.40 but I'll
> defer to Alasdair on whether he'd like to push to .39 last minute.

Hi Martin,

Alasdair and I just chatted about this patch: it will always pass the
the -EILSEQ up _without_ performing any mpath path failure.

The newfound concern is: is there ever a benefit to failing the path
before returning -EILSEQ?

Or will -EILSEQ always imply there is nothing wrong with the path?
E.g. a transport error cannot cause a CRC error -- the transport
error is always trapped by SCSI and cannot result in continued
processing of an IO (that goes on to have CRC error)?

Thanks,
Mike

> 
> > diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> > index a550a05..aa4e570 100644
> > --- a/drivers/md/dm-mpath.c
> > +++ b/drivers/md/dm-mpath.c
> > @@ -1290,7 +1290,7 @@ static int do_end_io(struct multipath *m, struct request *clone,
> >  	if (!error && !clone->errors)
> >  		return 0;	/* I/O complete */
> >  
> > -	if (error == -EOPNOTSUPP || error == -EREMOTEIO)
> > +	if (error == -EOPNOTSUPP || error == -EREMOTEIO || error == -EILSEQ)
> >  		return error;
> >  
> >  	if (mpio->pgpath)
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

Re: dm: Integrity errors should not cause failover

[Martin K. Petersen]

>>>>> "Mike" == Mike Snitzer <snitzer@redhat.com> writes:

Mike,

Mike> Alasdair and I just chatted about this patch: it will always pass
Mike> the the -EILSEQ up _without_ performing any mpath path failure.

Correct.


Mike> The newfound concern is: is there ever a benefit to failing the
Mike> path before returning -EILSEQ?

No, never.


Mike> Or will -EILSEQ always imply there is nothing wrong with the path?
Mike> E.g. a transport error cannot cause a CRC error -- the transport
Mike> error is always trapped by SCSI and cannot result in continued
Mike> processing of an IO (that goes on to have CRC error)?

-EILSEQ is only returned if a discrepancy is detected between data and
protection information. In both cases the block in question was
successfully received by either initiator or target. I.e. the path is
working fine.

-- 
Martin K. Petersen	Oracle Linux Engineering