Re: Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE

Re: Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE

[Laurence Oberman]

Hello

This has been a tough problem to chase down but was finally reproduced.
This issue is apparent on RHEL kernels and upstream so justified reporting here.

Its out there and some may not be aware its even happening other than very slow performance using ixgbe and software FCOE on large configurations.

Upstream Kernel used for reproducing is 4.8.0

I/O performance was noted to be very impacted on a large NUMA test system (64 CPUS 4 NUMA nodes) running the software fcoe stack with Intel ixgbe interfaces.
After capturing blktraces we saw for every I/O there was at least one blk_requeue_request and sometimes hundreds or more.
This resulted in IOPS rates being marginal at best with queuing and high wait times.
After narrowing this down with systemtap and trace-cmd we added further debug and it was apparent this was dues to SCSI_MLQUEUE_HOST_BUSY being returned.
So I/O passes but very slowly as it constantly having to be requeued.

The identical configuration in our lab with a single NUMA node and 4 CPUS does not see this issue at all.
The same large system that reproduces this was booted with numa=off and still sees the issue.

The flow is as follows:

>From with fc_queuecommand
          fc_fcp_pkt_send() calls fc_fcp_cmd_send() calls tt.exch_seq_send() which calls fc_exch_seq_send

this fails and returns NULL in fc_exch_alloc() as the list traveral never creates a match.

static struct fc_seq *fc_exch_seq_send(struct fc_lport *lport,
				       struct fc_frame *fp,
				       void (*resp)(struct fc_seq *,
						    struct fc_frame *fp,
						    void *arg),
				       void (*destructor)(struct fc_seq *,
							  void *),
				       void *arg, u32 timer_msec)
{
	struct fc_exch *ep;
	struct fc_seq *sp = NULL;
	struct fc_frame_header *fh;
	struct fc_fcp_pkt *fsp = NULL;
	int rc = 1;

	ep = fc_exch_alloc(lport, fp);     ***** Called Here and fails
	if (!ep) {
		fc_frame_free(fp);
		printk("RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep = %p\n",ep);
		return NULL;
	}
..
..
]


 fc_exch_alloc() - Allocate an exchange from an EM on a
 *	/**
 *	     local port's list of EMs.
 * @lport: The local port that will own the exchange
 * @fp:	   The FC frame that the exchange will be for
 *
 * This function walks the list of exchange manager(EM)
 * anchors to select an EM for a new exchange allocation. The
 * EM is selected when a NULL match function pointer is encountered
 * or when a call to a match function returns true.
 */
static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
					    struct fc_frame *fp)
{
	struct fc_exch_mgr_anchor *ema;

	list_for_each_entry(ema, &lport->ema_list, ema_list)
		if (!ema->match || ema->match(fp))
			return fc_exch_em_alloc(lport, ema->mp);
	return NULL;                                 ***** Never matches so returns NULL
}


RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep = (null)
RHDEBUG: rc -1 with !seq = (null) after calling tt.exch_seq_send  within fc_fcp_cmd_send
RHDEBUG: rc non zero in :unlock within fc_fcp_cmd_send = -1
RHDEBUG: In fc_fcp_pkt_send, we returned from  rc = lport->tt.fcp_cmd_send with rc = -1

RHDEBUG: We hit SCSI_MLQUEUE_HOST_BUSY in fc_queuecommand with rval in fc_fcp_pkt_send=-1

I am trying to get my head around why a large multi-node system sees this issue even with NUMA disabled.
Has anybody seen this or is aware of this with configurations (using fc_queuecommand)

I am continuing to add debug to narrow this down.

Thanks
Laurence 

Re: Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE

[Hannes Reinecke]

On 10/08/2016 02:57 PM, Laurence Oberman wrote:
> Hello
>
> This has been a tough problem to chase down but was finally reproduced.
> This issue is apparent on RHEL kernels and upstream so justified reporting here.
>
> Its out there and some may not be aware its even happening other than very slow
 > performance using ixgbe and software FCOE on large configurations.
>
> Upstream Kernel used for reproducing is 4.8.0
>
> I/O performance was noted to be very impacted on a large NUMA test system (64 CPUS 4 NUMA nodes) running the software fcoe stack with Intel ixgbe interfaces.
> After capturing blktraces we saw for every I/O there was at least one blk_requeue_request and sometimes hundreds or more.
> This resulted in IOPS rates being marginal at best with queuing and high wait times.
> After narrowing this down with systemtap and trace-cmd we added further debug and it was apparent this was dues to SCSI_MLQUEUE_HOST_BUSY being returned.
> So I/O passes but very slowly as it constantly having to be requeued.
>
> The identical configuration in our lab with a single NUMA node and 4 CPUS does not see this issue at all.
> The same large system that reproduces this was booted with numa=off and still sees the issue.
>
Have you tested with my FCoE fixes?
I've done quite some fixes for libfc/fcoe, and it would be nice to see 
how the patches behave with this setup.

> The flow is as follows:
>
> From with fc_queuecommand
>           fc_fcp_pkt_send() calls fc_fcp_cmd_send() calls tt.exch_seq_send() which calls fc_exch_seq_send
>
> this fails and returns NULL in fc_exch_alloc() as the list traveral never creates a match.
>
> static struct fc_seq *fc_exch_seq_send(struct fc_lport *lport,
> 				       struct fc_frame *fp,
> 				       void (*resp)(struct fc_seq *,
> 						    struct fc_frame *fp,
> 						    void *arg),
> 				       void (*destructor)(struct fc_seq *,
> 							  void *),
> 				       void *arg, u32 timer_msec)
> {
> 	struct fc_exch *ep;
> 	struct fc_seq *sp = NULL;
> 	struct fc_frame_header *fh;
> 	struct fc_fcp_pkt *fsp = NULL;
> 	int rc = 1;
>
> 	ep = fc_exch_alloc(lport, fp);     ***** Called Here and fails
> 	if (!ep) {
> 		fc_frame_free(fp);
> 		printk("RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep = %p\n",ep);
> 		return NULL;
> 	}
> ..
> ..
> ]
>
>
>  fc_exch_alloc() - Allocate an exchange from an EM on a
>  *	/**
>  *	     local port's list of EMs.
>  * @lport: The local port that will own the exchange
>  * @fp:	   The FC frame that the exchange will be for
>  *
>  * This function walks the list of exchange manager(EM)
>  * anchors to select an EM for a new exchange allocation. The
>  * EM is selected when a NULL match function pointer is encountered
>  * or when a call to a match function returns true.
>  */
> static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
> 					    struct fc_frame *fp)
> {
> 	struct fc_exch_mgr_anchor *ema;
>
> 	list_for_each_entry(ema, &lport->ema_list, ema_list)
> 		if (!ema->match || ema->match(fp))
> 			return fc_exch_em_alloc(lport, ema->mp);
> 	return NULL;                                 ***** Never matches so returns NULL
> }
>
>
> RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep = (null)
> RHDEBUG: rc -1 with !seq = (null) after calling tt.exch_seq_send  within fc_fcp_cmd_send
> RHDEBUG: rc non zero in :unlock within fc_fcp_cmd_send = -1
> RHDEBUG: In fc_fcp_pkt_send, we returned from  rc = lport->tt.fcp_cmd_send with rc = -1
>
> RHDEBUG: We hit SCSI_MLQUEUE_HOST_BUSY in fc_queuecommand with rval in fc_fcp_pkt_send=-1
>
> I am trying to get my head around why a large multi-node system sees this issue even with NUMA disabled.
> Has anybody seen this or is aware of this with configurations (using fc_queuecommand)
>
> I am continuing to add debug to narrow this down.
>
You might actually be hitting a limitation in the exchange manager code.
The libfc exchange manager tries to be really clever and will assign a 
per-cpu exchange manager (probably to increase locality). However, we 
only have a limited number of exchanges, so on large systems we might 
actually run into a exchange starvation problem, where we have in theory 
enough free exchanges, but none for the submitting cpu.

(Personally, the exchange manager code is in urgent need of reworking.
It should be replaced by the sbitmap code from Omar).

Do check how many free exchanges are actually present for the stalling 
CPU; it might be that you run into a starvation issue.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare-l3A5Bk7waGM@public.gmane.org			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE

[Laurence Oberman]

----- Original Message -----
> From: "Hannes Reinecke" <hare@suse.de>
> To: "Laurence Oberman" <loberman@redhat.com>, "Linux SCSI Mailinglist" <linux-scsi@vger.kernel.org>,
> fcoe-devel@open-fcoe.org
> Cc: "Curtis Taylor (cjt@us.ibm.com)" <cjt@us.ibm.com>, "Bud Brown" <bubrown@redhat.com>
> Sent: Saturday, October 8, 2016 1:35:19 PM
> Subject: Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large
> configurations with Intel ixgbe running FCOE
> 
> On 10/08/2016 02:57 PM, Laurence Oberman wrote:
> > Hello
> >
> > This has been a tough problem to chase down but was finally reproduced.
> > This issue is apparent on RHEL kernels and upstream so justified reporting
> > here.
> >
> > Its out there and some may not be aware its even happening other than very
> > slow
>  > performance using ixgbe and software FCOE on large configurations.
> >
> > Upstream Kernel used for reproducing is 4.8.0
> >
> > I/O performance was noted to be very impacted on a large NUMA test system
> > (64 CPUS 4 NUMA nodes) running the software fcoe stack with Intel ixgbe
> > interfaces.
> > After capturing blktraces we saw for every I/O there was at least one
> > blk_requeue_request and sometimes hundreds or more.
> > This resulted in IOPS rates being marginal at best with queuing and high
> > wait times.
> > After narrowing this down with systemtap and trace-cmd we added further
> > debug and it was apparent this was dues to SCSI_MLQUEUE_HOST_BUSY being
> > returned.
> > So I/O passes but very slowly as it constantly having to be requeued.
> >
> > The identical configuration in our lab with a single NUMA node and 4 CPUS
> > does not see this issue at all.
> > The same large system that reproduces this was booted with numa=off and
> > still sees the issue.
> >
> Have you tested with my FCoE fixes?
> I've done quite some fixes for libfc/fcoe, and it would be nice to see
> how the patches behave with this setup.
> 
> > The flow is as follows:
> >
> > From with fc_queuecommand
> >           fc_fcp_pkt_send() calls fc_fcp_cmd_send() calls
> >           tt.exch_seq_send() which calls fc_exch_seq_send
> >
> > this fails and returns NULL in fc_exch_alloc() as the list traveral never
> > creates a match.
> >
> > static struct fc_seq *fc_exch_seq_send(struct fc_lport *lport,
> > 				       struct fc_frame *fp,
> > 				       void (*resp)(struct fc_seq *,
> > 						    struct fc_frame *fp,
> > 						    void *arg),
> > 				       void (*destructor)(struct fc_seq *,
> > 							  void *),
> > 				       void *arg, u32 timer_msec)
> > {
> > 	struct fc_exch *ep;
> > 	struct fc_seq *sp = NULL;
> > 	struct fc_frame_header *fh;
> > 	struct fc_fcp_pkt *fsp = NULL;
> > 	int rc = 1;
> >
> > 	ep = fc_exch_alloc(lport, fp);     ***** Called Here and fails
> > 	if (!ep) {
> > 		fc_frame_free(fp);
> > 		printk("RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep =
> > 		%p\n",ep);
> > 		return NULL;
> > 	}
> > ..
> > ..
> > ]
> >
> >
> >  fc_exch_alloc() - Allocate an exchange from an EM on a
> >  *	/**
> >  *	     local port's list of EMs.
> >  * @lport: The local port that will own the exchange
> >  * @fp:	   The FC frame that the exchange will be for
> >  *
> >  * This function walks the list of exchange manager(EM)
> >  * anchors to select an EM for a new exchange allocation. The
> >  * EM is selected when a NULL match function pointer is encountered
> >  * or when a call to a match function returns true.
> >  */
> > static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
> > 					    struct fc_frame *fp)
> > {
> > 	struct fc_exch_mgr_anchor *ema;
> >
> > 	list_for_each_entry(ema, &lport->ema_list, ema_list)
> > 		if (!ema->match || ema->match(fp))
> > 			return fc_exch_em_alloc(lport, ema->mp);
> > 	return NULL;                                 ***** Never matches so
> > 	returns NULL
> > }
> >
> >
> > RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep = (null)
> > RHDEBUG: rc -1 with !seq = (null) after calling tt.exch_seq_send  within
> > fc_fcp_cmd_send
> > RHDEBUG: rc non zero in :unlock within fc_fcp_cmd_send = -1
> > RHDEBUG: In fc_fcp_pkt_send, we returned from  rc = lport->tt.fcp_cmd_send
> > with rc = -1
> >
> > RHDEBUG: We hit SCSI_MLQUEUE_HOST_BUSY in fc_queuecommand with rval in
> > fc_fcp_pkt_send=-1
> >
> > I am trying to get my head around why a large multi-node system sees this
> > issue even with NUMA disabled.
> > Has anybody seen this or is aware of this with configurations (using
> > fc_queuecommand)
> >
> > I am continuing to add debug to narrow this down.
> >
> You might actually be hitting a limitation in the exchange manager code.
> The libfc exchange manager tries to be really clever and will assign a
> per-cpu exchange manager (probably to increase locality). However, we
> only have a limited number of exchanges, so on large systems we might
> actually run into a exchange starvation problem, where we have in theory
> enough free exchanges, but none for the submitting cpu.
> 
> (Personally, the exchange manager code is in urgent need of reworking.
> It should be replaced by the sbitmap code from Omar).
> 
> Do check how many free exchanges are actually present for the stalling
> CPU; it might be that you run into a starvation issue.
> 
> Cheers,
> 
> Hannes
> --
> Dr. Hannes Reinecke		      zSeries & Storage
> hare@suse.de			      +49 911 74053 688
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
> 
Hi Hannes, 
Thanks for responding

I am adding additional debug as I type this.

I am using latest linux-next, I assume your latest FCOE are not in there yet.
What is puzzling here is a identical kernel with 1 numa node and only 8GB memory does not see this.
Surely if I was running out of exchanges that would show up on the smaller system as well.
I am able to get to 1500 IOPS/sec with the same I/O exerciser on the smaller system with ZERO blk_requeue_request() calls.
Again, same kernel, same ixgbe same FCOE switch etc.

I traced specifically those initially because we saw it in the blktrace.

I dont understand the match stuff going on in the list reversal stuff here very well, still trying to understand the code flow.
the bool match() I also cannot figure out from the code.
It runs the fc_exch_em_alloc() if ether bool *match is false or *match(fp)
I can't find the actual code for the match, will have to get a vmcore to find it.

 static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
 					    struct fc_frame *fp)
 {
	struct fc_exch_mgr_anchor *ema;

 	list_for_each_entry(ema, &lport->ema_list, ema_list)
 		if (!ema->match || ema->match(fp))
 			return fc_exch_em_alloc(lport, ema->mp);
 	return NULL;                                 ***** Never matches so
	returns NULL
 }

Will replay after some finer debug has been added

Again specific to fc_queuecommand and S/W FCOE, not an issue with the F/C queuecommand in the HBA templates fro example lpfc or qla2xxx and alos not applicable to full offload FCOE like the Emulex cards.
Thanks
Laurence

Re: Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE

[Laurence Oberman]

----- Original Message -----
> From: "Laurence Oberman" <loberman@redhat.com>
> To: "Hannes Reinecke" <hare@suse.de>
> Cc: "Linux SCSI Mailinglist" <linux-scsi@vger.kernel.org>, fcoe-devel@open-fcoe.org, "Curtis Taylor (cjt@us.ibm.com)"
> <cjt@us.ibm.com>, "Bud Brown" <bubrown@redhat.com>
> Sent: Saturday, October 8, 2016 1:53:01 PM
> Subject: Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large
> configurations with Intel ixgbe running FCOE
> 
> 
> 
> ----- Original Message -----
> > From: "Hannes Reinecke" <hare@suse.de>
> > To: "Laurence Oberman" <loberman@redhat.com>, "Linux SCSI Mailinglist"
> > <linux-scsi@vger.kernel.org>,
> > fcoe-devel@open-fcoe.org
> > Cc: "Curtis Taylor (cjt@us.ibm.com)" <cjt@us.ibm.com>, "Bud Brown"
> > <bubrown@redhat.com>
> > Sent: Saturday, October 8, 2016 1:35:19 PM
> > Subject: Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by
> > fc_queuecommand on NUMA or large
> > configurations with Intel ixgbe running FCOE
> > 
> > On 10/08/2016 02:57 PM, Laurence Oberman wrote:
> > > Hello
> > >
> > > This has been a tough problem to chase down but was finally reproduced.
> > > This issue is apparent on RHEL kernels and upstream so justified
> > > reporting
> > > here.
> > >
> > > Its out there and some may not be aware its even happening other than
> > > very
> > > slow
> >  > performance using ixgbe and software FCOE on large configurations.
> > >
> > > Upstream Kernel used for reproducing is 4.8.0
> > >
> > > I/O performance was noted to be very impacted on a large NUMA test system
> > > (64 CPUS 4 NUMA nodes) running the software fcoe stack with Intel ixgbe
> > > interfaces.
> > > After capturing blktraces we saw for every I/O there was at least one
> > > blk_requeue_request and sometimes hundreds or more.
> > > This resulted in IOPS rates being marginal at best with queuing and high
> > > wait times.
> > > After narrowing this down with systemtap and trace-cmd we added further
> > > debug and it was apparent this was dues to SCSI_MLQUEUE_HOST_BUSY being
> > > returned.
> > > So I/O passes but very slowly as it constantly having to be requeued.
> > >
> > > The identical configuration in our lab with a single NUMA node and 4 CPUS
> > > does not see this issue at all.
> > > The same large system that reproduces this was booted with numa=off and
> > > still sees the issue.
> > >
> > Have you tested with my FCoE fixes?
> > I've done quite some fixes for libfc/fcoe, and it would be nice to see
> > how the patches behave with this setup.
> > 
> > > The flow is as follows:
> > >
> > > From with fc_queuecommand
> > >           fc_fcp_pkt_send() calls fc_fcp_cmd_send() calls
> > >           tt.exch_seq_send() which calls fc_exch_seq_send
> > >
> > > this fails and returns NULL in fc_exch_alloc() as the list traveral never
> > > creates a match.
> > >
> > > static struct fc_seq *fc_exch_seq_send(struct fc_lport *lport,
> > > 				       struct fc_frame *fp,
> > > 				       void (*resp)(struct fc_seq *,
> > > 						    struct fc_frame *fp,
> > > 						    void *arg),
> > > 				       void (*destructor)(struct fc_seq *,
> > > 							  void *),
> > > 				       void *arg, u32 timer_msec)
> > > {
> > > 	struct fc_exch *ep;
> > > 	struct fc_seq *sp = NULL;
> > > 	struct fc_frame_header *fh;
> > > 	struct fc_fcp_pkt *fsp = NULL;
> > > 	int rc = 1;
> > >
> > > 	ep = fc_exch_alloc(lport, fp);     ***** Called Here and fails
> > > 	if (!ep) {
> > > 		fc_frame_free(fp);
> > > 		printk("RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep
> > > 		=
> > > 		%p\n",ep);
> > > 		return NULL;
> > > 	}
> > > ..
> > > ..
> > > ]
> > >
> > >
> > >  fc_exch_alloc() - Allocate an exchange from an EM on a
> > >  *	/**
> > >  *	     local port's list of EMs.
> > >  * @lport: The local port that will own the exchange
> > >  * @fp:	   The FC frame that the exchange will be for
> > >  *
> > >  * This function walks the list of exchange manager(EM)
> > >  * anchors to select an EM for a new exchange allocation. The
> > >  * EM is selected when a NULL match function pointer is encountered
> > >  * or when a call to a match function returns true.
> > >  */
> > > static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
> > > 					    struct fc_frame *fp)
> > > {
> > > 	struct fc_exch_mgr_anchor *ema;
> > >
> > > 	list_for_each_entry(ema, &lport->ema_list, ema_list)
> > > 		if (!ema->match || ema->match(fp))
> > > 			return fc_exch_em_alloc(lport, ema->mp);
> > > 	return NULL;                                 ***** Never matches so
> > > 	returns NULL
> > > }
> > >
> > >
> > > RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep = (null)
> > > RHDEBUG: rc -1 with !seq = (null) after calling tt.exch_seq_send  within
> > > fc_fcp_cmd_send
> > > RHDEBUG: rc non zero in :unlock within fc_fcp_cmd_send = -1
> > > RHDEBUG: In fc_fcp_pkt_send, we returned from  rc =
> > > lport->tt.fcp_cmd_send
> > > with rc = -1
> > >
> > > RHDEBUG: We hit SCSI_MLQUEUE_HOST_BUSY in fc_queuecommand with rval in
> > > fc_fcp_pkt_send=-1
> > >
> > > I am trying to get my head around why a large multi-node system sees this
> > > issue even with NUMA disabled.
> > > Has anybody seen this or is aware of this with configurations (using
> > > fc_queuecommand)
> > >
> > > I am continuing to add debug to narrow this down.
> > >
> > You might actually be hitting a limitation in the exchange manager code.
> > The libfc exchange manager tries to be really clever and will assign a
> > per-cpu exchange manager (probably to increase locality). However, we
> > only have a limited number of exchanges, so on large systems we might
> > actually run into a exchange starvation problem, where we have in theory
> > enough free exchanges, but none for the submitting cpu.
> > 
> > (Personally, the exchange manager code is in urgent need of reworking.
> > It should be replaced by the sbitmap code from Omar).
> > 
> > Do check how many free exchanges are actually present for the stalling
> > CPU; it might be that you run into a starvation issue.
> > 
> > Cheers,
> > 
> > Hannes
> > --
> > Dr. Hannes Reinecke		      zSeries & Storage
> > hare@suse.de			      +49 911 74053 688
> > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> > GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
> > 
> Hi Hannes,
> Thanks for responding
> 
> I am adding additional debug as I type this.
> 
> I am using latest linux-next, I assume your latest FCOE are not in there yet.
> What is puzzling here is a identical kernel with 1 numa node and only 8GB
> memory does not see this.
> Surely if I was running out of exchanges that would show up on the smaller
> system as well.
> I am able to get to 1500 IOPS/sec with the same I/O exerciser on the smaller
> system with ZERO blk_requeue_request() calls.
> Again, same kernel, same ixgbe same FCOE switch etc.
> 
> I traced specifically those initially because we saw it in the blktrace.
> 
> I dont understand the match stuff going on in the list reversal stuff here
> very well, still trying to understand the code flow.
> the bool match() I also cannot figure out from the code.
> It runs the fc_exch_em_alloc() if ether bool *match is false or *match(fp)
> I can't find the actual code for the match, will have to get a vmcore to find
> it.
> 
>  static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
>  					    struct fc_frame *fp)
>  {
> 	struct fc_exch_mgr_anchor *ema;
> 
>  	list_for_each_entry(ema, &lport->ema_list, ema_list)
>  		if (!ema->match || ema->match(fp))
>  			return fc_exch_em_alloc(lport, ema->mp);
>  	return NULL;                                 ***** Never matches so
> 	returns NULL
>  }
> 
> Will replay after some finer debug has been added
> 
> Again specific to fc_queuecommand and S/W FCOE, not an issue with the F/C
> queuecommand in the HBA templates fro example lpfc or qla2xxx and alos not
> applicable to full offload FCOE like the Emulex cards.
> Thanks
> Laurence
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Hi Hannes

Replying to my own prior message.
Added the additional debug

RHDEBUG: in fc_exch_em_alloc: returning NULL in err: path jumped from  allocate new exch from pool because index == pool->next_index
RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep =           (null)
RHDEBUG: rc -1 with !seq =           (null) after calling tt.exch_seq_send  within fc_fcp_cmd_send
RHDEBUG: rc non zero in :unlock within fc_fcp_cmd_send = -1
RHDEBUG: In fc_fcp_pkt_send, we returned from  rc = lport->tt.fcp_cmd_send with rc = -1
RHDEBUG: We hit SCSI_MLQUEUE_HOST_BUSY in fc_queuecommand with rval in fc_fcp_pkt_send=-1

So we are actually failing in fc_exch_em_alloc with index == pool->next_index, not in the list traversal as I originally thought.

This seems to then match what your said, we are running out of exchanges.

During my testing, if I start multiple dd's and ramp them up, for example 100 parallel dd's with 64 CPUS I see this.
On the 4 CPU system it does not happen.

/**
 * fc_exch_em_alloc() - Allocate an exchange from a specified EM.
 * @lport: The local port that the exchange is for
 * @mp:    The exchange manager that will allocate the exchange
 *
 * Returns pointer to allocated fc_exch with exch lock held.
 */
static struct fc_exch *fc_exch_em_alloc(struct fc_lport *lport,
                                        struct fc_exch_mgr *mp)
{
        struct fc_exch *ep;
        unsigned int cpu;
        u16 index;
        struct fc_exch_pool *pool;

        /* allocate memory for exchange */
        ep = mempool_alloc(mp->ep_pool, GFP_ATOMIC);
        if (!ep) {
                atomic_inc(&mp->stats.no_free_exch);
                goto out;
        }
        memset(ep, 0, sizeof(*ep));

        cpu = get_cpu();
        pool = per_cpu_ptr(mp->pool, cpu);
        spin_lock_bh(&pool->lock);
        put_cpu();

        /* peek cache of free slot */
        if (pool->left != FC_XID_UNKNOWN) {
                index = pool->left;
                pool->left = FC_XID_UNKNOWN;
                goto hit;
        }
        if (pool->right != FC_XID_UNKNOWN) {
                index = pool->right;
                pool->right = FC_XID_UNKNOWN;
                goto hit;
        }

        index = pool->next_index;
        /* allocate new exch from pool */
        while (fc_exch_ptr_get(pool, index)) {
                index = index == mp->pool_max_index ? 0 : index + 1;
                if (index == pool->next_index)
                        goto err;

I will apply your latest FCOE patches, can you provide a link to your tree.

Thanks
Laurence
_______________________________________________
fcoe-devel mailing list
fcoe-devel@open-fcoe.org
http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel

Re: Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE

[Laurence Oberman]

----- Original Message -----
> From: "Laurence Oberman" <loberman@redhat.com>
> To: "Hannes Reinecke" <hare@suse.de>
> Cc: "Linux SCSI Mailinglist" <linux-scsi@vger.kernel.org>, fcoe-devel@open-fcoe.org, "Curtis Taylor (cjt@us.ibm.com)"
> <cjt@us.ibm.com>, "Bud Brown" <bubrown@redhat.com>
> Sent: Saturday, October 8, 2016 3:44:16 PM
> Subject: Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large
> configurations with Intel ixgbe running FCOE
> 
> 
> 
> ----- Original Message -----
> > From: "Laurence Oberman" <loberman@redhat.com>
> > To: "Hannes Reinecke" <hare@suse.de>
> > Cc: "Linux SCSI Mailinglist" <linux-scsi@vger.kernel.org>,
> > fcoe-devel@open-fcoe.org, "Curtis Taylor (cjt@us.ibm.com)"
> > <cjt@us.ibm.com>, "Bud Brown" <bubrown@redhat.com>
> > Sent: Saturday, October 8, 2016 1:53:01 PM
> > Subject: Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by
> > fc_queuecommand on NUMA or large
> > configurations with Intel ixgbe running FCOE
> > 
> > 
> > 
> > ----- Original Message -----
> > > From: "Hannes Reinecke" <hare@suse.de>
> > > To: "Laurence Oberman" <loberman@redhat.com>, "Linux SCSI Mailinglist"
> > > <linux-scsi@vger.kernel.org>,
> > > fcoe-devel@open-fcoe.org
> > > Cc: "Curtis Taylor (cjt@us.ibm.com)" <cjt@us.ibm.com>, "Bud Brown"
> > > <bubrown@redhat.com>
> > > Sent: Saturday, October 8, 2016 1:35:19 PM
> > > Subject: Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by
> > > fc_queuecommand on NUMA or large
> > > configurations with Intel ixgbe running FCOE
> > > 
> > > On 10/08/2016 02:57 PM, Laurence Oberman wrote:
> > > > Hello
> > > >
> > > > This has been a tough problem to chase down but was finally reproduced.
> > > > This issue is apparent on RHEL kernels and upstream so justified
> > > > reporting
> > > > here.
> > > >
> > > > Its out there and some may not be aware its even happening other than
> > > > very
> > > > slow
> > >  > performance using ixgbe and software FCOE on large configurations.
> > > >
> > > > Upstream Kernel used for reproducing is 4.8.0
> > > >
> > > > I/O performance was noted to be very impacted on a large NUMA test
> > > > system
> > > > (64 CPUS 4 NUMA nodes) running the software fcoe stack with Intel ixgbe
> > > > interfaces.
> > > > After capturing blktraces we saw for every I/O there was at least one
> > > > blk_requeue_request and sometimes hundreds or more.
> > > > This resulted in IOPS rates being marginal at best with queuing and
> > > > high
> > > > wait times.
> > > > After narrowing this down with systemtap and trace-cmd we added further
> > > > debug and it was apparent this was dues to SCSI_MLQUEUE_HOST_BUSY being
> > > > returned.
> > > > So I/O passes but very slowly as it constantly having to be requeued.
> > > >
> > > > The identical configuration in our lab with a single NUMA node and 4
> > > > CPUS
> > > > does not see this issue at all.
> > > > The same large system that reproduces this was booted with numa=off and
> > > > still sees the issue.
> > > >
> > > Have you tested with my FCoE fixes?
> > > I've done quite some fixes for libfc/fcoe, and it would be nice to see
> > > how the patches behave with this setup.
> > > 
> > > > The flow is as follows:
> > > >
> > > > From with fc_queuecommand
> > > >           fc_fcp_pkt_send() calls fc_fcp_cmd_send() calls
> > > >           tt.exch_seq_send() which calls fc_exch_seq_send
> > > >
> > > > this fails and returns NULL in fc_exch_alloc() as the list traveral
> > > > never
> > > > creates a match.
> > > >
> > > > static struct fc_seq *fc_exch_seq_send(struct fc_lport *lport,
> > > > 				       struct fc_frame *fp,
> > > > 				       void (*resp)(struct fc_seq *,
> > > > 						    struct fc_frame *fp,
> > > > 						    void *arg),
> > > > 				       void (*destructor)(struct fc_seq *,
> > > > 							  void *),
> > > > 				       void *arg, u32 timer_msec)
> > > > {
> > > > 	struct fc_exch *ep;
> > > > 	struct fc_seq *sp = NULL;
> > > > 	struct fc_frame_header *fh;
> > > > 	struct fc_fcp_pkt *fsp = NULL;
> > > > 	int rc = 1;
> > > >
> > > > 	ep = fc_exch_alloc(lport, fp);     ***** Called Here and fails
> > > > 	if (!ep) {
> > > > 		fc_frame_free(fp);
> > > > 		printk("RHDEBUG: In fc_exch_seq_send returned NULL because !ep with
> > > > 		ep
> > > > 		=
> > > > 		%p\n",ep);
> > > > 		return NULL;
> > > > 	}
> > > > ..
> > > > ..
> > > > ]
> > > >
> > > >
> > > >  fc_exch_alloc() - Allocate an exchange from an EM on a
> > > >  *	/**
> > > >  *	     local port's list of EMs.
> > > >  * @lport: The local port that will own the exchange
> > > >  * @fp:	   The FC frame that the exchange will be for
> > > >  *
> > > >  * This function walks the list of exchange manager(EM)
> > > >  * anchors to select an EM for a new exchange allocation. The
> > > >  * EM is selected when a NULL match function pointer is encountered
> > > >  * or when a call to a match function returns true.
> > > >  */
> > > > static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
> > > > 					    struct fc_frame *fp)
> > > > {
> > > > 	struct fc_exch_mgr_anchor *ema;
> > > >
> > > > 	list_for_each_entry(ema, &lport->ema_list, ema_list)
> > > > 		if (!ema->match || ema->match(fp))
> > > > 			return fc_exch_em_alloc(lport, ema->mp);
> > > > 	return NULL;                                 ***** Never matches so
> > > > 	returns NULL
> > > > }
> > > >
> > > >
> > > > RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep = (null)
> > > > RHDEBUG: rc -1 with !seq = (null) after calling tt.exch_seq_send
> > > > within
> > > > fc_fcp_cmd_send
> > > > RHDEBUG: rc non zero in :unlock within fc_fcp_cmd_send = -1
> > > > RHDEBUG: In fc_fcp_pkt_send, we returned from  rc =
> > > > lport->tt.fcp_cmd_send
> > > > with rc = -1
> > > >
> > > > RHDEBUG: We hit SCSI_MLQUEUE_HOST_BUSY in fc_queuecommand with rval in
> > > > fc_fcp_pkt_send=-1
> > > >
> > > > I am trying to get my head around why a large multi-node system sees
> > > > this
> > > > issue even with NUMA disabled.
> > > > Has anybody seen this or is aware of this with configurations (using
> > > > fc_queuecommand)
> > > >
> > > > I am continuing to add debug to narrow this down.
> > > >
> > > You might actually be hitting a limitation in the exchange manager code.
> > > The libfc exchange manager tries to be really clever and will assign a
> > > per-cpu exchange manager (probably to increase locality). However, we
> > > only have a limited number of exchanges, so on large systems we might
> > > actually run into a exchange starvation problem, where we have in theory
> > > enough free exchanges, but none for the submitting cpu.
> > > 
> > > (Personally, the exchange manager code is in urgent need of reworking.
> > > It should be replaced by the sbitmap code from Omar).
> > > 
> > > Do check how many free exchanges are actually present for the stalling
> > > CPU; it might be that you run into a starvation issue.
> > > 
> > > Cheers,
> > > 
> > > Hannes
> > > --
> > > Dr. Hannes Reinecke		      zSeries & Storage
> > > hare@suse.de			      +49 911 74053 688
> > > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> > > GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
> > > 
> > Hi Hannes,
> > Thanks for responding
> > 
> > I am adding additional debug as I type this.
> > 
> > I am using latest linux-next, I assume your latest FCOE are not in there
> > yet.
> > What is puzzling here is a identical kernel with 1 numa node and only 8GB
> > memory does not see this.
> > Surely if I was running out of exchanges that would show up on the smaller
> > system as well.
> > I am able to get to 1500 IOPS/sec with the same I/O exerciser on the
> > smaller
> > system with ZERO blk_requeue_request() calls.
> > Again, same kernel, same ixgbe same FCOE switch etc.
> > 
> > I traced specifically those initially because we saw it in the blktrace.
> > 
> > I dont understand the match stuff going on in the list reversal stuff here
> > very well, still trying to understand the code flow.
> > the bool match() I also cannot figure out from the code.
> > It runs the fc_exch_em_alloc() if ether bool *match is false or *match(fp)
> > I can't find the actual code for the match, will have to get a vmcore to
> > find
> > it.
> > 
> >  static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
> >  					    struct fc_frame *fp)
> >  {
> > 	struct fc_exch_mgr_anchor *ema;
> > 
> >  	list_for_each_entry(ema, &lport->ema_list, ema_list)
> >  		if (!ema->match || ema->match(fp))
> >  			return fc_exch_em_alloc(lport, ema->mp);
> >  	return NULL;                                 ***** Never matches so
> > 	returns NULL
> >  }
> > 
> > Will replay after some finer debug has been added
> > 
> > Again specific to fc_queuecommand and S/W FCOE, not an issue with the F/C
> > queuecommand in the HBA templates fro example lpfc or qla2xxx and alos not
> > applicable to full offload FCOE like the Emulex cards.
> > Thanks
> > Laurence
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> Hi Hannes
> 
> Replying to my own prior message.
> Added the additional debug
> 
> RHDEBUG: in fc_exch_em_alloc: returning NULL in err: path jumped from
> allocate new exch from pool because index == pool->next_index
> RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep =
> (null)
> RHDEBUG: rc -1 with !seq =           (null) after calling tt.exch_seq_send
> within fc_fcp_cmd_send
> RHDEBUG: rc non zero in :unlock within fc_fcp_cmd_send = -1
> RHDEBUG: In fc_fcp_pkt_send, we returned from  rc = lport->tt.fcp_cmd_send
> with rc = -1
> RHDEBUG: We hit SCSI_MLQUEUE_HOST_BUSY in fc_queuecommand with rval in
> fc_fcp_pkt_send=-1
> 
> So we are actually failing in fc_exch_em_alloc with index ==
> pool->next_index, not in the list traversal as I originally thought.
> 
> This seems to then match what your said, we are running out of exchanges.
> 
> During my testing, if I start multiple dd's and ramp them up, for example 100
> parallel dd's with 64 CPUS I see this.
> On the 4 CPU system it does not happen.
> 
> /**
>  * fc_exch_em_alloc() - Allocate an exchange from a specified EM.
>  * @lport: The local port that the exchange is for
>  * @mp:    The exchange manager that will allocate the exchange
>  *
>  * Returns pointer to allocated fc_exch with exch lock held.
>  */
> static struct fc_exch *fc_exch_em_alloc(struct fc_lport *lport,
>                                         struct fc_exch_mgr *mp)
> {
>         struct fc_exch *ep;
>         unsigned int cpu;
>         u16 index;
>         struct fc_exch_pool *pool;
> 
>         /* allocate memory for exchange */
>         ep = mempool_alloc(mp->ep_pool, GFP_ATOMIC);
>         if (!ep) {
>                 atomic_inc(&mp->stats.no_free_exch);
>                 goto out;
>         }
>         memset(ep, 0, sizeof(*ep));
> 
>         cpu = get_cpu();
>         pool = per_cpu_ptr(mp->pool, cpu);
>         spin_lock_bh(&pool->lock);
>         put_cpu();
> 
>         /* peek cache of free slot */
>         if (pool->left != FC_XID_UNKNOWN) {
>                 index = pool->left;
>                 pool->left = FC_XID_UNKNOWN;
>                 goto hit;
>         }
>         if (pool->right != FC_XID_UNKNOWN) {
>                 index = pool->right;
>                 pool->right = FC_XID_UNKNOWN;
>                 goto hit;
>         }
> 
>         index = pool->next_index;
>         /* allocate new exch from pool */
>         while (fc_exch_ptr_get(pool, index)) {
>                 index = index == mp->pool_max_index ? 0 : index + 1;
>                 if (index == pool->next_index)
>                         goto err;
> 
> I will apply your latest FCOE patches, can you provide a link to your tree.
> 
> Thanks
> Laurence
> 
Replying to my own message.

We fail when index=0 and pool->next_index=0

RHDEBUG: in fc_exch_em_alloc: index=0 pool->next_index) = 0
RHDEBUG: in fc_exch_em_alloc: returning NULL in err: path jumped from  allocate new exch from pool because index == pool->next_index
RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep =           (null)
RHDEBUG: rc -1 with !seq =           (null) after calling tt.exch_seq_send  within fc_fcp_cmd_send
RHDEBUG: rc non zero in :unlock within fc_fcp_cmd_send = -1
RHDEBUG: In fc_fcp_pkt_send, we returned from  rc = lport->tt.fcp_cmd_send with rc = -1

Some additional information.

Does not happen with F/C adapters with same load to same array, just via lpfc and qla2xxx.
However we know that those have their own queuecommand in their scsi_template

I capped the number of CPUS to 4 and running enough parallel dd's I can still trigger this.

[root@fcoe-test-rhel6 ~]# numactl --hardware
available: 1 nodes (0)
node 0 cpus: 0 1 2 3
node 0 size: 147445 MB
node 0 free: 143676 MB
node distances:
node   0
  0:  10 
    
Its still a puzzle why the smaller system in my lab never sees this with same identical configuration.

Running 
for i in `seq 1 30`; do dd if=/dev/sdt of=/dev/null bs=512k iflag=direct count=10 & done   *** Does not trigger

Increment to 40
for i in `seq 1 40`; do dd if=/dev/sdt of=/dev/null bs=512k iflag=direct count=10 & done   *** Triggers the condition

I suspect we have many configurations out there running the ixgbe and software define FCOE stacks that are not even aware they are seeing this issue.

_______________________________________________
fcoe-devel mailing list
fcoe-devel@open-fcoe.org
http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel

Re: Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE

[Laurence Oberman]

----- Original Message -----
> From: "Laurence Oberman" <loberman@redhat.com>
> To: "Hannes Reinecke" <hare@suse.de>
> Cc: "Linux SCSI Mailinglist" <linux-scsi@vger.kernel.org>, fcoe-devel@open-fcoe.org, "Curtis Taylor (cjt@us.ibm.com)"
> <cjt@us.ibm.com>, "Bud Brown" <bubrown@redhat.com>
> Sent: Sunday, October 9, 2016 11:52:44 AM
> Subject: Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large
> configurations with Intel ixgbe running FCOE
> 
> 
> 
> ----- Original Message -----
> > From: "Laurence Oberman" <loberman@redhat.com>
> > To: "Hannes Reinecke" <hare@suse.de>
> > Cc: "Linux SCSI Mailinglist" <linux-scsi@vger.kernel.org>,
> > fcoe-devel@open-fcoe.org, "Curtis Taylor (cjt@us.ibm.com)"
> > <cjt@us.ibm.com>, "Bud Brown" <bubrown@redhat.com>
> > Sent: Saturday, October 8, 2016 3:44:16 PM
> > Subject: Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by
> > fc_queuecommand on NUMA or large
> > configurations with Intel ixgbe running FCOE
> > 
> > 
> > 
> > ----- Original Message -----
> > > From: "Laurence Oberman" <loberman@redhat.com>
> > > To: "Hannes Reinecke" <hare@suse.de>
> > > Cc: "Linux SCSI Mailinglist" <linux-scsi@vger.kernel.org>,
> > > fcoe-devel@open-fcoe.org, "Curtis Taylor (cjt@us.ibm.com)"
> > > <cjt@us.ibm.com>, "Bud Brown" <bubrown@redhat.com>
> > > Sent: Saturday, October 8, 2016 1:53:01 PM
> > > Subject: Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by
> > > fc_queuecommand on NUMA or large
> > > configurations with Intel ixgbe running FCOE
> > > 
> > > 
> > > 
> > > ----- Original Message -----
> > > > From: "Hannes Reinecke" <hare@suse.de>
> > > > To: "Laurence Oberman" <loberman@redhat.com>, "Linux SCSI Mailinglist"
> > > > <linux-scsi@vger.kernel.org>,
> > > > fcoe-devel@open-fcoe.org
> > > > Cc: "Curtis Taylor (cjt@us.ibm.com)" <cjt@us.ibm.com>, "Bud Brown"
> > > > <bubrown@redhat.com>
> > > > Sent: Saturday, October 8, 2016 1:35:19 PM
> > > > Subject: Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by
> > > > fc_queuecommand on NUMA or large
> > > > configurations with Intel ixgbe running FCOE
> > > > 
> > > > On 10/08/2016 02:57 PM, Laurence Oberman wrote:
> > > > > Hello
> > > > >
> > > > > This has been a tough problem to chase down but was finally
> > > > > reproduced.
> > > > > This issue is apparent on RHEL kernels and upstream so justified
> > > > > reporting
> > > > > here.
> > > > >
> > > > > Its out there and some may not be aware its even happening other than
> > > > > very
> > > > > slow
> > > >  > performance using ixgbe and software FCOE on large configurations.
> > > > >
> > > > > Upstream Kernel used for reproducing is 4.8.0
> > > > >
> > > > > I/O performance was noted to be very impacted on a large NUMA test
> > > > > system
> > > > > (64 CPUS 4 NUMA nodes) running the software fcoe stack with Intel
> > > > > ixgbe
> > > > > interfaces.
> > > > > After capturing blktraces we saw for every I/O there was at least one
> > > > > blk_requeue_request and sometimes hundreds or more.
> > > > > This resulted in IOPS rates being marginal at best with queuing and
> > > > > high
> > > > > wait times.
> > > > > After narrowing this down with systemtap and trace-cmd we added
> > > > > further
> > > > > debug and it was apparent this was dues to SCSI_MLQUEUE_HOST_BUSY
> > > > > being
> > > > > returned.
> > > > > So I/O passes but very slowly as it constantly having to be requeued.
> > > > >
> > > > > The identical configuration in our lab with a single NUMA node and 4
> > > > > CPUS
> > > > > does not see this issue at all.
> > > > > The same large system that reproduces this was booted with numa=off
> > > > > and
> > > > > still sees the issue.
> > > > >
> > > > Have you tested with my FCoE fixes?
> > > > I've done quite some fixes for libfc/fcoe, and it would be nice to see
> > > > how the patches behave with this setup.
> > > > 
> > > > > The flow is as follows:
> > > > >
> > > > > From with fc_queuecommand
> > > > >           fc_fcp_pkt_send() calls fc_fcp_cmd_send() calls
> > > > >           tt.exch_seq_send() which calls fc_exch_seq_send
> > > > >
> > > > > this fails and returns NULL in fc_exch_alloc() as the list traveral
> > > > > never
> > > > > creates a match.
> > > > >
> > > > > static struct fc_seq *fc_exch_seq_send(struct fc_lport *lport,
> > > > > 				       struct fc_frame *fp,
> > > > > 				       void (*resp)(struct fc_seq *,
> > > > > 						    struct fc_frame *fp,
> > > > > 						    void *arg),
> > > > > 				       void (*destructor)(struct fc_seq *,
> > > > > 							  void *),
> > > > > 				       void *arg, u32 timer_msec)
> > > > > {
> > > > > 	struct fc_exch *ep;
> > > > > 	struct fc_seq *sp = NULL;
> > > > > 	struct fc_frame_header *fh;
> > > > > 	struct fc_fcp_pkt *fsp = NULL;
> > > > > 	int rc = 1;
> > > > >
> > > > > 	ep = fc_exch_alloc(lport, fp);     ***** Called Here and fails
> > > > > 	if (!ep) {
> > > > > 		fc_frame_free(fp);
> > > > > 		printk("RHDEBUG: In fc_exch_seq_send returned NULL because !ep with
> > > > > 		ep
> > > > > 		=
> > > > > 		%p\n",ep);
> > > > > 		return NULL;
> > > > > 	}
> > > > > ..
> > > > > ..
> > > > > ]
> > > > >
> > > > >
> > > > >  fc_exch_alloc() - Allocate an exchange from an EM on a
> > > > >  *	/**
> > > > >  *	     local port's list of EMs.
> > > > >  * @lport: The local port that will own the exchange
> > > > >  * @fp:	   The FC frame that the exchange will be for
> > > > >  *
> > > > >  * This function walks the list of exchange manager(EM)
> > > > >  * anchors to select an EM for a new exchange allocation. The
> > > > >  * EM is selected when a NULL match function pointer is encountered
> > > > >  * or when a call to a match function returns true.
> > > > >  */
> > > > > static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
> > > > > 					    struct fc_frame *fp)
> > > > > {
> > > > > 	struct fc_exch_mgr_anchor *ema;
> > > > >
> > > > > 	list_for_each_entry(ema, &lport->ema_list, ema_list)
> > > > > 		if (!ema->match || ema->match(fp))
> > > > > 			return fc_exch_em_alloc(lport, ema->mp);
> > > > > 	return NULL;                                 ***** Never matches so
> > > > > 	returns NULL
> > > > > }
> > > > >
> > > > >
> > > > > RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep =
> > > > > (null)
> > > > > RHDEBUG: rc -1 with !seq = (null) after calling tt.exch_seq_send
> > > > > within
> > > > > fc_fcp_cmd_send
> > > > > RHDEBUG: rc non zero in :unlock within fc_fcp_cmd_send = -1
> > > > > RHDEBUG: In fc_fcp_pkt_send, we returned from  rc =
> > > > > lport->tt.fcp_cmd_send
> > > > > with rc = -1
> > > > >
> > > > > RHDEBUG: We hit SCSI_MLQUEUE_HOST_BUSY in fc_queuecommand with rval
> > > > > in
> > > > > fc_fcp_pkt_send=-1
> > > > >
> > > > > I am trying to get my head around why a large multi-node system sees
> > > > > this
> > > > > issue even with NUMA disabled.
> > > > > Has anybody seen this or is aware of this with configurations (using
> > > > > fc_queuecommand)
> > > > >
> > > > > I am continuing to add debug to narrow this down.
> > > > >
> > > > You might actually be hitting a limitation in the exchange manager
> > > > code.
> > > > The libfc exchange manager tries to be really clever and will assign a
> > > > per-cpu exchange manager (probably to increase locality). However, we
> > > > only have a limited number of exchanges, so on large systems we might
> > > > actually run into a exchange starvation problem, where we have in
> > > > theory
> > > > enough free exchanges, but none for the submitting cpu.
> > > > 
> > > > (Personally, the exchange manager code is in urgent need of reworking.
> > > > It should be replaced by the sbitmap code from Omar).
> > > > 
> > > > Do check how many free exchanges are actually present for the stalling
> > > > CPU; it might be that you run into a starvation issue.
> > > > 
> > > > Cheers,
> > > > 
> > > > Hannes
> > > > --
> > > > Dr. Hannes Reinecke		      zSeries & Storage
> > > > hare@suse.de			      +49 911 74053 688
> > > > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> > > > GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
> > > > 
> > > Hi Hannes,
> > > Thanks for responding
> > > 
> > > I am adding additional debug as I type this.
> > > 
> > > I am using latest linux-next, I assume your latest FCOE are not in there
> > > yet.
> > > What is puzzling here is a identical kernel with 1 numa node and only 8GB
> > > memory does not see this.
> > > Surely if I was running out of exchanges that would show up on the
> > > smaller
> > > system as well.
> > > I am able to get to 1500 IOPS/sec with the same I/O exerciser on the
> > > smaller
> > > system with ZERO blk_requeue_request() calls.
> > > Again, same kernel, same ixgbe same FCOE switch etc.
> > > 
> > > I traced specifically those initially because we saw it in the blktrace.
> > > 
> > > I dont understand the match stuff going on in the list reversal stuff
> > > here
> > > very well, still trying to understand the code flow.
> > > the bool match() I also cannot figure out from the code.
> > > It runs the fc_exch_em_alloc() if ether bool *match is false or
> > > *match(fp)
> > > I can't find the actual code for the match, will have to get a vmcore to
> > > find
> > > it.
> > > 
> > >  static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
> > >  					    struct fc_frame *fp)
> > >  {
> > > 	struct fc_exch_mgr_anchor *ema;
> > > 
> > >  	list_for_each_entry(ema, &lport->ema_list, ema_list)
> > >  		if (!ema->match || ema->match(fp))
> > >  			return fc_exch_em_alloc(lport, ema->mp);
> > >  	return NULL;                                 ***** Never matches so
> > > 	returns NULL
> > >  }
> > > 
> > > Will replay after some finer debug has been added
> > > 
> > > Again specific to fc_queuecommand and S/W FCOE, not an issue with the F/C
> > > queuecommand in the HBA templates fro example lpfc or qla2xxx and alos
> > > not
> > > applicable to full offload FCOE like the Emulex cards.
> > > Thanks
> > > Laurence
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> > Hi Hannes
> > 
> > Replying to my own prior message.
> > Added the additional debug
> > 
> > RHDEBUG: in fc_exch_em_alloc: returning NULL in err: path jumped from
> > allocate new exch from pool because index == pool->next_index
> > RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep =
> > (null)
> > RHDEBUG: rc -1 with !seq =           (null) after calling tt.exch_seq_send
> > within fc_fcp_cmd_send
> > RHDEBUG: rc non zero in :unlock within fc_fcp_cmd_send = -1
> > RHDEBUG: In fc_fcp_pkt_send, we returned from  rc = lport->tt.fcp_cmd_send
> > with rc = -1
> > RHDEBUG: We hit SCSI_MLQUEUE_HOST_BUSY in fc_queuecommand with rval in
> > fc_fcp_pkt_send=-1
> > 
> > So we are actually failing in fc_exch_em_alloc with index ==
> > pool->next_index, not in the list traversal as I originally thought.
> > 
> > This seems to then match what your said, we are running out of exchanges.
> > 
> > During my testing, if I start multiple dd's and ramp them up, for example
> > 100
> > parallel dd's with 64 CPUS I see this.
> > On the 4 CPU system it does not happen.
> > 
> > /**
> >  * fc_exch_em_alloc() - Allocate an exchange from a specified EM.
> >  * @lport: The local port that the exchange is for
> >  * @mp:    The exchange manager that will allocate the exchange
> >  *
> >  * Returns pointer to allocated fc_exch with exch lock held.
> >  */
> > static struct fc_exch *fc_exch_em_alloc(struct fc_lport *lport,
> >                                         struct fc_exch_mgr *mp)
> > {
> >         struct fc_exch *ep;
> >         unsigned int cpu;
> >         u16 index;
> >         struct fc_exch_pool *pool;
> > 
> >         /* allocate memory for exchange */
> >         ep = mempool_alloc(mp->ep_pool, GFP_ATOMIC);
> >         if (!ep) {
> >                 atomic_inc(&mp->stats.no_free_exch);
> >                 goto out;
> >         }
> >         memset(ep, 0, sizeof(*ep));
> > 
> >         cpu = get_cpu();
> >         pool = per_cpu_ptr(mp->pool, cpu);
> >         spin_lock_bh(&pool->lock);
> >         put_cpu();
> > 
> >         /* peek cache of free slot */
> >         if (pool->left != FC_XID_UNKNOWN) {
> >                 index = pool->left;
> >                 pool->left = FC_XID_UNKNOWN;
> >                 goto hit;
> >         }
> >         if (pool->right != FC_XID_UNKNOWN) {
> >                 index = pool->right;
> >                 pool->right = FC_XID_UNKNOWN;
> >                 goto hit;
> >         }
> > 
> >         index = pool->next_index;
> >         /* allocate new exch from pool */
> >         while (fc_exch_ptr_get(pool, index)) {
> >                 index = index == mp->pool_max_index ? 0 : index + 1;
> >                 if (index == pool->next_index)
> >                         goto err;
> > 
> > I will apply your latest FCOE patches, can you provide a link to your tree.
> > 
> > Thanks
> > Laurence
> > 
> Replying to my own message.
> 
> We fail when index=0 and pool->next_index=0
> 
> RHDEBUG: in fc_exch_em_alloc: index=0 pool->next_index) = 0
> RHDEBUG: in fc_exch_em_alloc: returning NULL in err: path jumped from
> allocate new exch from pool because index == pool->next_index
> RHDEBUG: In fc_exch_seq_send returned NULL because !ep with ep =
> (null)
> RHDEBUG: rc -1 with !seq =           (null) after calling tt.exch_seq_send
> within fc_fcp_cmd_send
> RHDEBUG: rc non zero in :unlock within fc_fcp_cmd_send = -1
> RHDEBUG: In fc_fcp_pkt_send, we returned from  rc = lport->tt.fcp_cmd_send
> with rc = -1
> 
> Some additional information.
> 
> Does not happen with F/C adapters with same load to same array, just via lpfc
> and qla2xxx.
> However we know that those have their own queuecommand in their scsi_template
> 
> I capped the number of CPUS to 4 and running enough parallel dd's I can still
> trigger this.
> 
> [root@fcoe-test-rhel6 ~]# numactl --hardware
> available: 1 nodes (0)
> node 0 cpus: 0 1 2 3
> node 0 size: 147445 MB
> node 0 free: 143676 MB
> node distances:
> node   0
>   0:  10
>     
> Its still a puzzle why the smaller system in my lab never sees this with same
> identical configuration.
> 
> Running
> for i in `seq 1 30`; do dd if=/dev/sdt of=/dev/null bs=512k iflag=direct
> count=10 & done   *** Does not trigger
> 
> Increment to 40
> for i in `seq 1 40`; do dd if=/dev/sdt of=/dev/null bs=512k iflag=direct
> count=10 & done   *** Triggers the condition
> 
> I suspect we have many configurations out there running the ixgbe and
> software define FCOE stacks that are not even aware they are seeing this
> issue.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Replying to my own message

Just an update here.
I have been working with Curtis and Ewan, and we have been further debugging this Intel EE issue.
What is apparent here is that we are indeed suffering from a high CPU count issue here that is exhausting resources.
In my opinion, this card was never really tested on systems with high numbers of CPUS to see how the exchange resources are mapped.
By decreasing the CPU count we get better performance as we see less of the MLQUEUE_HOST_BUSY returned.

This has to be affecting any installations that have high CPU count systems using this Intel EE card.
Likely, as already said, thay are unaware of this.

For comparison I tested a Qlogic BCM57810 card that is partial offload and I see zero blk_requeue_request() calls for the identical test.

Qlogic BCM57810
Ethernet controller: Broadcom Corporation NetXtreme II BCM57810 10 Gigabit Ethernet (rev 10)

The BCM57810 is a partial offload in that it needs some of the software fcoe support but manages the rest on the card.

Note we do not use the DCB service provided in software, so we disable it.
# Indicate if DCB service is required at the Ethernet port
# Normally set to "yes"
DCB_REQUIRED="no"

Again ZERO blk_requeue_request() calls

What was apparent in the DEBUG was the xid_min and xid_max start higher.

We start with a min of 4096 and see a max of 12286

RHDEBUG: In fc_exch_mgr_alloc, we were called with min_xid=4096 and max_xid=12286
RHDEBUG: In fc_exch_mgr_alloc, we are returning mp with min_xid=4096 and max_xid=12286
RHDEBUG: In fc_exch_mgr_alloc, we were called with min_xid=4096 and max_xid=12286
RHDEBUG: In fc_exch_mgr_alloc, we are returning mp with min_xid=4096 and max_xid=12286

Whereas the Intel EE card with all FCOE in software sees:

RHDEBUG: In fc_exch_mgr_alloc, we were called with min_xid=0 and max_xid=511
RHDEBUG: In fc_exch_mgr_alloc, we were called with min_xid=512 and max_xid=4095
RHDEBUG: In fc_exch_mgr_alloc, we were called with min_xid=0 and max_xid=511
RHDEBUG: In fc_exch_mgr_alloc, we were called with min_xid=512 and max_xid=4095

We are still tracing this and I will report back when I have an update.
_______________________________________________
fcoe-devel mailing list
fcoe-devel@open-fcoe.org
http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel

Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE

[Ewan D. Milne]

On Sat, 2016-10-08 at 19:35 +0200, Hannes Reinecke wrote:
> You might actually be hitting a limitation in the exchange manager code.
> The libfc exchange manager tries to be really clever and will assign a 
> per-cpu exchange manager (probably to increase locality). However, we 
> only have a limited number of exchanges, so on large systems we might 
> actually run into a exchange starvation problem, where we have in theory 
> enough free exchanges, but none for the submitting cpu.
> 
> (Personally, the exchange manager code is in urgent need of reworking.
> It should be replaced by the sbitmap code from Omar).
> 
> Do check how many free exchanges are actually present for the stalling 
> CPU; it might be that you run into a starvation issue.

We are still looking into this but one thing that looks bad is that
the exchange manager code rounds up the number of CPUs to the next
power of 2 before dividing up the exchange id space (and uses the lsbs
of the xid to extract the CPU when looking up an xid). We have a machine
with 288 CPUs, this code is just begging for a rewrite as it looks to
be wasting most of the limited xid space on ixgbe FCoE.

Looks like we get 512 offloaded xids on this adapter and 4096-512
non-offloaded xids.  This would give 1 + 7 xids per CPU.  However, I'm
not sure that even 4096 / 288 = 14 would be enough to prevent stalling.

And, of course, potentially most of the CPUs aren't submitting I/O, so
the whole idea of per-CPU xid space is questionable.

-Ewan

Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE

[Ewan D. Milne]

On Tue, 2016-10-11 at 10:51 -0400, Ewan D. Milne wrote:
> On Sat, 2016-10-08 at 19:35 +0200, Hannes Reinecke wrote:
> > You might actually be hitting a limitation in the exchange manager code.
> > The libfc exchange manager tries to be really clever and will assign a 
> > per-cpu exchange manager (probably to increase locality). However, we 
> > only have a limited number of exchanges, so on large systems we might 
> > actually run into a exchange starvation problem, where we have in theory 
> > enough free exchanges, but none for the submitting cpu.
> > 
> > (Personally, the exchange manager code is in urgent need of reworking.
> > It should be replaced by the sbitmap code from Omar).
> > 
> > Do check how many free exchanges are actually present for the stalling 
> > CPU; it might be that you run into a starvation issue.
> 
> We are still looking into this but one thing that looks bad is that
> the exchange manager code rounds up the number of CPUs to the next
> power of 2 before dividing up the exchange id space (and uses the lsbs
> of the xid to extract the CPU when looking up an xid). We have a machine
> with 288 CPUs, this code is just begging for a rewrite as it looks to
> be wasting most of the limited xid space on ixgbe FCoE.
> 
> Looks like we get 512 offloaded xids on this adapter and 4096-512
> non-offloaded xids.  This would give 1 + 7 xids per CPU.  However, I'm
> not sure that even 4096 / 288 = 14 would be enough to prevent stalling.
> 
> And, of course, potentially most of the CPUs aren't submitting I/O, so
> the whole idea of per-CPU xid space is questionable.
> 

fc_exch_alloc() used to try all the available exchange managers in the
list for an available exchange id, but this was changed in 2010 so that
if the first matched exchange manager couldn't allocate one, it fails
and we end up returning host busy.  This was due to commit:

commit 3e22760d4db6fd89e0be46c3d132390a251da9c6
Author: Vasu Dev <vasu.dev@intel.com>
Date:   Fri Mar 12 16:08:39 2010 -0800

    [SCSI] libfc: use offload EM instance again instead jumping to next EM
    
    Since use of offloads is more efficient than switching
    to non-offload EM. However kept logic same to call em_match
    if it is provided in the list of EMs.
    
    Converted fc_exch_alloc to inline being now tiny a function
    and already not an exported libfc API any more.
    
    Signed-off-by: Vasu Dev <vasu.dev@intel.com>
    Signed-off-by: Robert Love <robert.w.love@intel.com>
    Signed-off-by: James Bottomley <James.Bottomley@suse.de>

---

Setting the ddp_min module parameter to fcoe to 128MB prevents the ->match
function from permitting the use of the offload exchange manager for the frame,
and we no longer see the problem with host busy status, since it uses the
larger non-offloaded pool.

I couldn't find any history on the motivation for the above commit, was
there a significant benefit in some case?  There seems to be a big downside.

-Ewan

Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE

[Hannes Reinecke]

On 10/12/2016 05:26 PM, Ewan D. Milne wrote:
> On Tue, 2016-10-11 at 10:51 -0400, Ewan D. Milne wrote:
>> On Sat, 2016-10-08 at 19:35 +0200, Hannes Reinecke wrote:
>>> You might actually be hitting a limitation in the exchange manager code.
>>> The libfc exchange manager tries to be really clever and will assign a
>>> per-cpu exchange manager (probably to increase locality). However, we
>>> only have a limited number of exchanges, so on large systems we might
>>> actually run into a exchange starvation problem, where we have in theory
>>> enough free exchanges, but none for the submitting cpu.
>>>
>>> (Personally, the exchange manager code is in urgent need of reworking.
>>> It should be replaced by the sbitmap code from Omar).
>>>
>>> Do check how many free exchanges are actually present for the stalling
>>> CPU; it might be that you run into a starvation issue.
>>
>> We are still looking into this but one thing that looks bad is that
>> the exchange manager code rounds up the number of CPUs to the next
>> power of 2 before dividing up the exchange id space (and uses the lsbs
>> of the xid to extract the CPU when looking up an xid). We have a machine
>> with 288 CPUs, this code is just begging for a rewrite as it looks to
>> be wasting most of the limited xid space on ixgbe FCoE.
>>
>> Looks like we get 512 offloaded xids on this adapter and 4096-512
>> non-offloaded xids.  This would give 1 + 7 xids per CPU.  However, I'm
>> not sure that even 4096 / 288 = 14 would be enough to prevent stalling.
>>
>> And, of course, potentially most of the CPUs aren't submitting I/O, so
>> the whole idea of per-CPU xid space is questionable.
>>
>
> fc_exch_alloc() used to try all the available exchange managers in the
> list for an available exchange id, but this was changed in 2010 so that
> if the first matched exchange manager couldn't allocate one, it fails
> and we end up returning host busy.  This was due to commit:
>
> commit 3e22760d4db6fd89e0be46c3d132390a251da9c6
> Author: Vasu Dev <vasu.dev@intel.com>
> Date:   Fri Mar 12 16:08:39 2010 -0800
>
>     [SCSI] libfc: use offload EM instance again instead jumping to next EM
>
>     Since use of offloads is more efficient than switching
>     to non-offload EM. However kept logic same to call em_match
>     if it is provided in the list of EMs.
>
>     Converted fc_exch_alloc to inline being now tiny a function
>     and already not an exported libfc API any more.
>
>     Signed-off-by: Vasu Dev <vasu.dev@intel.com>
>     Signed-off-by: Robert Love <robert.w.love@intel.com>
>     Signed-off-by: James Bottomley <James.Bottomley@suse.de>
>
> ---
>
> Setting the ddp_min module parameter to fcoe to 128MB prevents the ->match
> function from permitting the use of the offload exchange manager for the frame,
> and we no longer see the problem with host busy status, since it uses the
> larger non-offloaded pool.
>
Yes, this is also the impression I got from reading the spec.
The offload pool is mainly designed for large read or write commands, so 
using it for _every_ frame is probably not a good idea.
And limiting it by the size of the transfers solves the problem quite 
nicely, as a large size typically is only used by read and writes.
So please send a patch to revert that.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE

[Laurence Oberman]

----- Original Message -----
> From: "Hannes Reinecke" <hare@suse.de>
> To: emilne@redhat.com, "vasu dev" <vasu.dev@intel.com>, "robert w love" <robert.w.love@intel.com>
> Cc: "Laurence Oberman" <loberman@redhat.com>, "Linux SCSI Mailinglist" <linux-scsi@vger.kernel.org>,
> fcoe-devel@open-fcoe.org, "Curtis Taylor (cjt@us.ibm.com)" <cjt@us.ibm.com>, "Bud Brown" <bubrown@redhat.com>
> Sent: Wednesday, October 12, 2016 11:46:16 AM
> Subject: Re: [Open-FCoE] Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large
> configurations with Intel ixgbe running FCOE
> 
> On 10/12/2016 05:26 PM, Ewan D. Milne wrote:
> > On Tue, 2016-10-11 at 10:51 -0400, Ewan D. Milne wrote:
> >> On Sat, 2016-10-08 at 19:35 +0200, Hannes Reinecke wrote:
> >>> You might actually be hitting a limitation in the exchange manager code.
> >>> The libfc exchange manager tries to be really clever and will assign a
> >>> per-cpu exchange manager (probably to increase locality). However, we
> >>> only have a limited number of exchanges, so on large systems we might
> >>> actually run into a exchange starvation problem, where we have in theory
> >>> enough free exchanges, but none for the submitting cpu.
> >>>
> >>> (Personally, the exchange manager code is in urgent need of reworking.
> >>> It should be replaced by the sbitmap code from Omar).
> >>>
> >>> Do check how many free exchanges are actually present for the stalling
> >>> CPU; it might be that you run into a starvation issue.
> >>
> >> We are still looking into this but one thing that looks bad is that
> >> the exchange manager code rounds up the number of CPUs to the next
> >> power of 2 before dividing up the exchange id space (and uses the lsbs
> >> of the xid to extract the CPU when looking up an xid). We have a machine
> >> with 288 CPUs, this code is just begging for a rewrite as it looks to
> >> be wasting most of the limited xid space on ixgbe FCoE.
> >>
> >> Looks like we get 512 offloaded xids on this adapter and 4096-512
> >> non-offloaded xids.  This would give 1 + 7 xids per CPU.  However, I'm
> >> not sure that even 4096 / 288 = 14 would be enough to prevent stalling.
> >>
> >> And, of course, potentially most of the CPUs aren't submitting I/O, so
> >> the whole idea of per-CPU xid space is questionable.
> >>
> >
> > fc_exch_alloc() used to try all the available exchange managers in the
> > list for an available exchange id, but this was changed in 2010 so that
> > if the first matched exchange manager couldn't allocate one, it fails
> > and we end up returning host busy.  This was due to commit:
> >
> > commit 3e22760d4db6fd89e0be46c3d132390a251da9c6
> > Author: Vasu Dev <vasu.dev@intel.com>
> > Date:   Fri Mar 12 16:08:39 2010 -0800
> >
> >     [SCSI] libfc: use offload EM instance again instead jumping to next EM
> >
> >     Since use of offloads is more efficient than switching
> >     to non-offload EM. However kept logic same to call em_match
> >     if it is provided in the list of EMs.
> >
> >     Converted fc_exch_alloc to inline being now tiny a function
> >     and already not an exported libfc API any more.
> >
> >     Signed-off-by: Vasu Dev <vasu.dev@intel.com>
> >     Signed-off-by: Robert Love <robert.w.love@intel.com>
> >     Signed-off-by: James Bottomley <James.Bottomley@suse.de>
> >
> > ---
> >
> > Setting the ddp_min module parameter to fcoe to 128MB prevents the ->match
> > function from permitting the use of the offload exchange manager for the
> > frame,
> > and we no longer see the problem with host busy status, since it uses the
> > larger non-offloaded pool.
> >
> Yes, this is also the impression I got from reading the spec.
> The offload pool is mainly designed for large read or write commands, so
> using it for _every_ frame is probably not a good idea.
> And limiting it by the size of the transfers solves the problem quite
> nicely, as a large size typically is only used by read and writes.
> So please send a patch to revert that.
> 
> Cheers,
> 
> Hannes
> --
> Dr. Hannes Reinecke		      zSeries & Storage
> hare@suse.de			      +49 911 74053 688
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
I will revert the commit and test it here in the lab, and then submit the revert patch.
Ewan can review.

Thanks
Laurence

Patch: Revert commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 due to performance issues

[Laurence Oberman]

Hello

This patch reverts commit 3e22760d4db6fd89e0be46c3d132390a251da9c6.

This revert came about because of efforts by Ewan Milne, Curtis Taylor and I.
In researching this issue, significant performance issues were seen on large CPU count 
systems using the software FCOE stack.
Hannes also weighed in.

The same was not apparent on much smaller low count CPU systems.
The behavior introduced by commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 lands sup with large
count CPU systems seeing continual blk_requeue_request() calls due to ML_QUEUE_HOST_BUSY.

>From Ewan:

fc_exch_alloc() used to try all the available exchange managers in the
list for an available exchange id, but this was changed in 2010 so that
if the first matched exchange manager couldn't allocate one, it fails
and we end up returning host busy.  This was due to commit:

Setting the ddp_min module parameter to fcoe to 128MB prevents the ->match
function from permitting the use of the offload exchange manager for the frame,
and we no longer see the problem with host busy status, since it uses the
larger non-offloaded pool.

Reverting commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 was tested to also
prevent the host busy issue due to failing allocations.

Suggested-by: Ewan Milne <emilne-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Suggested-by: Curtis Taylor <cjt-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Tested-by: Laurence Oberman <loberman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Laurence Oberman <loberman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index e72673b..d5c4048 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -892,10 +892,15 @@ static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
 					    struct fc_frame *fp)
 {
 	struct fc_exch_mgr_anchor *ema;
+	struct fc_exch *ep;
 
-	list_for_each_entry(ema, &lport->ema_list, ema_list)
-		if (!ema->match || ema->match(fp))
-			return fc_exch_em_alloc(lport, ema->mp);
+	list_for_each_entry(ema, &lport->ema_list, ema_list) {
+		if (!ema->match || ema->match(fp)) {
+			ep = fc_exch_em_alloc(lport, ema->mp);
+			if (ep)
+				return ep;
+		}
+	}
 	return NULL;
 }

Re: Patch: Revert commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 due to performance issues

[Hannes Reinecke]

On 10/13/2016 02:43 PM, Laurence Oberman wrote:
> 
> 
> Hello
> 
> This patch reverts commit 3e22760d4db6fd89e0be46c3d132390a251da9c6.
> 
> This revert came about because of efforts by Ewan Milne, Curtis Taylor and I.
> In researching this issue, significant performance issues were seen on large CPU count 
> systems using the software FCOE stack.
> Hannes also weighed in.
> 
> The same was not apparent on much smaller low count CPU systems.
> The behavior introduced by commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 lands sup with large
> count CPU systems seeing continual blk_requeue_request() calls due to ML_QUEUE_HOST_BUSY.
> 
> From Ewan:
> 
> fc_exch_alloc() used to try all the available exchange managers in the
> list for an available exchange id, but this was changed in 2010 so that
> if the first matched exchange manager couldn't allocate one, it fails
> and we end up returning host busy.  This was due to commit:
> 
> Setting the ddp_min module parameter to fcoe to 128MB prevents the ->match
> function from permitting the use of the offload exchange manager for the frame,
> and we no longer see the problem with host busy status, since it uses the
> larger non-offloaded pool.
> 
> Reverting commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 was tested to also
> prevent the host busy issue due to failing allocations.
> 
> Suggested-by: Ewan Milne <emilne@redhat.com>
> Suggested-by: Curtis Taylor <cjt@us.ibm.com>
> Tested-by: Laurence Oberman <loberman@redhat.com>
> Signed-off-by: Laurence Oberman <loberman@redhat.com>
> 
> diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
> index e72673b..d5c4048 100644
> --- a/drivers/scsi/libfc/fc_exch.c
> +++ b/drivers/scsi/libfc/fc_exch.c
> @@ -892,10 +892,15 @@ static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
>  					    struct fc_frame *fp)
>  {
>  	struct fc_exch_mgr_anchor *ema;
> +	struct fc_exch *ep;
>  
> -	list_for_each_entry(ema, &lport->ema_list, ema_list)
> -		if (!ema->match || ema->match(fp))
> -			return fc_exch_em_alloc(lport, ema->mp);
> +	list_for_each_entry(ema, &lport->ema_list, ema_list) {
> +		if (!ema->match || ema->match(fp)) {
> +			ep = fc_exch_em_alloc(lport, ema->mp);
> +			if (ep)
> +				return ep;
> +		}
> +	}
>  	return NULL;
>  }
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

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)
_______________________________________________
fcoe-devel mailing list
fcoe-devel@open-fcoe.org
http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel

