[PING / RESEND] handling reservation conflicts in dm-mpath

[PING / RESEND] handling reservation conflicts in dm-mpath

[Christoph Hellwig]

Hannes sent this patch a bit more than a year ago, but it got silently
dropped.  When using the pNFS SCSI layout we can easily hit a
failover "livelock" without it as the reservation conflicts on a
newly detected device that doesn't have layouts yet, or after a fency
will keep failing over from one path to another for no reason at all.

Any chance to get it into mainline now?

[PATCH] dm-mpath: always return reservation conflict

[Christoph Hellwig]

From: Hannes Reinecke <hare@suse.de>

If dm-mpath encounters an reservation conflict it should not fail the
path (as communication with the target is not affected) but should
rather retry on another path. However, in doing so we might be inducing
a ping-pong between paths, with no guarantee of any forward progress.

And arguably a reservation conflict is an unexpected error, so we should
be passing it upwards to allow the application to take appropriate steps.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Christoph Hellwig <hch@lst.de>
Tested-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/dm-mpath.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 7eac080..8d2f916 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1555,16 +1555,22 @@ static int do_end_io(struct multipath *m, struct request *clone,
 	if (noretry_error(error))
 		return error;
 
-	if (mpio->pgpath)
+	/*
+	 * EBADE signals an reservation conflict.
+	 * We shouldn't fail the path here as we can communicate with
+	 * the target. We should failover to the next path, but in
+	 * doing so we might be causing a ping-pong between paths.
+	 * So just return the reservation conflict error.
+	 */
+	if (error == -EBADE)
+		r = error;
+	else if (mpio->pgpath)
 		fail_path(mpio->pgpath);
 
 	if (!atomic_read(&m->nr_valid_paths)) {
 		if (!test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) {
 			if (!must_push_back_rq(m))
 				r = -EIO;
-		} else {
-			if (error == -EBADE)
-				r = error;
 		}
 	}
 
-- 
2.1.4

Re: [dm-devel] [PATCH] dm-mpath: always return reservation conflict

[Christoph Hellwig]

ping?

On Tue, Aug 02, 2016 at 02:36:32PM +0200, Christoph Hellwig wrote:
> From: Hannes Reinecke <hare@suse.de>
> 
> If dm-mpath encounters an reservation conflict it should not fail the
> path (as communication with the target is not affected) but should
> rather retry on another path. However, in doing so we might be inducing
> a ping-pong between paths, with no guarantee of any forward progress.
> 
> And arguably a reservation conflict is an unexpected error, so we should
> be passing it upwards to allow the application to take appropriate steps.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Acked-by: Christoph Hellwig <hch@lst.de>
> Tested-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/md/dm-mpath.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index 7eac080..8d2f916 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -1555,16 +1555,22 @@ static int do_end_io(struct multipath *m, struct request *clone,
>  	if (noretry_error(error))
>  		return error;
>  
> -	if (mpio->pgpath)
> +	/*
> +	 * EBADE signals an reservation conflict.
> +	 * We shouldn't fail the path here as we can communicate with
> +	 * the target. We should failover to the next path, but in
> +	 * doing so we might be causing a ping-pong between paths.
> +	 * So just return the reservation conflict error.
> +	 */
> +	if (error == -EBADE)
> +		r = error;
> +	else if (mpio->pgpath)
>  		fail_path(mpio->pgpath);
>  
>  	if (!atomic_read(&m->nr_valid_paths)) {
>  		if (!test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) {
>  			if (!must_push_back_rq(m))
>  				r = -EIO;
> -		} else {
> -			if (error == -EBADE)
> -				r = error;
>  		}
>  	}
>  
> -- 
> 2.1.4
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
---end quoted text---

Re: dm-mpath: always return reservation conflict

[Mike Snitzer]

Not sure how Hannes' original patch was overlooked but...

One issue I see with the patch is it will return -EBADE regardless of
whether 'queue_if_no_path' is set.  That's fine (since path isn't being
failed for this case any more).  But why not just return error
immediately?

But taking a step back, shouldn't all paths be tried once before
returning an error?  Obviously that'd impose the use of a new
'conflict_seen' (or whatever) flag at the end of 'struct pgpath'.  And
then only return error if the flag is set.

I threw together the following RFC patch to illustrate what I'm
thinking, but thinking about this further it is tough to know all paths
have seen the reservation conflict (my patch assumes if 'conflict_seen'
is set then the conflict iterated through all paths.. but if paths
aren't being failed there isn't a guarantee that the path selector
didn't just hand us back the same path that just experienced the
conflict).  So this is throw-away for now (and I'll get Hannes' patch
applied for 4.8-rc3, with the tweak of returning -EBADE immediately): 

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index ac734e5..c3d92db 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -41,6 +41,7 @@ struct pgpath {
 	struct delayed_work activate_path;
 
 	bool is_active:1;		/* Path status */
+	bool conflict_seen:1;
 };
 
 #define path_to_pgpath(__pgp) container_of((__pgp), struct pgpath, path)
@@ -1569,6 +1570,33 @@ static int do_end_io(struct multipath *m, struct request *clone,
 	if (noretry_error(error))
 		return error;
 
+	if (error == -EBADE) {
+		/*
+		 * EBADE signals a reservation conflict.
+		 * We shouldn't fail the path here as we can communicate with
+		 * the target. We should failover to the next path, but in
+		 * doing so we might be causing a ping-pong between paths.
+		 * Avoid this by only returning the reservation conflict error
+		 * if a conflict has been seen on all paths.
+		 */
+		if (!mpio->pgpath || mpio->pgpath->conflict_seen) {
+			struct priority_group *pg;
+			struct pgpath *p;
+
+			/* clear 'conflict_seen' for all pgpaths */
+			list_for_each_entry(pg, &m->priority_groups, list) {
+				list_for_each_entry(p, &pg->pgpaths, list) {
+					p->conflict_seen = false;
+				}
+			}
+			return error;
+		}
+		else if (mpio->pgpath) {
+			mpio->pgpath->conflict_seen = true;
+			return r;
+		}
+	}
+
 	if (mpio->pgpath)
 		fail_path(mpio->pgpath);
 
@@ -1576,9 +1604,6 @@ static int do_end_io(struct multipath *m, struct request *clone,
 		if (!test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) {
 			if (!must_push_back_rq(m))
 				r = -EIO;
-		} else {
-			if (error == -EBADE)
-				r = error;
 		}
 	}
 

Re: dm-mpath: always return reservation conflict

[Mike Snitzer]

On Mon, Aug 15 2016 at  9:08P -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> Not sure how Hannes' original patch was overlooked but...

It wasn't overlooked.  It was very much unresolved.  The original thread
unraveled to all sorts of PR edge case concerns (and doubt about whether
anything relies on the current multipath handling of reservation
conflicts).  See patchwork thread below.

Obviously you have found a problematic case which requires Hannes'
patch.  So there is definitely increased pressure to fix this.

> One issue I see with the patch is it will return -EBADE regardless of
> whether 'queue_if_no_path' is set.  That's fine (since path isn't being
> failed for this case any more).  But why not just return error
> immediately?
> 
> But taking a step back, shouldn't all paths be tried once before
> returning an error?  Obviously that'd impose the use of a new
> 'conflict_seen' (or whatever) flag at the end of 'struct pgpath'.  And
> then only return error if the flag is set.
> 
> I threw together the following RFC patch to illustrate what I'm
> thinking, but thinking about this further it is tough to know all paths
> have seen the reservation conflict (my patch assumes if 'conflict_seen'
> is set then the conflict iterated through all paths.. but if paths
> aren't being failed there isn't a guarantee that the path selector
> didn't just hand us back the same path that just experienced the
> conflict).

Seems we still need a more sophisticated approach.  But I'm left
wondering: if we didn't do it would anything notice?  Sadly, the same
big question from the original thread from a year ago:

https://patchwork.kernel.org/patch/6797111/

> So this is throw-away for now (and I'll get Hannes' patch applied for
> 4.8-rc3, with the tweak of returning -EBADE immediately):

Unfortunately, I'm _not_ staging Hannes' patch until I have James
Bottomley's Ack (given his original issues with the patch haven't been
explained away AFAICT).

