diff for duplicates of <1492601312.3108.3.camel@spectralink.com> diff --git a/a/1.txt b/N1/1.txt index cc13580..a531006 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,20 +1,35 @@ -T24gV2VkLCAyMDE3LTA0LTE5IGF0IDEyOjMxICswMjAwLCBSaWNoYXJkIENvY2hyYW4gd3JvdGU6 -DQo+IENhbiB3ZSBwbGVhc2UgZml4IHRoaXMgaW4gYW5vdGhlciB3YXk/wqDCoFRoZXJlIGlzIG5v -IG5lZWQgdG8gaG9sZCB0aGUNCj4gc3BpbiBsb2NrIGR1cmluZyB0aGUgY2FsbGJhY2sgaW50byB0 -aGUgbmV0d29ya2luZyBzdGFjay7CoMKgSW5zdGVhZCwNCj4gaG93IGFib3V0IHRoZSBmb2xsb3dp -bmcgZGlmZiwgd2hpY2ggYWxzbyBmaXhlcyB0aGUgb3RoZXIgY2FsbCBzaXRlLi4uDQoNCg0KSSBh -Z3JlZSB0aGF0IHRoaXMgaXMgYSBiZXR0ZXIgd2F5IHRvIGZpeCBpdCAtIEkgdGhpbmsgdGhhdCB0 -aGlzIHdhcyB0aGUNCnB1cnBvc2Ugb2YgdGhlIHBhdGNoIHRoYXQgZ290IGhhbGZ3YXkgbWl4ZWQg -aW4uDQoNCkBAIC04NzQsMTQgKzg3NCwxNSBAQCBzdGF0aWMgdm9pZCBkZWNvZGVfcnh0cyhzdHJ1 -Y3QgZHA4MzY0MF9wcml2YXRlDQo+ICpkcDgzNjQwLA0KPiDCoAkJCXNoaHd0c3RhbXBzID0gc2ti -X2h3dHN0YW1wcyhza2IpOw0KPiDCoAkJCW1lbXNldChzaGh3dHN0YW1wcywgMCwNCj4gc2l6ZW9m -KCpzaGh3dHN0YW1wcykpOw0KPiDCoAkJCXNoaHd0c3RhbXBzLT5od3RzdGFtcCA9IG5zX3RvX2t0 -aW1lKHJ4dHMtDQo+ID5ucyk7DQo+IC0JCQluZXRpZl9yeF9uaShza2IpOw0KPiDCoAkJCWxpc3Rf -YWRkKCZyeHRzLT5saXN0LCAmZHA4MzY0MC0+cnhwb29sKTsNCj4gwqAJCQlicmVhazsNCj4gwqAJ -CX0NCj4gwqAJfQ0KPiDCoAlzcGluX3VubG9jaygmZHA4MzY0MC0+cnhfcXVldWUubG9jayk7DQo+ -IMKgDQo+IC0JaWYgKCFzaGh3dHN0YW1wcykNCj4gKwlpZiAoc2hod3RzdGFtcHMpDQo+ICsJCW5l -dGlmX3J4X25pKHNrYik7DQo+ICsJZWxzZQ0KPiDCoAkJbGlzdF9hZGRfdGFpbCgmcnh0cy0+bGlz -dCwgJmRwODM2NDAtPnJ4dHMpOw0KPiDCoG91dDoNCj4gwqAJc3Bpbl91bmxvY2tfaXJxcmVzdG9y -ZSgmZHA4MzY0MC0+cnhfbG9jaywgZmxhZ3MpOw0KDQpIZXJlIHdlIHN0aWxsIGhvbGQgcnhfbG9j -ayB3aGlsZSBkdXJpbmcgdGhlIGNhbGxiYWNrLCB3b3VsZG4ndCBpdCBiZQ0KYmVuZWZpY2lhbCB0 -byByZWxlYXNlIHRoYXQgZmlyc3Q/DQoNClN0ZWZhbg0K +On Wed, 2017-04-19 at 12:31 +0200, Richard Cochran wrote: +> Can we please fix this in another way? There is no need to hold the +> spin lock during the callback into the networking stack. Instead, +> how about the following diff, which also fixes the other call site... + + +I agree that this is a better way to fix it - I think that this was the +purpose of the patch that got halfway mixed in. + +@@ -874,14 +874,15 @@ static void decode_rxts(struct dp83640_private +> *dp83640, +> shhwtstamps = skb_hwtstamps(skb); +> memset(shhwtstamps, 0, +> sizeof(*shhwtstamps)); +> shhwtstamps->hwtstamp = ns_to_ktime(rxts- +> >ns); +> - netif_rx_ni(skb); +> list_add(&rxts->list, &dp83640->rxpool); +> break; +> } +> } +> spin_unlock(&dp83640->rx_queue.lock); +> +> - if (!shhwtstamps) +> + if (shhwtstamps) +> + netif_rx_ni(skb); +> + else +> list_add_tail(&rxts->list, &dp83640->rxts); +> out: +> spin_unlock_irqrestore(&dp83640->rx_lock, flags); + +Here we still hold rx_lock while during the callback, wouldn't it be +beneficial to release that first? + +Stefan diff --git a/a/content_digest b/N1/content_digest index be0d435..007b296 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -12,25 +12,40 @@ " kernel-janitors@vger.kernel.org <kernel-janitors@vger.kernel.org>\0" "\00:1\0" "b\0" - "T24gV2VkLCAyMDE3LTA0LTE5IGF0IDEyOjMxICswMjAwLCBSaWNoYXJkIENvY2hyYW4gd3JvdGU6\n" - "DQo+IENhbiB3ZSBwbGVhc2UgZml4IHRoaXMgaW4gYW5vdGhlciB3YXk/wqDCoFRoZXJlIGlzIG5v\n" - "IG5lZWQgdG8gaG9sZCB0aGUNCj4gc3BpbiBsb2NrIGR1cmluZyB0aGUgY2FsbGJhY2sgaW50byB0\n" - "aGUgbmV0d29ya2luZyBzdGFjay7CoMKgSW5zdGVhZCwNCj4gaG93IGFib3V0IHRoZSBmb2xsb3dp\n" - "bmcgZGlmZiwgd2hpY2ggYWxzbyBmaXhlcyB0aGUgb3RoZXIgY2FsbCBzaXRlLi4uDQoNCg0KSSBh\n" - "Z3JlZSB0aGF0IHRoaXMgaXMgYSBiZXR0ZXIgd2F5IHRvIGZpeCBpdCAtIEkgdGhpbmsgdGhhdCB0\n" - "aGlzIHdhcyB0aGUNCnB1cnBvc2Ugb2YgdGhlIHBhdGNoIHRoYXQgZ290IGhhbGZ3YXkgbWl4ZWQg\n" - "aW4uDQoNCkBAIC04NzQsMTQgKzg3NCwxNSBAQCBzdGF0aWMgdm9pZCBkZWNvZGVfcnh0cyhzdHJ1\n" - "Y3QgZHA4MzY0MF9wcml2YXRlDQo+ICpkcDgzNjQwLA0KPiDCoAkJCXNoaHd0c3RhbXBzID0gc2ti\n" - "X2h3dHN0YW1wcyhza2IpOw0KPiDCoAkJCW1lbXNldChzaGh3dHN0YW1wcywgMCwNCj4gc2l6ZW9m\n" - "KCpzaGh3dHN0YW1wcykpOw0KPiDCoAkJCXNoaHd0c3RhbXBzLT5od3RzdGFtcCA9IG5zX3RvX2t0\n" - "aW1lKHJ4dHMtDQo+ID5ucyk7DQo+IC0JCQluZXRpZl9yeF9uaShza2IpOw0KPiDCoAkJCWxpc3Rf\n" - "YWRkKCZyeHRzLT5saXN0LCAmZHA4MzY0MC0+cnhwb29sKTsNCj4gwqAJCQlicmVhazsNCj4gwqAJ\n" - "CX0NCj4gwqAJfQ0KPiDCoAlzcGluX3VubG9jaygmZHA4MzY0MC0+cnhfcXVldWUubG9jayk7DQo+\n" - "IMKgDQo+IC0JaWYgKCFzaGh3dHN0YW1wcykNCj4gKwlpZiAoc2hod3RzdGFtcHMpDQo+ICsJCW5l\n" - "dGlmX3J4X25pKHNrYik7DQo+ICsJZWxzZQ0KPiDCoAkJbGlzdF9hZGRfdGFpbCgmcnh0cy0+bGlz\n" - "dCwgJmRwODM2NDAtPnJ4dHMpOw0KPiDCoG91dDoNCj4gwqAJc3Bpbl91bmxvY2tfaXJxcmVzdG9y\n" - "ZSgmZHA4MzY0MC0+cnhfbG9jaywgZmxhZ3MpOw0KDQpIZXJlIHdlIHN0aWxsIGhvbGQgcnhfbG9j\n" - "ayB3aGlsZSBkdXJpbmcgdGhlIGNhbGxiYWNrLCB3b3VsZG4ndCBpdCBiZQ0KYmVuZWZpY2lhbCB0\n" - byByZWxlYXNlIHRoYXQgZmlyc3Q/DQoNClN0ZWZhbg0K + "On Wed, 2017-04-19 at 12:31 +0200, Richard Cochran wrote:\n" + "> Can we please fix this in another way?\302\240\302\240There is no need to hold the\n" + "> spin lock during the callback into the networking stack.\302\240\302\240Instead,\n" + "> how about the following diff, which also fixes the other call site...\n" + "\n" + "\n" + "I agree that this is a better way to fix it - I think that this was the\n" + "purpose of the patch that got halfway mixed in.\n" + "\n" + "@@ -874,14 +874,15 @@ static void decode_rxts(struct dp83640_private\n" + "> *dp83640,\n" + "> \302\240\t\t\tshhwtstamps = skb_hwtstamps(skb);\n" + "> \302\240\t\t\tmemset(shhwtstamps, 0,\n" + "> sizeof(*shhwtstamps));\n" + "> \302\240\t\t\tshhwtstamps->hwtstamp = ns_to_ktime(rxts-\n" + "> >ns);\n" + "> -\t\t\tnetif_rx_ni(skb);\n" + "> \302\240\t\t\tlist_add(&rxts->list, &dp83640->rxpool);\n" + "> \302\240\t\t\tbreak;\n" + "> \302\240\t\t}\n" + "> \302\240\t}\n" + "> \302\240\tspin_unlock(&dp83640->rx_queue.lock);\n" + "> \302\240\n" + "> -\tif (!shhwtstamps)\n" + "> +\tif (shhwtstamps)\n" + "> +\t\tnetif_rx_ni(skb);\n" + "> +\telse\n" + "> \302\240\t\tlist_add_tail(&rxts->list, &dp83640->rxts);\n" + "> \302\240out:\n" + "> \302\240\tspin_unlock_irqrestore(&dp83640->rx_lock, flags);\n" + "\n" + "Here we still hold rx_lock while during the callback, wouldn't it be\n" + "beneficial to release that first?\n" + "\n" + Stefan -e4502c273e743cea8bbce093a41c78a4e738327c85d7c03ff3096ce326437401 +3e4c3aa1ae45a7ff403574f104bf110fae11f9916855a654c05b4c4a9ad6fae9
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.