Re: Patch: Revert commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 due to performance issues

[Johannes Thumshirn]

On Thu, Oct 13, 2016 at 08:43:30AM -0400, Laurence Oberman wrote:
> 
> 
> Hello
> 
> This patch reverts commit 3e22760d4db6fd89e0be46c3d132390a251da9c6.
> 
> This revert came about because of efforts by Ewan Milne, Curtis Taylor and I.
> In researching this issue, significant performance issues were seen on large CPU count 
> systems using the software FCOE stack.
> Hannes also weighed in.
> 
> The same was not apparent on much smaller low count CPU systems.
> The behavior introduced by commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 lands sup with large
> count CPU systems seeing continual blk_requeue_request() calls due to ML_QUEUE_HOST_BUSY.
> 
> From Ewan:
> 
> fc_exch_alloc() used to try all the available exchange managers in the
> list for an available exchange id, but this was changed in 2010 so that
> if the first matched exchange manager couldn't allocate one, it fails
> and we end up returning host busy.  This was due to commit:
> 
> Setting the ddp_min module parameter to fcoe to 128MB prevents the ->match
> function from permitting the use of the offload exchange manager for the frame,
> and we no longer see the problem with host busy status, since it uses the
> larger non-offloaded pool.
> 
> Reverting commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 was tested to also
> prevent the host busy issue due to failing allocations.
> 
> Suggested-by: Ewan Milne <emilne-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Suggested-by: Curtis Taylor <cjt-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> Tested-by: Laurence Oberman <loberman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Laurence Oberman <loberman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> 

Acked-by: Johannes Thumshirn <jth-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

-- 
Johannes Thumshirn                                          Storage
jthumshirn-l3A5Bk7waGM@public.gmane.org                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: Patch: [Open-FCoE] Revert commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 due to performance issues

[Martin K. Petersen]

>>>>> "Laurence" == Laurence Oberman <loberman@redhat.com> writes:

Laurence> This patch reverts commit
Laurence> 3e22760d4db6fd89e0be46c3d132390a251da9c6.

Laurence> This revert came about because of efforts by Ewan Milne,
Laurence> Curtis Taylor and I.  In researching this issue, significant
Laurence> performance issues were seen on large CPU count systems using
Laurence> the software FCOE stack.  Hannes also weighed in.

Laurence> The same was not apparent on much smaller low count CPU
Laurence> systems.  The behavior introduced by commit
Laurence> 3e22760d4db6fd89e0be46c3d132390a251da9c6 lands sup with large
Laurence> count CPU systems seeing continual blk_requeue_request() calls
Laurence> due to ML_QUEUE_HOST_BUSY.

Applied to 4.10/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering