From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cong Wang Subject: Re: [PATCH net] ipv4: disable BH in set_ping_group_range() Date: Thu, 20 Oct 2016 14:00:20 -0700 Message-ID: References: <1476984408.7065.21.camel@edumazet-glaptop3.roam.corp.google.com> <1476996209.7065.28.camel@edumazet-glaptop3.roam.corp.google.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: David Miller , netdev , Eric Salo To: Eric Dumazet Return-path: Received: from mail-it0-f41.google.com ([209.85.214.41]:35083 "EHLO mail-it0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755474AbcJTVAn (ORCPT ); Thu, 20 Oct 2016 17:00:43 -0400 Received: by mail-it0-f41.google.com with SMTP id 4so195439241itv.0 for ; Thu, 20 Oct 2016 14:00:43 -0700 (PDT) In-Reply-To: <1476996209.7065.28.camel@edumazet-glaptop3.roam.corp.google.com> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, Oct 20, 2016 at 1:43 PM, Eric Dumazet wrote: > On Thu, 2016-10-20 at 12:44 -0700, Cong Wang wrote: >> On Thu, Oct 20, 2016 at 12:40 PM, Cong Wang wrote: >> > On Thu, Oct 20, 2016 at 12:32 PM, Cong Wang wrote: >> >> On Thu, Oct 20, 2016 at 10:26 AM, Eric Dumazet wrote: >> >>> From: Eric Dumazet >> >>> >> >>> In commit 4ee3bd4a8c746 ("ipv4: disable BH when changing ip local port >> >>> range") Cong added BH protection in set_local_port_range() but missed >> >>> that same fix was needed in set_ping_group_range() >> >> >> >> Don't know why ping_group_range shares the same lock with local_port_range... >> >> Perhaps just for saving a few bytes, but that is why I missed this place. >> > >> > Hold on... We clearly have typos there... Your fix is not correct. >> >> We need the attached patch, your patch should be reverted, because >> unlike local_port_range we never read it in BH context, no need to bother _bh. > > Well, we do not change this sysctl very often, so I am not sure why we > need different seqlocks to protect these ranges. > > Seems a waste of space really (per netns) Error prone vs. space saving, it's up to you... But clearly current code is still broken even after your patch. I will send a revert + previous typo fix.