Mike

Re: [dm-devel] dm-mpath: always return reservation conflict

[Christoph Hellwig]

Getting back to this after Hannes recovered from his vacation
and I had a chat with him..

On Mon, Aug 15, 2016 at 09:40:39AM -0400, Mike Snitzer wrote:
> Seems we still need a more sophisticated approach.  But I'm left
> wondering: if we didn't do it would anything notice?  Sadly, the same
> big question from the original thread from a year ago:

Yes.  I have a customer looking to push the pNFS SCSI layout into
a product, and the major show stopper right now is that we can
trivially get into failver loops without this (or and equivalent)
fix.

A year ago SCSI layout was still work in progress in the IETF,
people use the similar block layout instead that doesn't use
PRs and we also didn't have the in-kernel PR API, so you effectively
couldn't use PRs with multipathing.

> https://patchwork.kernel.org/patch/6797111/
> 
> > So this is throw-away for now (and I'll get Hannes' patch applied for
> > 4.8-rc3, with the tweak of returning -EBADE immediately):
> 
> Unfortunately, I'm _not_ staging Hannes' patch until I have James
> Bottomley's Ack (given his original issues with the patch haven't been
> explained away AFAICT).

I've added James to the Cc.  His argument was that the old behavior
could be implemented to use some non-standard use of reservations
without a specific example.  I don't really think his example even
is practical - once we use dm-mpath it exclusively claims the underling
block devices, so any sort of selective reservations would have had
to happen before even starting dm-multipath.  So a dynamic SAN
controller would have to tear down and rebuild the dm-multipath setup
at all the time.

