| Message ID | 2049046384.1533310.1476362610308.JavaMail.zimbra@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
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
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@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> > Acked-by: Johannes Thumshirn <jth@kernel.org>
>>>>> "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.
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; } -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in