From owner-freebsd-net@freebsd.org Fri Jul 21 05:23:20 2017 Return-Path: Delivered-To: freebsd-net@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 731D8D7FD44 for ; Fri, 21 Jul 2017 05:23:20 +0000 (UTC) (envelope-from mmacy@llnw.com) Received: from mail-it0-x22a.google.com (mail-it0-x22a.google.com [IPv6:2607:f8b0:4001:c0b::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4109063E84 for ; Fri, 21 Jul 2017 05:23:20 +0000 (UTC) (envelope-from mmacy@llnw.com) Received: by mail-it0-x22a.google.com with SMTP id h199so2679651ith.1 for ; Thu, 20 Jul 2017 22:23:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=llnw.com; s=google; h=mime-version:from:date:message-id:subject:to; bh=c9dxUGqxOHqduKxyQiba1nPMeZsyarROXggqkSuGESE=; b=YkEx2cCZMXKf7WNWDo8mfE6W0tmWA7y5ABWsBmmKkTzPAv+g3tiRapOAFCyy0AnMIh cz8o7mj6j7lVnsswkfbyJO3FFObUzh+dFpYjs7sFGXdVfxfipoaNJKcx4A+qVVeQAzpV flAAQBpfbwoU+f+EZpbcYaB6ixLfdYXRFhHb0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=c9dxUGqxOHqduKxyQiba1nPMeZsyarROXggqkSuGESE=; b=tfl5j9h4jJQShVzIlhpb7+BH8S11/jnXrLtWHslrF8F+1olQnddGpPUmagL6t/v4Da sTMxmhm1v3UME4g5AIAW83lLYRMQ8vPE7s0aZGWoNBGRyeGo25e9IEFb7t4vjOkj51bW LayW8bY98/vbQqr2uSIb6yimVLYuPozYJ3uUTmkU3OJQ5Meq1BeYDyktngUEixy9Gg6F mY5jBfwh3X8+JhvV1sys5EdYbUZsILhlPe2OWnWo2XowDaNxgaSf6BG2s+O0lKZ4ubg4 arAccFBBzsKJG+7616XmsK7xdSDoSGHQfktZuUi/YPrR+T6GH1VjiG8Fal46rZ3IvA3R dACg== X-Gm-Message-State: AIVw113AHQoh/OkH80OJD0FWXrO3MMHZUcwbQZ9hIR/Xal1XYv4KCTaJ OsJP2G/Q1dPWOw6BFzTR9o4uxVmdSaBhDYzMnfGxYBigq1aogu/Z1jvHbAeIGZBOoQdusaqjKNM 2gLPoAR6QVBDUhGzg4Dk= X-Received: by 10.36.111.73 with SMTP id x70mr5777671itb.15.1500614599159; Thu, 20 Jul 2017 22:23:19 -0700 (PDT) MIME-Version: 1.0 Received: by 10.79.133.194 with HTTP; Thu, 20 Jul 2017 22:23:18 -0700 (PDT) From: Matt Macy Date: Thu, 20 Jul 2017 22:23:18 -0700 Message-ID: Subject: locking anti-patterns To: freebsd-net@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.23 X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Jul 2017 05:23:20 -0000 The immediate solution that anyone could have made is to not call stats collection (or any other routines that use this dubious synchronization mechanism) from swi context: https://github.com/mattmacy/networking/commit/421da0083e4325e242bdece18fa198 e1a96a6c67 The *_acquire_swfw code seems to be a common anti-pattern amongst drivers. I found something very similar in bxe. My hypothesis in both cases is that they rolled their own simply because they don't know any better. Understanding the locking hierarchy on either Linux or FreeBSD is something that we all have to do a better job of educating people on the periphery of kernel development about. I have no words to describe calling DELAY(50) up to 200 times in a row to poll for release of device resources that I would use in a durable medium such as this. This "lock" serializes: i2c read/write EEPROM access PHY _read_/write As an immediate addendum we want to assert that no real locks are held while we're engaged in these shenanigans: https://github.com/mattmacy/networking/commit/5437e3109bbd0c21a5d4bfcc3d807f 20c6ee5751 And to avoid further foot shooting we'll want to assert that these routines are never called from swi context or one serving as an ithread. Eventually these constructs should be replaced by sx locks and the DELAYs replaced with sleeps. -- [image: Limelight Networks] Matt Macy - *Consultant* +1 650 440 8947 www.limelight.com Delivering Faster Better Join the conversation at Limelight Connect [image: Facebook] [image: Facebook] [image: LinkedIn] [image: Twitter] -- The information in this message may be confidential. It is intended solely for the addressee(s). If you are not the intended recipient, any disclosure, copying or distribution of the message, or any action or omission taken by you in reliance on it, is prohibited and may be unlawful. Please immediately contact the sender if you have received this message in error.