From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965400AbXCFRqa (ORCPT ); Tue, 6 Mar 2007 12:46:30 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S965404AbXCFRqa (ORCPT ); Tue, 6 Mar 2007 12:46:30 -0500 Received: from smtp.osdl.org ([65.172.181.24]:49967 "EHLO smtp.osdl.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965400AbXCFRq3 (ORCPT ); Tue, 6 Mar 2007 12:46:29 -0500 Date: Tue, 6 Mar 2007 09:41:33 -0800 (PST) From: Linus Torvalds To: Thomas Gleixner cc: Ingo Molnar , "Michael S. Tsirkin" , Adrian Bunk , Andrew Morton , Linux Kernel Mailing List , Michal Piotrowski , Emil Karlson , Soeren Sonnenburg , Len Brown Subject: Re: [PATCH] highres: do not run the TIMER_SOFTIRQ after switching to highres mode In-Reply-To: <1173202195.24738.350.camel@localhost.localdomain> Message-ID: References: <20070305015040.GJ3441@stusta.de> <1173138193.24738.236.camel@localhost.localdomain> <1173142948.24738.257.camel@localhost.localdomain> <20070306103315.GA4295@mellanox.co.il> <20070306103712.GA10942@elte.hu> <20070306104618.GA5792@mellanox.co.il> <20070306113225.GA22561@elte.hu> <1173202195.24738.350.camel@localhost.localdomain> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 6 Mar 2007, Thomas Gleixner wrote: > > /me pleads guilty ! > > Replacement patch below. Well, I already applied the original one that came through Andrew, so I really just wanted to note the coding style in general, and your fixed patch no longer applied ;) Also, you do end up testing for "not error" with a > if (tick_check_oneshot_change(!hrtimer_is_hres_enabled())) > - hrtimer_switch_to_hres(); > + if (!hrtimer_switch_to_hres()) > + return; which I guess is ok, if only because we simply don't care about what the exact error was. But it means that this particular code sequence ends up having the same problem (which is still fewer places than the original patch, so we're good). I personally hate the if (hrtimer_switch_to_hres() == SUCCESS) return; kind of syntax (it's just too long, and it's *not* obvious at all that SUCCESS is zero and that this is a "negative error or zero" kind of function, so it's actually *worse* than just doing what you did, but some projects seem to have that kind of approach. We could encourage people to do if (hrtimer_switch_to_hres() >= 0) return; which is fairly obviously a "success" case for a negative error value, but I'm not sure the extra typing really is worth it. Does anybody have any smart ideas that people might even be ok with following (just making things more cumbersome is anti-productive, so I don't want to have some stupid rule that everybody really hates)? Linus