From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46569) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhDfs-0004bb-Gg for qemu-devel@nongnu.org; Tue, 06 Sep 2016 06:32:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bhDfp-0003we-UY for qemu-devel@nongnu.org; Tue, 06 Sep 2016 06:32:15 -0400 References: <034e2688-83f6-23df-4e4f-da68ef1f580a@gmail.com> From: Dmitry Osipenko Message-ID: Date: Tue, 6 Sep 2016 13:32:04 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v15 00/15] PTimer fixes/features and ARM MPTimer conversion List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers , qemu-arm , Peter Crosthwaite On 06.09.2016 01:12, Peter Maydell wrote: > On 5 September 2016 at 22:52, Dmitry Osipenko wrote: >> On 05.09.2016 21:14, Peter Maydell wrote: >>> I tried "check this compiles and passes the tests for each commit", >>> and what I found was that you get a lot of new warnings running >>> 'make check' about "Timer with period zero, disabling", which were >>> not there before. Could you look into this and fix it, please? > >> Sure, I'll silence those ptimer error messages by making them conditional of >> qtest_enabled(). The messages are actually harmless, however not very useful in >> the context of testing. Thanks a lot for returning to that series, I appreciate it. > > I had a look at the rest of the patches, but to be honest I found > it very difficult to figure out whether any of the changes were > making the right changes or the wrong changes. So it's not clear > to me that "silence the warning if running under qtest" is right: > why is the warning being produced at all? > The ptimer tests cover all ptimer behaviour cases, including the cases where ptimer stops because of the error condition. This helps to ensure that further applied ptimer patches (like new policies) are not affecting old ptimer (policy) behaviour, including those error cases. So the warning message is being emitted each time some of the ptimer tests checks the error condition behaviour. If you have any thoughts on how to make review of this series easier for you, please tell. I'm open to suggestions. BTW, I'm going to turn "Fix counter - 1 returned by ptimer_get_count for the active timer" patch into a ptimer policy. The patch is correct for all of the current ptimer users, however that "counter - 1" feature could be useful for some of the future added timers, like nios2 timer [0], and could be already used by some of the QEMU forks. So it might be better to retain old behaviour for the default policy. [0] https://lists.nongnu.org/archive/html/qemu-devel/2016-07/msg06423.html -- Dmitry