From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 87381B3BB for ; Mon, 22 Sep 2014 12:19:00 +0200 (CEST) Received: from [2001:470:8:a08:18c5:c64e:4bf:67a] (helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1XW0nh-0001ZD-Cr; Mon, 22 Sep 2014 06:25:02 -0400 Date: Mon, 22 Sep 2014 06:24:55 -0400 From: Neil Horman To: "Wodkowski, PawelX" Message-ID: <20140922102455.GA25406@hmsreliant.think-freely.org> References: <1410963713-13837-1-git-send-email-pawelx.wodkowski@intel.com> <1410963713-13837-3-git-send-email-pawelx.wodkowski@intel.com> <20140917151304.GD4213@localhost.localdomain> <20140918160234.GJ20389@hmsreliant.think-freely.org> <20140919172907.GE12897@hmsreliant.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: "dev@dpdk.org" , "Jastrzebski, MichalX K" Subject: Re: [dpdk-dev] [PATCH 2/2] bond: add mode 4 support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Sep 2014 10:19:00 -0000 On Mon, Sep 22, 2014 at 06:26:21AM +0000, Wodkowski, PawelX wrote: > > I think that will work, but I believe you're making it more complicated (and > > less reusable) than it needs to be. What I think you really need to do is > > create a new rte api call, rte_eal_alarm_cancel_sync (something like the > > equivalent of del_timer_sync in linux, that wraps up the > > while(rte_eal_alarm_cancel(...) == 0) {rte_pause} in its own function (so other > > call sites can use it, as I don't think this is an uncommon problem), Then just > > create a bonding-internal state flag to signal the periodic callback that it > > shouldn't re-arm the timer. That way all you have to do is set the flag, and > > call rte_eal_alarm_cancel_sync, and you're done. And other applications will be > > able to handle this common type of operation as well > > > > Neil > > I agree with you that alarms should be upgraded but this need to discussed and > tested. Now there is not time for that. > Nak, thats a completely broken argument. I've just demonstrated to you a race condition in the driver you are submitting. Granted it stems from a design lmitation in the alarm subsystem, but its what we all have to work with, and we can work around it and make the driver safe. To say there is "no time" to fix it, implies to me that you care more about just slamming your code in than making anything work properly. What exactly is your rush that makes you think its more important for the code to be merged than fixing it to work correctly? Neil