From owner-freebsd-net@FreeBSD.ORG Thu Jan 29 15:41:09 2015 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id A57C6BC5 for ; Thu, 29 Jan 2015 15:41:09 +0000 (UTC) Received: from phabric-backend.isc.freebsd.org (phabric-backend.isc.freebsd.org [IPv6:2001:4f8:3:ffe0:406a:0:50:2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 7E3B8BD9 for ; Thu, 29 Jan 2015 15:41:09 +0000 (UTC) Received: from phabric-backend.isc.freebsd.org (phabric-backend.isc.freebsd.org [127.0.1.5]) by phabric-backend.isc.freebsd.org (8.14.9/8.14.9) with ESMTP id t0TFf9vn066563 for ; Thu, 29 Jan 2015 15:41:09 GMT (envelope-from mat@phabric-backend.isc.freebsd.org) Received: (from root@localhost) by phabric-backend.isc.freebsd.org (8.14.9/8.14.9/Submit) id t0TFf9Vs066562; Thu, 29 Jan 2015 15:41:09 GMT (envelope-from mat) Date: Thu, 29 Jan 2015 15:41:09 +0000 To: freebsd-net@freebsd.org From: "rrs (Randall Stewart)" Subject: [Differential] [Commented On] D1711: Changes to the callout code to restore active semantics and also add a test-framework and test to validate thecallout code (and potentially for use by other tests). Message-ID: <6b9877fabfc054b3bce9c3a97f7e0455@localhost.localdomain> X-Priority: 3 Thread-Topic: D1711: Changes to the callout code to restore active semantics and also add a test-framework and test to validate the callout code (and potentially for use by other tests). X-Herald-Rules: none X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: In-Reply-To: References: Thread-Index: Y2JjMTcyODJkYzgxM2NkZDFjY2RhOGRmMTlkIFTKVJU= X-Phabricator-Sent-This-Message: Yes X-Mail-Transport-Agent: MetaMTA X-Auto-Response-Suppress: All X-Phabricator-Mail-Tags: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="utf-8" X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Jan 2015 15:41:09 -0000 rrs added a comment. To answer jhb and kostikbel 1) Yes this does address the two issues that Hans re-write of the callout system did without changing the KPI. There may be other bugs as well, but with the test framework and the old code I could reproduce both issues (spin lock held to long and broken callout_active() semantic). The spin lock held to long I was able to get a vmcore and actually pinpoint *exactly* why it happened and *exactly* how the patch fixes it (as Gleb would say, no mystery ...) 2) Yes, its a good idea to split the test framework out as imp suggested, which I will do into test framework and callout changes. 3) As to simpler testing, yes we originally were running the test out of the kldload, but this causes at least two problems. It appears that the GIANT lock is held during kldload, any test that needs to do locking can experience LOR's and other fun stuff with Witness on. Second it becomes *much* harder to do multi-threaded testing. This is/was essential to find the bugs and reproduce them in the callout system. Basically you just put num_threads = 2 and you have two kthreads running the tests which helps located nasty races more easily ;-) 4) Imp, in an early version of the patch, complained I was not using the macros and what he had complained about was just code that I cut and paste. Thats what prompted me to fix and use the macro's properly throughout the code. Backing that out would be painful at this point so I see no need to split it out except to do a *lot* of extra work. REVISION DETAIL https://reviews.freebsd.org/D1711 To: rrs, gnn, rwatson, adrian, sbruno, lstewart, imp, hselasky Cc: jhb, kostikbel, emaste, delphij, neel, erj, freebsd-net