Re: [dm-devel] dm-mpath: always return reservation conflict

[James Bottomley]

On Mon, 2016-09-26 at 09:52 -0700, Christoph Hellwig wrote:
> Getting back to this after Hannes recovered from his vacation
> and I had a chat with him..
> 
> On Mon, Aug 15, 2016 at 09:40:39AM -0400, Mike Snitzer wrote:
> > Seems we still need a more sophisticated approach.  But I'm left
> > wondering: if we didn't do it would anything notice?  Sadly, the
> > same
> > big question from the original thread from a year ago:
> 
> Yes.  I have a customer looking to push the pNFS SCSI layout into
> a product, and the major show stopper right now is that we can
> trivially get into failver loops without this (or and equivalent)
> fix.
> 
> A year ago SCSI layout was still work in progress in the IETF,
> people use the similar block layout instead that doesn't use
> PRs and we also didn't have the in-kernel PR API, so you effectively
> couldn't use PRs with multipathing.
> 
> > https://patchwork.kernel.org/patch/6797111/
> > 
> > > So this is throw-away for now (and I'll get Hannes' patch applied
> > > for
> > > 4.8-rc3, with the tweak of returning -EBADE immediately):
> > 
> > Unfortunately, I'm _not_ staging Hannes' patch until I have James
> > Bottomley's Ack (given his original issues with the patch haven't
> > been
> > explained away AFAICT).
> 
> I've added James to the Cc.  His argument was that the old behavior
> could be implemented to use some non-standard use of reservations
> without a specific example.  I don't really think his example even
> is practical - once we use dm-mpath it exclusively claims the 
> underling block devices, so any sort of selective reservations would 
> have had to happen before even starting dm-multipath.

Well, now that you've made me reread the thread from 14 months ago that
wasn't quite my objection.  The objection hinged on the fact that
anything that uses path specific reservations would now fail instead of
retrying on a different path.  I thought the IBM SVC did this and
Hannes implied he'd be able to check this ... did anyone check?  If
we've checked and there's no issue with the SVC, then I don't have any
other objections.

>   So a dynamic SAN controller would have to tear down and rebuild the
> dm-multipath setup at all the time.

That was the job of the SVC: it sat in the middle of the SAN and
controlled which node saw what storage.

https://www.ibm.com/support/knowledgecenter/STPVGU/com.ibm.storage.svc.console.720.doc/svc_svcovr_1bcfiq.html

The SVC can issue its own reservations in those circumstances.  What
I'm not at all clear on is whether they'll interact badly with the dm
-mp reservations.

James

Re: [dm-devel] dm-mpath: always return reservation conflict

[Hannes Reinecke]

On 09/26/2016 09:06 PM, James Bottomley wrote:
> On Mon, 2016-09-26 at 09:52 -0700, Christoph Hellwig wrote:
>> Getting back to this after Hannes recovered from his vacation
>> and I had a chat with him..
>>
>> On Mon, Aug 15, 2016 at 09:40:39AM -0400, Mike Snitzer wrote:
>>> Seems we still need a more sophisticated approach.  But I'm left
>>> wondering: if we didn't do it would anything notice?  Sadly, the
>>> same
>>> big question from the original thread from a year ago:
>>
>> Yes.  I have a customer looking to push the pNFS SCSI layout into
>> a product, and the major show stopper right now is that we can
>> trivially get into failver loops without this (or and equivalent)
>> fix.
>>
>> A year ago SCSI layout was still work in progress in the IETF,
>> people use the similar block layout instead that doesn't use
>> PRs and we also didn't have the in-kernel PR API, so you effectively
>> couldn't use PRs with multipathing.
>>
>>> https://patchwork.kernel.org/patch/6797111/
>>>
>>>> So this is throw-away for now (and I'll get Hannes' patch applied
>>>> for
>>>> 4.8-rc3, with the tweak of returning -EBADE immediately):
>>>
>>> Unfortunately, I'm _not_ staging Hannes' patch until I have James
>>> Bottomley's Ack (given his original issues with the patch haven't
>>> been
>>> explained away AFAICT).
>>
>> I've added James to the Cc.  His argument was that the old behavior
>> could be implemented to use some non-standard use of reservations
>> without a specific example.  I don't really think his example even
>> is practical - once we use dm-mpath it exclusively claims the 
>> underling block devices, so any sort of selective reservations would 
>> have had to happen before even starting dm-multipath.
> 
> Well, now that you've made me reread the thread from 14 months ago that
> wasn't quite my objection.  The objection hinged on the fact that
> anything that uses path specific reservations would now fail instead of
> retrying on a different path.  I thought the IBM SVC did this and
> Hannes implied he'd be able to check this ... did anyone check?  If
> we've checked and there's no issue with the SVC, then I don't have any
> other objections.
> 
>>   So a dynamic SAN controller would have to tear down and rebuild the
>> dm-multipath setup at all the time.
> 
> That was the job of the SVC: it sat in the middle of the SAN and
> controlled which node saw what storage.
> 
> https://www.ibm.com/support/knowledgecenter/STPVGU/com.ibm.storage.svc.console.720.doc/svc_svcovr_1bcfiq.html
> 
> The SVC can issue its own reservations in those circumstances.  What
> I'm not at all clear on is whether they'll interact badly with the dm
> -mp reservations.
> 
In the end SVC is (for us) just another storage array.
If and what SVC does in the background is of no interest to us.
OTOH I'd be very surprised if the SVC would be allowing us to see
remnants of its internal working (like persistent reservation errors);
in doing so third-party applications would be able to see and possibly
modify these persistent reservations and the SVC would find itself in a
very fragile operating scenario.
Also interactions with GPFS (which uses it's own set of reservations)
will become very tricky.

So I sincerely doubt we'll ever see SVC-originated persistent
reservations errors.

And as a side-note, this particular patch is included in SLES since
2011. With no noticeable side-effect.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [dm-devel] dm-mpath: always return reservation conflict

[James Bottomley]

On Tue, 2016-09-27 at 08:34 +0200, Hannes Reinecke wrote:
> On 09/26/2016 09:06 PM, James Bottomley wrote:
> > On Mon, 2016-09-26 at 09:52 -0700, Christoph Hellwig wrote:
> > > Getting back to this after Hannes recovered from his vacation
> > > and I had a chat with him..
> > > 
> > > On Mon, Aug 15, 2016 at 09:40:39AM -0400, Mike Snitzer wrote:
> > > > Seems we still need a more sophisticated approach.  But I'm 
> > > > left wondering: if we didn't do it would anything notice? 
> > > >  Sadly, the same big question from the original thread from a
> > > > year ago:
> > > 
> > > Yes.  I have a customer looking to push the pNFS SCSI layout into
> > > a product, and the major show stopper right now is that we can
> > > trivially get into failver loops without this (or and equivalent)
> > > fix.
> > > 
> > > A year ago SCSI layout was still work in progress in the IETF,
> > > people use the similar block layout instead that doesn't use
> > > PRs and we also didn't have the in-kernel PR API, so you 
> > > effectively couldn't use PRs with multipathing.
> > > 
> > > > https://patchwork.kernel.org/patch/6797111/
> > > > 
> > > > > So this is throw-away for now (and I'll get Hannes' patch 
> > > > > applied for 4.8-rc3, with the tweak of returning -EBADE
> > > > > immediately):
> > > > 
> > > > Unfortunately, I'm _not_ staging Hannes' patch until I have 
> > > > James Bottomley's Ack (given his original issues with the patch
> > > > haven't been explained away AFAICT).
> > > 
> > > I've added James to the Cc.  His argument was that the old 
> > > behavior could be implemented to use some non-standard use of 
> > > reservations without a specific example.  I don't really think 
> > > his example even is practical - once we use dm-mpath it 
> > > exclusively claims the underling block devices, so any sort of 
> > > selective reservations would have had to happen before even
> > > starting dm-multipath.
> > 
> > Well, now that you've made me reread the thread from 14 months ago 
> > that wasn't quite my objection.  The objection hinged on the fact 
> > that anything that uses path specific reservations would now fail
> > instead of retrying on a different path.  I thought the IBM SVC did 
> > this and Hannes implied he'd be able to check this ... did anyone 
> > check?  If we've checked and there's no issue with the SVC, then I 
> > don't have any other objections.
> > 
> > >   So a dynamic SAN controller would have to tear down and rebuild 
> > > the dm-multipath setup at all the time.
> > 
> > That was the job of the SVC: it sat in the middle of the SAN and
> > controlled which node saw what storage.
> > 
> > https://www.ibm.com/support/knowledgecenter/STPVGU/com.ibm.storage.
> > svc.console.720.doc/svc_svcovr_1bcfiq.html
> > 
> > The SVC can issue its own reservations in those circumstances. 
> >  What I'm not at all clear on is whether they'll interact badly 
> > with the dm-mp reservations.
> > 
> In the end SVC is (for us) just another storage array.
> If and what SVC does in the background is of no interest to us.

How can that be true?  It sits *on* the san and manages devices, it
doesn't sit between the initators and the devices.  It applies
reservations to devices under management, but every node usually sees
everything else, so devices under SVC management are visible to all
initators unless you zone them off.

The last SVC manual I saw included a procedure for manually releasing
stuck SVC reservations from an initator, which illustrates the
expectation.

> OTOH I'd be very surprised if the SVC would be allowing us to see
> remnants of its internal working (like persistent reservation 
> errors); in doing so third-party applications would be able to see 
> and possibly modify these persistent reservations and the SVC would 
> find itself in a very fragile operating scenario.

Because unless you zone the fibre, that's precisely what you do see.

> Also interactions with GPFS (which uses it's own set of reservations)
> will become very tricky.
> 
> So I sincerely doubt we'll ever see SVC-originated persistent
> reservations errors.
> 
> And as a side-note, this particular patch is included in SLES since
> 2011. With no noticeable side-effect.

OK, so can you actually say that someone has tested this scenario?  If
not, do you have the capacity to test it?

James

Re: dm-mpath: always return reservation conflict

[Mike Snitzer]

On Tue, Sep 27 2016 at  2:50pm -0400,
James Bottomley <James.Bottomley@hansenpartnership.com> wrote:

> On Tue, 2016-09-27 at 08:34 +0200, Hannes Reinecke wrote:
> > On 09/26/2016 09:06 PM, James Bottomley wrote:
> > > On Mon, 2016-09-26 at 09:52 -0700, Christoph Hellwig wrote:
> > > > Getting back to this after Hannes recovered from his vacation
> > > > and I had a chat with him..
> > > > 
> > > > On Mon, Aug 15, 2016 at 09:40:39AM -0400, Mike Snitzer wrote:
> > > > > Seems we still need a more sophisticated approach.  But I'm 
> > > > > left wondering: if we didn't do it would anything notice? 
> > > > >  Sadly, the same big question from the original thread from a
> > > > > year ago:
> > > > 
> > > > Yes.  I have a customer looking to push the pNFS SCSI layout into
> > > > a product, and the major show stopper right now is that we can
> > > > trivially get into failver loops without this (or and equivalent)
> > > > fix.
> > > > 
> > > > A year ago SCSI layout was still work in progress in the IETF,
> > > > people use the similar block layout instead that doesn't use
> > > > PRs and we also didn't have the in-kernel PR API, so you 
> > > > effectively couldn't use PRs with multipathing.
> > > > 
> > > > > https://patchwork.kernel.org/patch/6797111/
> > > > > 
> > > > > > So this is throw-away for now (and I'll get Hannes' patch 
> > > > > > applied for 4.8-rc3, with the tweak of returning -EBADE
> > > > > > immediately):
> > > > > 
> > > > > Unfortunately, I'm _not_ staging Hannes' patch until I have 
> > > > > James Bottomley's Ack (given his original issues with the patch
> > > > > haven't been explained away AFAICT).
> > > > 
> > > > I've added James to the Cc.  His argument was that the old 
> > > > behavior could be implemented to use some non-standard use of 
> > > > reservations without a specific example.  I don't really think 
> > > > his example even is practical - once we use dm-mpath it 
> > > > exclusively claims the underling block devices, so any sort of 
> > > > selective reservations would have had to happen before even
> > > > starting dm-multipath.
> > > 
> > > Well, now that you've made me reread the thread from 14 months ago 
> > > that wasn't quite my objection.  The objection hinged on the fact 
> > > that anything that uses path specific reservations would now fail
> > > instead of retrying on a different path.  I thought the IBM SVC did 
> > > this and Hannes implied he'd be able to check this ... did anyone 
> > > check?  If we've checked and there's no issue with the SVC, then I 
> > > don't have any other objections.
> > > 
> > > >   So a dynamic SAN controller would have to tear down and rebuild 
> > > > the dm-multipath setup at all the time.
> > > 
> > > That was the job of the SVC: it sat in the middle of the SAN and
> > > controlled which node saw what storage.
> > > 
> > > https://www.ibm.com/support/knowledgecenter/STPVGU/com.ibm.storage.
> > > svc.console.720.doc/svc_svcovr_1bcfiq.html
> > > 
> > > The SVC can issue its own reservations in those circumstances. 
> > >  What I'm not at all clear on is whether they'll interact badly 
> > > with the dm-mp reservations.
> > > 
> > In the end SVC is (for us) just another storage array.
> > If and what SVC does in the background is of no interest to us.
> 
> How can that be true?  It sits *on* the san and manages devices, it
> doesn't sit between the initators and the devices.  It applies
> reservations to devices under management, but every node usually sees
> everything else, so devices under SVC management are visible to all
> initators unless you zone them off.
> 
> The last SVC manual I saw included a procedure for manually releasing
> stuck SVC reservations from an initator, which illustrates the
> expectation.
> 
> > OTOH I'd be very surprised if the SVC would be allowing us to see
> > remnants of its internal working (like persistent reservation 
> > errors); in doing so third-party applications would be able to see 
> > and possibly modify these persistent reservations and the SVC would 
> > find itself in a very fragile operating scenario.
> 
> Because unless you zone the fibre, that's precisely what you do see.
> 
> > Also interactions with GPFS (which uses it's own set of reservations)
> > will become very tricky.
> > 
> > So I sincerely doubt we'll ever see SVC-originated persistent
> > reservations errors.
> > 
> > And as a side-note, this particular patch is included in SLES since
> > 2011. With no noticeable side-effect.
> 
> OK, so can you actually say that someone has tested this scenario?  If
> not, do you have the capacity to test it?

I've elected to just take this change for 4.9.  Please see:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.9&id=8ff232c1a819c2e98d85974a3bff0b7b8e2970ed

Re: dm-mpath: always return reservation conflict

[Christoph Hellwig]

On Thu, Sep 29, 2016 at 11:01:33AM -0400, Mike Snitzer wrote:
> I've elected to just take this change for 4.9.  Please see:
> https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.9&id=8ff232c1a819c2e98d85974a3bff0b7b8e2970ed

Thanks Mike.

If any problems show up I will send you an incremental patch that limits
this behavior to devices where we created reservations using the kernel
pr_ops API.

Re: dm-mpath: always return reservation conflict

[James Bottomley]

On Thu, 2016-09-29 at 11:01 -0400, Mike Snitzer wrote:
> On Tue, Sep 27 2016 at  2:50pm -0400,
> James Bottomley <James.Bottomley@hansenpartnership.com> wrote:
> 
> > On Tue, 2016-09-27 at 08:34 +0200, Hannes Reinecke wrote:
> > > On 09/26/2016 09:06 PM, James Bottomley wrote:
> > > > On Mon, 2016-09-26 at 09:52 -0700, Christoph Hellwig wrote:
> > > > > Getting back to this after Hannes recovered from his vacation
> > > > > and I had a chat with him..
> > > > > 
> > > > > On Mon, Aug 15, 2016 at 09:40:39AM -0400, Mike Snitzer wrote:
> > > > > > Seems we still need a more sophisticated approach.  But I'm
> > > > > > left wondering: if we didn't do it would anything notice? 
> > > > > >  Sadly, the same big question from the original thread from
> > > > > > a
> > > > > > year ago:
> > > > > 
> > > > > Yes.  I have a customer looking to push the pNFS SCSI layout
> > > > > into
> > > > > a product, and the major show stopper right now is that we
> > > > > can
> > > > > trivially get into failver loops without this (or and
> > > > > equivalent)
> > > > > fix.
> > > > > 
> > > > > A year ago SCSI layout was still work in progress in the
> > > > > IETF,
> > > > > people use the similar block layout instead that doesn't use
> > > > > PRs and we also didn't have the in-kernel PR API, so you 
> > > > > effectively couldn't use PRs with multipathing.
> > > > > 
> > > > > > https://patchwork.kernel.org/patch/6797111/
> > > > > > 
> > > > > > > So this is throw-away for now (and I'll get Hannes' patch
> > > > > > > applied for 4.8-rc3, with the tweak of returning -EBADE
> > > > > > > immediately):
> > > > > > 
> > > > > > Unfortunately, I'm _not_ staging Hannes' patch until I have
> > > > > > James Bottomley's Ack (given his original issues with the
> > > > > > patch
> > > > > > haven't been explained away AFAICT).
> > > > > 
> > > > > I've added James to the Cc.  His argument was that the old 
> > > > > behavior could be implemented to use some non-standard use of
> > > > > reservations without a specific example.  I don't really
> > > > > think 
> > > > > his example even is practical - once we use dm-mpath it 
> > > > > exclusively claims the underling block devices, so any sort
> > > > > of 
> > > > > selective reservations would have had to happen before even
> > > > > starting dm-multipath.
> > > > 
> > > > Well, now that you've made me reread the thread from 14 months
> > > > ago 
> > > > that wasn't quite my objection.  The objection hinged on the
> > > > fact 
> > > > that anything that uses path specific reservations would now
> > > > fail
> > > > instead of retrying on a different path.  I thought the IBM SVC
> > > > did 
> > > > this and Hannes implied he'd be able to check this ... did
> > > > anyone 
> > > > check?  If we've checked and there's no issue with the SVC,
> > > > then I 
> > > > don't have any other objections.
> > > > 
> > > > >   So a dynamic SAN controller would have to tear down and
> > > > > rebuild 
> > > > > the dm-multipath setup at all the time.
> > > > 
> > > > That was the job of the SVC: it sat in the middle of the SAN
> > > > and
> > > > controlled which node saw what storage.
> > > > 
> > > > https://www.ibm.com/support/knowledgecenter/STPVGU/com.ibm.stor
> > > > age.
> > > > svc.console.720.doc/svc_svcovr_1bcfiq.html
> > > > 
> > > > The SVC can issue its own reservations in those circumstances. 
> > > >  What I'm not at all clear on is whether they'll interact badly
> > > > with the dm-mp reservations.
> > > > 
> > > In the end SVC is (for us) just another storage array.
> > > If and what SVC does in the background is of no interest to us.
> > 
> > How can that be true?  It sits *on* the san and manages devices, it
> > doesn't sit between the initators and the devices.  It applies
> > reservations to devices under management, but every node usually
> > sees
> > everything else, so devices under SVC management are visible to all
> > initators unless you zone them off.
> > 
> > The last SVC manual I saw included a procedure for manually
> > releasing
> > stuck SVC reservations from an initator, which illustrates the
> > expectation.
> > 
> > > OTOH I'd be very surprised if the SVC would be allowing us to see
> > > remnants of its internal working (like persistent reservation 
> > > errors); in doing so third-party applications would be able to
> > > see 
> > > and possibly modify these persistent reservations and the SVC
> > > would 
> > > find itself in a very fragile operating scenario.
> > 
> > Because unless you zone the fibre, that's precisely what you do
> > see.
> > 
> > > Also interactions with GPFS (which uses it's own set of
> > > reservations)
> > > will become very tricky.
> > > 
> > > So I sincerely doubt we'll ever see SVC-originated persistent
> > > reservations errors.
> > > 
> > > And as a side-note, this particular patch is included in SLES
> > > since
> > > 2011. With no noticeable side-effect.
> > 
> > OK, so can you actually say that someone has tested this scenario? 
> >  If not, do you have the capacity to test it?
> 
> I've elected to just take this change for 4.9.  Please see:
> https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.g
> it/commit/?h=dm-4.9&id=8ff232c1a819c2e98d85974a3bff0b7b8e2970ed

That's fine.  I think the answer is that SVC technology is not around
much so it can't be tested, so I was going to dump it on you to make
the call anyway ...

James