From owner-freebsd-wireless@FreeBSD.ORG Fri Aug 17 08:53:22 2012 Return-Path: Delivered-To: freebsd-wireless@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 23309106566B for ; Fri, 17 Aug 2012 08:53:22 +0000 (UTC) (envelope-from bschmidt@techwires.net) Received: from mail-lpp01m010-f54.google.com (mail-lpp01m010-f54.google.com [209.85.215.54]) by mx1.freebsd.org (Postfix) with ESMTP id 913AC8FC15 for ; Fri, 17 Aug 2012 08:53:21 +0000 (UTC) Received: by lage12 with SMTP id e12so2348930lag.13 for ; Fri, 17 Aug 2012 01:53:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-originating-ip:in-reply-to:references:date :message-id:subject:from:to:cc:content-type:x-gm-message-state; bh=dN++O+r520epph9byzv69gopGcy/iULZmSC1ZUEkIbw=; b=gCnXj45lMLJ80FLs0C3MU4+YSH77suKZrLD9HvjURGMU6cNdq+smsexU06sM9zpkQH MPNGvfGGv42KUwftvzQmPXDt1aCxNrDDHPE58Y1ovfxunrbU8PMN1ZdIJyYRdsBZ6io6 1f/ZeFOVD9xmFwRhjGTfb/6lAzW5XAM01jmXtRqNkJXPraOiCfJUH1a88Zc65uFFExaa fYxUIW3Pl+/jugnBM9ygOiTX8LebRSz+by8CvpjXMNi2K0QFBZW96MLUflIL+VXC09ak 0GzeaVSQNHtLFe/9qMSYS35uSBOnOEjnWptJft660DayEKBCk3bqTvSYi2qMijRGEzvF mRQg== MIME-Version: 1.0 Received: by 10.112.102.10 with SMTP id fk10mr1896756lbb.39.1345193600312; Fri, 17 Aug 2012 01:53:20 -0700 (PDT) Received: by 10.152.144.66 with HTTP; Fri, 17 Aug 2012 01:53:20 -0700 (PDT) X-Originating-IP: [79.140.39.245] In-Reply-To: References: Date: Fri, 17 Aug 2012 10:53:20 +0200 Message-ID: From: Bernhard Schmidt To: PseudoCylon Content-Type: text/plain; charset=ISO-8859-1 X-Gm-Message-State: ALoCoQmxybFVW5LsVTlvKAwLtOE7+x3n2Q7Zbx97g0u6YEtPMF+gICrlOwilCLrTrOEyk/GIe9Bg Cc: freebsd-wireless@freebsd.org, Kim Culhan Subject: Re: (ANother) stall fixed, please update to HEAD X-BeenThere: freebsd-wireless@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussions of 802.11 stack, tools device driver development." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Aug 2012 08:53:22 -0000 On Thu, Aug 9, 2012 at 11:32 AM, PseudoCylon wrote: >> ------------------------------ >> >> Message: 5 >> Date: Tue, 7 Aug 2012 12:34:52 -0400 >> From: Kim Culhan >> Subject: Re: (ANother) stall fixed, please update to HEAD >> To: Adrian Chadd >> Cc: freebsd-wireless@freebsd.org >> Message-ID: >> >> Content-Type: text/plain; charset=ISO-8859-1 >> >> >>> Yup. Would you be able to work with PseudoCylon and test his >>> ieee80211_iterate_nodes() patch? I'd like verification that it fixes >>> it for you before I tidy it up and commit it to -HEAD. >> >> iter.patch and iter2.patch cannot be applied in that order >> soo.. PseudoCylon could you please generate a diff >> against -HEAD from your present local source? >> > > No it won't. Only one of them need to apply. I guess I didn't explain well. > > iter.path only print outs debug message when the array overflowed > (most unlikely it will). At this moment, this is what we need. > > iter2.patch does iter.patch + revert changes + abort iterating just > for piece of mind. Probably this is unnecessary. The code need to be > patched in the way the array won't overflow if it ever happens. (I > leave it to committers what to commit.) > > The attached patch can be applied over iter.patch > > Sorry for the confusion. Sorry for jumping in that late, but I've a bit of a bellyache with that change especially the one which got committed. You've turned a totally safe function call into one which can fail, and malloc will fail eventually, therefore all callers need to either handle the error case or need to clearly state that it is not necessary to do something. I have the feeling that you've introduced some cases where due to lost calls the whole system might be hanging (not as hard as a deadlock, but hard enough to require manual intervention) by not doing a state change or something. Basically, all you've tried to achieve here is an additional unlock to avoid a deadlock when the called function itself needs to acquire the lock, performance aside, I'd really prefer if we're simply adding the unlock and go back to the "restart" approach. At least until someone either fixes the callers or can come up with a safe enough solution. Thanks -- Bernhard