| Message ID | 20180315025724.GB9973@bombadil.infradead.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
T24gV2VkLCAyMDE4LTAzLTE0IGF0IDE5OjU3IC0wNzAwLCBNYXR0aGV3IFdpbGNveCB3cm90ZToN Cj4gRnJvbTogTWF0dGhldyBXaWxjb3ggPG1hd2lsY294QG1pY3Jvc29mdC5jb20+DQo+IA0KPiBU aGUgbWx4NSBkcml2ZXIgY2FsbHMgaWRhX3ByZV9nZXQoKSBpbiBhIGxvb3AgZm9yIG5vIHJlYWRp bHkgYXBwYXJlbnQNCj4gcmVhc29uLiAgVGhlIGRyaXZlciB1c2VzIGlkYV9zaW1wbGVfZ2V0KCkg d2hpY2ggd2lsbCBjYWxsDQo+IGlkYV9wcmVfZ2V0KCkNCj4gYnkgaXRzZWxmIGFuZCB0aGVyZSdz IG5vIG5lZWQgdG8gdXNlIGlkYV9wcmVfZ2V0KCkgdW5sZXNzIHVzaW5nDQo+IGlkYV9nZXRfbmV3 KCkuDQo+IA0KDQpIaSBNYXR0aGV3LA0KDQpJcyB0aGlzIGlzIGNhdXNpbmcgYW55IGlzc3VlcyA/ IG9yIGp1c3QgYSBzaW1wbGUgY2xlYW51cCA/DQoNCkFkZGluZyBNYW9yLCB0aGUgYXV0aG9yIG9m IHRoaXMgY2hhbmdlLA0KDQpJIGJlbGlldmUgdGhlIGlkZWEgaXMgdG8gc3BlZWQgdXAgaW5zZXJ0 X2Z0ZSAod2hpY2ggY2FsbHMNCmlkYV9zaW1wbGVfZ2V0KSBzaW5jZSBpbnNlcnRfZnRlIHJ1bnMg dW5kZXIgdGhlIEZURSB3cml0ZSBzZW1hcGhvcmUsDQppbiB0aGlzIGNhc2UgaWYgaWRhX3ByZV9n ZXQgd2FzIHN1Y2Nlc3NmdWwgYmVmb3JlIHRha2luZyB0aGUgc2VtYXBob3JlDQpmb3IgYWxsIHRo ZSBGVEUgbm9kZXMgaW4gdGhlIGxvb3AsIHRoaXMgd2lsbCBiZSBhIGh1Z2Ugd2luIGZvcg0KaWRh X3NpbXBsZV9nZXQgd2hpY2ggd2lsbCBpbW1lZGlhdGVseSByZXR1cm4gc3VjY2VzcyB3aXRob3V0 IGV2ZW4NCnRyeWluZyB0byBhbGxvY2F0ZS4NCg0Kc28gaXQgaXMgYSBiZXN0IGVmZm9ydCB0byBz cGVlZCB1cCBjcml0aWNhbCBwYXRoLg0KDQpNYW9yLCBpZiB0aGlzIGlzIHJlYWxseSB0aGUgY2Fz ZSBhbmQgdGhpcyBpcyBub3QgY2F1c2luZyBhbnkgaXNzdWVzLA0KdGhlbiB3ZSBuZWVkIHRvIGNv bnNpZGVyIGFkZGluZyBhIGNvbW1lbnQuDQoNCg0KPiBTaWduZWQtb2ZmLWJ5OiBNYXR0aGV3IFdp bGNveCA8bWF3aWxjb3hAbWljcm9zb2Z0LmNvbT4NCj4gDQo+IGRpZmYgLS1naXQgYS9kcml2ZXJz L25ldC9ldGhlcm5ldC9tZWxsYW5veC9tbHg1L2NvcmUvZnNfY29yZS5jDQo+IGIvZHJpdmVycy9u ZXQvZXRoZXJuZXQvbWVsbGFub3gvbWx4NS9jb3JlL2ZzX2NvcmUuYw0KPiBpbmRleCAxMGUxNjM4 MWYyMGEuLjNiYTA3YzcwOTZlZiAxMDA2NDQNCj4gLS0tIGEvZHJpdmVycy9uZXQvZXRoZXJuZXQv bWVsbGFub3gvbWx4NS9jb3JlL2ZzX2NvcmUuYw0KPiArKysgYi9kcml2ZXJzL25ldC9ldGhlcm5l dC9tZWxsYW5veC9tbHg1L2NvcmUvZnNfY29yZS5jDQo+IEBAIC0xNjQ3LDcgKzE2NDcsNiBAQCB0 cnlfYWRkX3RvX2V4aXN0aW5nX2ZnKHN0cnVjdCBtbHg1X2Zsb3dfdGFibGUNCj4gKmZ0LA0KPiAg DQo+ICAJbGlzdF9mb3JfZWFjaF9lbnRyeShpdGVyLCBtYXRjaF9oZWFkLCBsaXN0KSB7DQo+ICAJ CW5lc3RlZF9kb3duX3JlYWRfcmVmX25vZGUoJml0ZXItPmctPm5vZGUsDQo+IEZTX0xPQ0tfUEFS RU5UKTsNCj4gLQkJaWRhX3ByZV9nZXQoJml0ZXItPmctPmZ0ZV9hbGxvY2F0b3IsIEdGUF9LRVJO RUwpOw0KPiAgCX0NCj4gIA0KPiAgc2VhcmNoX2FnYWluX2xvY2tlZDoNCj4g -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 15, 2018 at 11:58:07PM +0000, Saeed Mahameed wrote: > On Wed, 2018-03-14 at 19:57 -0700, Matthew Wilcox wrote: > > From: Matthew Wilcox <mawilcox@microsoft.com> > > > > The mlx5 driver calls ida_pre_get() in a loop for no readily apparent > > reason. The driver uses ida_simple_get() which will call > > ida_pre_get() > > by itself and there's no need to use ida_pre_get() unless using > > ida_get_new(). > > > > Hi Matthew, > > Is this is causing any issues ? or just a simple cleanup ? I'm removing the API. At the end of this cleanup, there will be no more preallocation; instead we will rely on the slab allocator not sucking. > Adding Maor, the author of this change, > > I believe the idea is to speed up insert_fte (which calls > ida_simple_get) since insert_fte runs under the FTE write semaphore, > in this case if ida_pre_get was successful before taking the semaphore > for all the FTE nodes in the loop, this will be a huge win for > ida_simple_get which will immediately return success without even > trying to allocate. I think that's misguided. The IDA allocator is only going to allocate memory once in every 1024 allocations. Also, it does try to allocate, even if there are preallocated nodes. So you're just wasting time, unfortunately. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
T24gVGh1LCAyMDE4LTAzLTE1IGF0IDE4OjMwIC0wNzAwLCBNYXR0aGV3IFdpbGNveCB3cm90ZToN Cj4gT24gVGh1LCBNYXIgMTUsIDIwMTggYXQgMTE6NTg6MDdQTSArMDAwMCwgU2FlZWQgTWFoYW1l ZWQgd3JvdGU6DQo+ID4gT24gV2VkLCAyMDE4LTAzLTE0IGF0IDE5OjU3IC0wNzAwLCBNYXR0aGV3 IFdpbGNveCB3cm90ZToNCj4gPiA+IEZyb206IE1hdHRoZXcgV2lsY294IDxtYXdpbGNveEBtaWNy b3NvZnQuY29tPg0KPiA+ID4gDQo+ID4gPiBUaGUgbWx4NSBkcml2ZXIgY2FsbHMgaWRhX3ByZV9n ZXQoKSBpbiBhIGxvb3AgZm9yIG5vIHJlYWRpbHkNCj4gPiA+IGFwcGFyZW50DQo+ID4gPiByZWFz b24uICBUaGUgZHJpdmVyIHVzZXMgaWRhX3NpbXBsZV9nZXQoKSB3aGljaCB3aWxsIGNhbGwNCj4g PiA+IGlkYV9wcmVfZ2V0KCkNCj4gPiA+IGJ5IGl0c2VsZiBhbmQgdGhlcmUncyBubyBuZWVkIHRv IHVzZSBpZGFfcHJlX2dldCgpIHVubGVzcyB1c2luZw0KPiA+ID4gaWRhX2dldF9uZXcoKS4NCj4g PiA+IA0KPiA+IA0KPiA+IEhpIE1hdHRoZXcsDQo+ID4gDQo+ID4gSXMgdGhpcyBpcyBjYXVzaW5n IGFueSBpc3N1ZXMgPyBvciBqdXN0IGEgc2ltcGxlIGNsZWFudXAgPw0KPiANCj4gSSdtIHJlbW92 aW5nIHRoZSBBUEkuICBBdCB0aGUgZW5kIG9mIHRoaXMgY2xlYW51cCwgdGhlcmUgd2lsbCBiZSBu bw0KPiBtb3JlDQo+IHByZWFsbG9jYXRpb247IGluc3RlYWQgd2Ugd2lsbCByZWx5IG9uIHRoZSBz bGFiIGFsbG9jYXRvciBub3QNCj4gc3Vja2luZy4NCj4gDQoNCk9rLCBTZWVtcyByZWFzb25hYmxl LCBJIGFtIG9rIHdpdGggdGhpcy4NCg0KPiA+IEFkZGluZyBNYW9yLCB0aGUgYXV0aG9yIG9mIHRo aXMgY2hhbmdlLA0KPiA+IA0KPiA+IEkgYmVsaWV2ZSB0aGUgaWRlYSBpcyB0byBzcGVlZCB1cCBp bnNlcnRfZnRlICh3aGljaCBjYWxscw0KPiA+IGlkYV9zaW1wbGVfZ2V0KSBzaW5jZSBpbnNlcnRf ZnRlIHJ1bnMgdW5kZXIgdGhlIEZURSB3cml0ZQ0KPiA+IHNlbWFwaG9yZSwNCj4gPiBpbiB0aGlz IGNhc2UgaWYgaWRhX3ByZV9nZXQgd2FzIHN1Y2Nlc3NmdWwgYmVmb3JlIHRha2luZyB0aGUNCj4g PiBzZW1hcGhvcmUNCj4gPiBmb3IgYWxsIHRoZSBGVEUgbm9kZXMgaW4gdGhlIGxvb3AsIHRoaXMg d2lsbCBiZSBhIGh1Z2Ugd2luIGZvcg0KPiA+IGlkYV9zaW1wbGVfZ2V0IHdoaWNoIHdpbGwgaW1t ZWRpYXRlbHkgcmV0dXJuIHN1Y2Nlc3Mgd2l0aG91dCBldmVuDQo+ID4gdHJ5aW5nIHRvIGFsbG9j YXRlLg0KPiANCj4gSSB0aGluayB0aGF0J3MgbWlzZ3VpZGVkLiAgVGhlIElEQSBhbGxvY2F0b3Ig aXMgb25seSBnb2luZyB0bw0KPiBhbGxvY2F0ZQ0KPiBtZW1vcnkgb25jZSBpbiBldmVyeSAxMDI0 IGFsbG9jYXRpb25zLiAgQWxzbywgaXQgZG9lcyB0cnkgdG8NCj4gYWxsb2NhdGUsDQo+IGV2ZW4g aWYgdGhlcmUgYXJlIHByZWFsbG9jYXRlZCBub2Rlcy4gIFNvIHlvdSdyZSBqdXN0IHdhc3Rpbmcg dGltZSwNCj4gdW5mb3J0dW5hdGVseS4NCj4gDQoNCldlbGwganVzdCBieSBsb29raW5nIGF0IHRo ZSBjb2RlIHlvdSBjYW4gdGVsbCBmb3Igc3VyZSB0aGF0IA0KdHdvIGNvbnNlY3V0aXZlIGNhbGxz IHRvIGlkYV9wcmVfZ2V0IHdpbGwgcmVzdWx0IGluIG9uZSBhbGxvY2F0aW9uDQpvbmx5Lg0KZHVl IHRvICJpZiAoIXRoaXNfY3B1X3JlYWQoaWRhX2JpdG1hcCkpIg0KDQpidXQgaSBkaWRuJ3QgZGln IGludG8gZGV0YWlscyBhbmQgZGlkbid0IGdvIHRocm91Z2ggdGhlIHdob2xlDQppZGFfZ2V0X25l d19hYm92ZSwgc28gaSB3aWxsIGNvdW50IG9uIHlvdXIganVkZ21lbnQgaGVyZS4NCg0KU3RpbGwg aSB3b3VsZCBsaWtlIHRvIHdhaXQgZm9yIE1hb3IncyBpbnB1dCBoZXJlLCB0aGUgYXV0aG9yLi4N CkkgV2lsbCBwaW5nIGhpbSB0b2RheS4NCg0KVGhhbmtzLA0KU2FlZWQu -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3/20/2018 5:29 AM, Saeed Mahameed wrote: > On Thu, 2018-03-15 at 18:30 -0700, Matthew Wilcox wrote: >> On Thu, Mar 15, 2018 at 11:58:07PM +0000, Saeed Mahameed wrote: >>> On Wed, 2018-03-14 at 19:57 -0700, Matthew Wilcox wrote: >>>> From: Matthew Wilcox <mawilcox@microsoft.com> >>>> >>>> The mlx5 driver calls ida_pre_get() in a loop for no readily >>>> apparent >>>> reason. The driver uses ida_simple_get() which will call >>>> ida_pre_get() >>>> by itself and there's no need to use ida_pre_get() unless using >>>> ida_get_new(). >>>> >>> Hi Matthew, >>> >>> Is this is causing any issues ? or just a simple cleanup ? >> I'm removing the API. At the end of this cleanup, there will be no >> more >> preallocation; instead we will rely on the slab allocator not >> sucking. >> > Ok, Seems reasonable, I am ok with this. > >>> Adding Maor, the author of this change, >>> >>> I believe the idea is to speed up insert_fte (which calls >>> ida_simple_get) since insert_fte runs under the FTE write >>> semaphore, >>> in this case if ida_pre_get was successful before taking the >>> semaphore >>> for all the FTE nodes in the loop, this will be a huge win for >>> ida_simple_get which will immediately return success without even >>> trying to allocate. >> I think that's misguided. The IDA allocator is only going to >> allocate >> memory once in every 1024 allocations. Also, it does try to >> allocate, >> even if there are preallocated nodes. So you're just wasting time, >> unfortunately. >> > Well just by looking at the code you can tell for sure that > two consecutive calls to ida_pre_get will result in one allocation > only. > due to "if (!this_cpu_read(ida_bitmap))" > > but i didn't dig into details and didn't go through the whole > ida_get_new_above, so i will count on your judgment here. > > Still i would like to wait for Maor's input here, the author.. > I Will ping him today. > > Thanks, > Saeed. Saeed, Matan and I okay with this fix as well, it looks like it shouldn't impact on the insertion rate. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Maor Gottlieb <maorg@mellanox.com> Date: Tue, 20 Mar 2018 14:41:49 +0200 > Saeed, Matan and I okay with this fix as well, it looks like it > shouldn't impact on the insertion rate. I've applied this to net-next, thanks everyone. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 20, 2018 at 10:46:20AM -0400, David Miller wrote: > From: Maor Gottlieb <maorg@mellanox.com> > Date: Tue, 20 Mar 2018 14:41:49 +0200 > > > Saeed, Matan and I okay with this fix as well, it looks like it > > shouldn't impact on the insertion rate. > > I've applied this to net-next, thanks everyone. Thanks, Dave. I realised why this made sense when it was originally written. Before December 2016 (commit 7ad3d4d85c7a), ida_pre_get used to allocate one bitmap per ida. I moved it to a percpu variable, and at that point this stopped making sense. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c index 10e16381f20a..3ba07c7096ef 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c @@ -1647,7 +1647,6 @@ try_add_to_existing_fg(struct mlx5_flow_table *ft, list_for_each_entry(iter, match_head, list) { nested_down_read_ref_node(&iter->g->node, FS_LOCK_PARENT); - ida_pre_get(&iter->g->fte_allocator, GFP_KERNEL); } search_again_locked: