| Message ID | 1448973589-9216-6-git-send-email-RaghavaAditya.Renukunta@pmcs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Raghava, On Tue, 2015-12-01 at 04:39 -0800, Raghava Aditya Renukunta wrote: > From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com> > > During EEH recovery number of online CPU's might change thereby changing > the number of MSIx vectors. Since each fib is allocated to a vector, > changes in the number of vectors causes fib to be sent thru invalid > vectors.In addition the correct number of MSIx vectors is not > updated in the INIT struct sent to the controller, when it is > reinitialized. > > Fixed by reassigning vectors to fibs based on the updated number of MSIx > vectors and updating the INIT structure before sending to controller. > [...] > - if (!dev->sync_mode) > + if (!dev->sync_mode) { > + /* After EEH recovery or suspend resume, max_msix count > + * may change, therfore updating in init as well. > + */ > aac_adapter_start(dev); > + dev->init->Sa_MSIXVectors = cpu_to_le32(dev->max_msix); > + } > return 0; > > error_iounmap: This is for EEH and suspend/resume isn't it? If it fixes MSI-X vector calculation for suspend/resume as well you probably should tag it for inclusion into stable as well. Thanks, Johannes -- 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
SGVsbG8gSm9oYW5uZXMsDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTog Sm9oYW5uZXMgVGh1bXNoaXJuIFttYWlsdG86anRodW1zaGlybkBzdXNlLmRlXQ0KPiBTZW50OiBX ZWRuZXNkYXksIERlY2VtYmVyIDIsIDIwMTUgMjoyNyBBTQ0KPiBUbzogUmFnaGF2YSBBZGl0eWEg UmVudWt1bnRhOyBKQm90dG9tbGV5QFBhcmFsbGVscy5jb207IGxpbnV4LQ0KPiBzY3NpQHZnZXIu a2VybmVsLm9yZw0KPiBDYzogTWFoZXNoIFJhamFzaGVraGFyYTsgTXVydGh5IEJoYXQ7IFNhbnRv c2ggQWt1bGE7IEdhbmEgU3JpZGFyYW47DQo+IGFhY3JhaWRAcG1jLXNpZXJyYS5jb207IFJpY2gg Qm9ubw0KPiBTdWJqZWN0OiBSZTogW1BBVENIIDA1LzEwXSBhYWNyYWlkOiBTZXQgY29ycmVjdCBt c2l4IGNvdW50IGZvciBFRUggcmVjb3ZlcnkNCj4gDQo+IEhpIFJhZ2hhdmEsDQo+IA0KPiBPbiBU dWUsIDIwMTUtMTItMDEgYXQgMDQ6MzkgLTA4MDAsIFJhZ2hhdmEgQWRpdHlhIFJlbnVrdW50YSB3 cm90ZToNCj4gPiBGcm9tOiBSYWdoYXZhIEFkaXR5YSBSZW51a3VudGEgPHJhZ2hhdmFhZGl0eWEu cmVudWt1bnRhQHBtY3MuY29tPg0KPiA+DQo+ID4gRHVyaW5nIEVFSCByZWNvdmVyeSBudW1iZXIg b2Ygb25saW5lIENQVSdzIG1pZ2h0IGNoYW5nZSB0aGVyZWJ5DQo+ID4gY2hhbmdpbmcgdGhlIG51 bWJlciBvZiBNU0l4IHZlY3RvcnMuIFNpbmNlIGVhY2ggZmliIGlzIGFsbG9jYXRlZCB0byBhDQo+ ID4gdmVjdG9yLCBjaGFuZ2VzIGluIHRoZSBudW1iZXIgb2YgdmVjdG9ycyBjYXVzZXMgZmliIHRv IGJlIHNlbnQgdGhydQ0KPiA+IGludmFsaWQgdmVjdG9ycy5JbiBhZGRpdGlvbiB0aGUgY29ycmVj dCBudW1iZXIgb2YgTVNJeCB2ZWN0b3JzIGlzIG5vdA0KPiA+IHVwZGF0ZWQgaW4gdGhlIElOSVQg c3RydWN0IHNlbnQgdG8gdGhlIGNvbnRyb2xsZXIsIHdoZW4gaXQgaXMNCj4gPiByZWluaXRpYWxp emVkLg0KPiA+DQo+ID4gRml4ZWQgYnkgcmVhc3NpZ25pbmcgdmVjdG9ycyB0byBmaWJzIGJhc2Vk IG9uIHRoZSB1cGRhdGVkIG51bWJlciBvZg0KPiA+IE1TSXggdmVjdG9ycyBhbmQgdXBkYXRpbmcg dGhlIElOSVQgc3RydWN0dXJlIGJlZm9yZSBzZW5kaW5nIHRvIGNvbnRyb2xsZXIuDQo+ID4NCj4g DQo+IFsuLi5dDQo+IA0KPiA+IC0JaWYgKCFkZXYtPnN5bmNfbW9kZSkNCj4gPiArCWlmICghZGV2 LT5zeW5jX21vZGUpIHsNCj4gPiArCQkvKiBBZnRlciBFRUggcmVjb3Zlcnkgb3Igc3VzcGVuZCBy ZXN1bWUsIG1heF9tc2l4IGNvdW50DQo+ID4gKwkJwqAqIG1heSBjaGFuZ2UsIHRoZXJmb3JlIHVw ZGF0aW5nIGluIGluaXQgYXMgd2VsbC4NCj4gPiArCQnCoCovDQo+ID4gwqAJCWFhY19hZGFwdGVy X3N0YXJ0KGRldik7DQo+ID4gKwkJZGV2LT5pbml0LT5TYV9NU0lYVmVjdG9ycyA9IGNwdV90b19s ZTMyKGRldi0+bWF4X21zaXgpOw0KPiA+ICsJfQ0KPiA+IMKgCXJldHVybiAwOw0KPiA+DQo+ID4g wqBlcnJvcl9pb3VubWFwOg0KPiANCj4gDQo+IFRoaXMgaXMgZm9yIEVFSCBhbmQgc3VzcGVuZC9y ZXN1bWUgaXNuJ3QgaXQ/IElmIGl0IGZpeGVzIE1TSS1YIHZlY3RvciBjYWxjdWxhdGlvbg0KPiBm b3Igc3VzcGVuZC9yZXN1bWUgYXMgd2VsbCB5b3UgcHJvYmFibHkgc2hvdWxkIHRhZyBpdCBmb3Ig aW5jbHVzaW9uIGludG8NCj4gc3RhYmxlIGFzIHdlbGwuDQoNClllcyB0aGlzIGlzIGZvciBFRUgg YW5kIHN1c3BlbmQgYW5kIHJlc3VtZS4gSXQgZG9lcyBmaXggTVNJLVggdmVjdG9yIGNhbGN1bGF0 aW9uIGZvcg0KU3VzcGVuZC9yZXN1bWUuIEkgd2lsbCB0YWcgaXQgaW50byBmb3IgaW5jbHVzaW9u IHRvIHN0YWJsZS4NCiANClRoYW5rIHlvdSwNClJhZ2hhdmEgQWRpdHlhDQoNCg0K -- 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
On 1.12.2015 13:39, Raghava Aditya Renukunta wrote: > From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com> > > During EEH recovery number of online CPU's might change thereby changing > the number of MSIx vectors. Since each fib is allocated to a vector, > changes in the number of vectors causes fib to be sent thru invalid > vectors.In addition the correct number of MSIx vectors is not > updated in the INIT struct sent to the controller, when it is > reinitialized. > > Fixed by reassigning vectors to fibs based on the updated number of MSIx > vectors and updating the INIT structure before sending to controller. > > Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com> > --- > drivers/scsi/aacraid/linit.c | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c > index 0147210..f88f1132 100644 > --- a/drivers/scsi/aacraid/linit.c > +++ b/drivers/scsi/aacraid/linit.c > @@ -1355,6 +1355,7 @@ static int aac_acquire_resources(struct aac_dev *dev) > int i, j; > int instance = dev->id; > const char *name = dev->name; > + int vector = 0; > unsigned long status; > /* > * First clear out all interrupts. Then enable the one's that we > @@ -1409,9 +1410,31 @@ static int aac_acquire_resources(struct aac_dev *dev) > } > > aac_adapter_enable_int(dev); > + /*max msix may change after EEH > + * Re-assign vectors to fibs > + */ > + for (i = 0; > + i < (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB); > + i++) { > + if ((dev->max_msix == 1) || > + (i > ((dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB - 1) > + - dev->vector_cap))) { > + dev->fibs[i].vector_no = 0; > + } else { > + dev->fibs[i].vector_no = vector; > + vector++; > + if (vector == dev->max_msix) > + vector = 1; > + } > + } The above hunk added code looks identical to the part you have just added in 02/10 "aacraid: Fix RRQ overload" could you make this to function ? Thanks, tomash > > - if (!dev->sync_mode) > + if (!dev->sync_mode) { > + /* After EEH recovery or suspend resume, max_msix count > + * may change, therfore updating in init as well. > + */ > aac_adapter_start(dev); > + dev->init->Sa_MSIXVectors = cpu_to_le32(dev->max_msix); > + } > return 0; > > error_iounmap: -- 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
Hello Tomas, > -----Original Message----- > From: Tomas Henzl [mailto:thenzl@redhat.com] > Sent: Friday, December 4, 2015 6:10 AM > To: Raghava Aditya Renukunta; JBottomley@Parallels.com; linux- > scsi@vger.kernel.org > Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; > aacraid@pmc-sierra.com; Rich Bono > Subject: Re: [PATCH 05/10] aacraid: Set correct msix count for EEH recovery > > On 1.12.2015 13:39, Raghava Aditya Renukunta wrote: > > From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com> > > > > During EEH recovery number of online CPU's might change thereby > changing > > the number of MSIx vectors. Since each fib is allocated to a vector, > > changes in the number of vectors causes fib to be sent thru invalid > > vectors.In addition the correct number of MSIx vectors is not > > updated in the INIT struct sent to the controller, when it is > > reinitialized. > > > > Fixed by reassigning vectors to fibs based on the updated number of MSIx > > vectors and updating the INIT structure before sending to controller. > > > > Signed-off-by: Raghava Aditya Renukunta > <raghavaaditya.renukunta@pmcs.com> > > --- > > drivers/scsi/aacraid/linit.c | 25 ++++++++++++++++++++++++- > > 1 file changed, 24 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c > > index 0147210..f88f1132 100644 > > --- a/drivers/scsi/aacraid/linit.c > > +++ b/drivers/scsi/aacraid/linit.c > > @@ -1355,6 +1355,7 @@ static int aac_acquire_resources(struct aac_dev > *dev) > > int i, j; > > int instance = dev->id; > > const char *name = dev->name; > > + int vector = 0; > > unsigned long status; > > /* > > * First clear out all interrupts. Then enable the one's that we > > @@ -1409,9 +1410,31 @@ static int aac_acquire_resources(struct aac_dev > *dev) > > } > > > > aac_adapter_enable_int(dev); > > + /*max msix may change after EEH > > + * Re-assign vectors to fibs > > + */ > > + for (i = 0; > > + i < (dev->scsi_host_ptr->can_queue + > AAC_NUM_MGT_FIB); > > + i++) { > > + if ((dev->max_msix == 1) || > > + (i > ((dev->scsi_host_ptr->can_queue + > AAC_NUM_MGT_FIB - 1) > > + - dev->vector_cap))) { > > + dev->fibs[i].vector_no = 0; > > + } else { > > + dev->fibs[i].vector_no = vector; > > + vector++; > > + if (vector == dev->max_msix) > > + vector = 1; > > + } > > + } > > The above hunk added code looks identical to the part you have just added > in 02/10 "aacraid: Fix RRQ overload" could you make this to function ? > Thanks, tomash Yes, I will make the necessary changes. > > > > - if (!dev->sync_mode) > > + if (!dev->sync_mode) { > > + /* After EEH recovery or suspend resume, max_msix count > > + * may change, therfore updating in init as well. > > + */ > > aac_adapter_start(dev); > > + dev->init->Sa_MSIXVectors = cpu_to_le32(dev->max_msix); > > + } > > return 0; > > > > error_iounmap: Regards, Raghava Aditya -- 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
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c index 0147210..f88f1132 100644 --- a/drivers/scsi/aacraid/linit.c +++ b/drivers/scsi/aacraid/linit.c @@ -1355,6 +1355,7 @@ static int aac_acquire_resources(struct aac_dev *dev) int i, j; int instance = dev->id; const char *name = dev->name; + int vector = 0; unsigned long status; /* * First clear out all interrupts. Then enable the one's that we @@ -1409,9 +1410,31 @@ static int aac_acquire_resources(struct aac_dev *dev) } aac_adapter_enable_int(dev); + /*max msix may change after EEH + * Re-assign vectors to fibs + */ + for (i = 0; + i < (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB); + i++) { + if ((dev->max_msix == 1) || + (i > ((dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB - 1) + - dev->vector_cap))) { + dev->fibs[i].vector_no = 0; + } else { + dev->fibs[i].vector_no = vector; + vector++; + if (vector == dev->max_msix) + vector = 1; + } + } - if (!dev->sync_mode) + if (!dev->sync_mode) { + /* After EEH recovery or suspend resume, max_msix count + * may change, therfore updating in init as well. + */ aac_adapter_start(dev); + dev->init->Sa_MSIXVectors = cpu_to_le32(dev->max_msix); + } return 0; error_iounmap: