From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8C03BC4741F for ; Mon, 28 Sep 2020 17:24:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4339E2100A for ; Mon, 28 Sep 2020 17:24:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=pensando.io header.i=@pensando.io header.b="sJXRxO3V" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726684AbgI1RYw (ORCPT ); Mon, 28 Sep 2020 13:24:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39008 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726672AbgI1RYw (ORCPT ); Mon, 28 Sep 2020 13:24:52 -0400 Received: from mail-pf1-x441.google.com (mail-pf1-x441.google.com [IPv6:2607:f8b0:4864:20::441]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3833BC0613D3 for ; Mon, 28 Sep 2020 10:24:52 -0700 (PDT) Received: by mail-pf1-x441.google.com with SMTP id f18so1640127pfa.10 for ; Mon, 28 Sep 2020 10:24:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pensando.io; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=dXB5sy6RuZTJPqx7JJLXNWe1a908CXA07ElMJ2w7R68=; b=sJXRxO3VX4I/D6OKhoVkZO/piViwl1rsupkYazfFzknNozgI8OsHjhRNoJktdix5sL XUePafekvxNj5dX2ZMNgPcxdgvjKnkbGxEJHb5M7MD6jsXR24Z16f7Q8EMF5gSrpcqV3 6mdJsMem+f4C7b9Iy8bk9humZkeCkuU0VDUAFi3VrIZ1AvBJykcO3j/m813G3+0PUELA 8jig6g1XAuBTfOqXNacgr0soT1uIdlZZ/QhywRu6HBMy1Ncr+lfoiCUVKIuNsV2E8yzC 6Etpltkq/5EZsG8x+m98hzY644sBlZD9g9rlktFFEXZxJkhYAnrqwKkSvnbtDB8ytbwH WhHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=dXB5sy6RuZTJPqx7JJLXNWe1a908CXA07ElMJ2w7R68=; b=Imo1r5oXAQ4Hw+bm2V3gRkXbEwpVrnXInWZsEqJ8J06uDapiWjvmK6kXvid08EO44J e9p6Krksd0FUGB+ug+zrDrZPijgeWJjkBezlIj1AGanIiY/BKRkfX2ERuMRBWbwXzZmZ 0nSgohsNH/nqXLtTM29KWxnDkbMYk0e0LD483Jpop8gOc/gRElhUYQ6RgRkXZfWRbM3I NGIZInlgQKv/A00AYBmEiN9cCjeudKdC3xrs7lG80W3wp7ED645NDTMaNX8pwYNOwVNG K6BWQIUdHeeXi1BE1iaYUohfFgPPLqmf5joe/+tSHAmE9d9eo/luaISEgMresB6t4Go8 aU1g== X-Gm-Message-State: AOAM5329LKx4TdxJdsN92JGyhjbYECEowyrppIWHuHVHceFaVoO3fD35 QNYtvzBKx99AvB0jpkHOtMagVw== X-Google-Smtp-Source: ABdhPJyoyl1fomYslG+xBdkpZIhAHLVX6EHj3huo2N9gQMS+dYAudkP3ZT0Owaa2qZhTwb+CGqQqNA== X-Received: by 2002:a63:784:: with SMTP id 126mr121408pgh.428.1601313891289; Mon, 28 Sep 2020 10:24:51 -0700 (PDT) Received: from Shannons-MacBook-Pro.local (static-50-53-47-17.bvtn.or.frontiernet.net. [50.53.47.17]) by smtp.gmail.com with ESMTPSA id u71sm2511314pfc.43.2020.09.28.10.24.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 28 Sep 2020 10:24:50 -0700 (PDT) Subject: Re: [patch 11/35] net: ionic: Replace in_interrupt() usage. To: Thomas Gleixner , LKML Cc: Peter Zijlstra , Linus Torvalds , Paul McKenney , Matthew Wilcox , Sebastian Andrzej Siewior , Pensando Drivers , "David S. Miller" , Jakub Kicinski , netdev@vger.kernel.org, Christian Benvenuti , Govindarajulu Varadarajan <_govind@gmx.com>, Jonathan Corbet , Mauro Carvalho Chehab , linux-doc@vger.kernel.org, Luc Van Oostenryck , Jay Cliburn , Chris Snook , Vishal Kulkarni , Jeff Kirsher , intel-wired-lan@lists.osuosl.org, Andrew Lunn , Heiner Kallweit , Russell King , Thomas Bogendoerfer , Solarflare linux maintainers , Edward Cree , Martin Habets , Jon Mason , Daniel Drake , Ulrich Kunitz , Kalle Valo , linux-wireless@vger.kernel.org, linux-usb@vger.kernel.org, Greg Kroah-Hartman , Arend van Spriel , Franky Lin , Hante Meuleman , Chi-Hsien Lin , Wright Feng , brcm80211-dev-list.pdl@broadcom.com, brcm80211-dev-list@cypress.com, Stanislav Yakovlev , Stanislaw Gruszka , Johannes Berg , Emmanuel Grumbach , Luca Coelho , Intel Linux Wireless , Jouni Malinen , Amitkumar Karwar , Ganapathi Bhat , Xinming Hu , libertas-dev@lists.infradead.org, Pascal Terjan , Ping-Ke Shih References: <20200927194846.045411263@linutronix.de> <20200927194920.918550822@linutronix.de> From: Shannon Nelson Message-ID: <5e4c3201-9d90-65b1-5c13-e2381445be1d@pensando.io> Date: Mon, 28 Sep 2020 10:24:47 -0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20200927194920.918550822@linutronix.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org On 9/27/20 12:48 PM, Thomas Gleixner wrote: > From: Sebastian Andrzej Siewior > > The in_interrupt() usage in this driver tries to figure out which context > may sleep and which context may not sleep. in_interrupt() is not really > suitable as it misses both preemption disabled and interrupt disabled > invocations from task context. > > Conditionals like that in driver code are frowned upon in general because > invocations of functions from invalid contexts might not be detected > as the conditional papers over it. > > ionic_lif_addr() can be called from: > > 1) ->ndo_set_rx_mode() which is under netif_addr_lock_bh()) so it must not > sleep. > > 2) Init and setup functions which are in fully preemptible task context. > > _ionic_lif_rx_mode() has only one call path with BH disabled. > > ionic_link_status_check_request() has two call paths: > > 1) NAPI which obviously cannot sleep > > 2) Setup which is again fully preemptible task context > > Add 'can_sleep' arguments to the affected functions and let the callers > provide the context instead of letting the functions deduce it. > > Signed-off-by: Sebastian Andrzej Siewior > Signed-off-by: Thomas Gleixner > Cc: Shannon Nelson > Cc: Pensando Drivers > Cc: "David S. Miller" > Cc: Jakub Kicinski > Cc: netdev@vger.kernel.org Acked-by: Shannon Nelson > --- > > While reviewing the callpaths, a couple of things were observed which could > be improved: > > - ionic_lif_deferred_work() can iterate over the list. There is no need > to schedule the work item after each iteration I think the original writer's intent was to avoid monopolizing the work thread for very long on any one cycle, with the thought that we'd be making more use of this than we currently are.  I'll address this. > > - ionic_link_status_check_request() could have ionic_deferred_work within > ionic_lif(). This would avoid memory allocation from NAPI. More > important, once IONIC_LIF_F_LINK_CHECK_REQUESTED is set and that alloc > fails, the link check never happens. Thanks, I'll fix up that error condition. > > - ionic_lif_handle_fw_down() sets IONIC_LIF_F_FW_RESET. Invokes then > ionic_lif_deinit() which only invokes cancel_work_sync() if > IONIC_LIF_F_FW_RESET is not set. I think the logic is wrong here as > the work must always be cancled. Also the list with ionic_deferred > work items needs a clean up. I'll look at that, thanks. sln