Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S932434AbcKHST3 (ORCPT <rfc822;w@1wt.eu>);
        Tue, 8 Nov 2016 13:19:29 -0500
Received: from mail-qk0-f174.google.com ([209.85.220.174]:35639 "EHLO
        mail-qk0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752899AbcKHSTY (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 8 Nov 2016 13:19:24 -0500
Date: Tue, 8 Nov 2016 13:19:22 -0500 (EST)
From: Nicolas Pitre <nicolas.pitre@linaro.org>
To: John Stultz <john.stultz@linaro.org>
cc: Michal Marek <mmarek@suse.com>,
        Richard Cochran <richardcochran@gmail.com>,
        Paul Bolle <pebolle@tiscali.nl>, Thomas Gleixner <tglx@linutronix.de>,
        Josh Triplett <josh@joshtriplett.org>,
        Edward Cree <ecree@solarflare.com>, netdev@vger.kernel.org,
        linux-kbuild@vger.kernel.org, lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 4/4] posix-timers: make it configurable
In-Reply-To: <CALAqxLVrGRW3ODzZk=4-8FSLe5wBwHJSOEAW=+kMFjTRoe1HzQ@mail.gmail.com>
Message-ID: <alpine.LFD.2.20.1611081311420.11818@knanqh.ubzr>
References: <1478556899-2951-1-git-send-email-nicolas.pitre@linaro.org> <1478556899-2951-5-git-send-email-nicolas.pitre@linaro.org> <CALAqxLVrGRW3ODzZk=4-8FSLe5wBwHJSOEAW=+kMFjTRoe1HzQ@mail.gmail.com>
User-Agent: Alpine 2.20 (LFD 67 2015-01-07)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1923
Lines: 51

On Tue, 8 Nov 2016, John Stultz wrote:

> One spot of concern is that the
> tools/testing/selftests/timers/posix_timers.c test hangs testing
> virtual itimers. Looking through the code I'm not seeing where an
> error case is missed.
> 
> The strace looks like:
> ...
> write(1, "Testing posix timers. False nega"..., 66Testing posix
> timers. False negative may happen on CPU execution
> ) = 66
> write(1, "based timers if other threads ru"..., 48based timers if
> other threads run on the CPU...
> ) = 48
> write(1, "Check itimer virtual... ", 24Check itimer virtual... ) = 24
> rt_sigaction(SIGVTALRM, {0x400a80, [VTALRM], SA_RESTORER|SA_RESTART,
> 0x7fb73306ccb0}, {SIG_DFL, [], 0}, 8) = 0
> gettimeofday({1478710402, 937476}, NULL) = 0
> setitimer(ITIMER_VIRTUAL, {it_interval={0, 0}, it_value={2, 0}}, NULL) = 0
> <Hang>
> 
> 
> Where as with posix timers enabled:
> ...
> write(1, "Testing posix timers. False nega"..., 138Testing posix
> timers. False negative may happen on CPU execution
> based timers if other threads run on the CPU...
> Check itimer virtual... ) = 138
> rt_sigaction(SIGVTALRM, {0x400a80, [VTALRM], SA_RESTORER|SA_RESTART,
> 0x7f231ba8ccb0}, {SIG_DFL, [], 0}, 8) = 0
> gettimeofday({1478626751, 904856}, NULL) = 0
> setitimer(ITIMER_VIRTUAL, {it_interval={0, 0}, it_value={2, 0}}, NULL) = 0
> --- SIGVTALRM {si_signo=SIGVTALRM, si_code=SI_KERNEL} ---
> rt_sigreturn()                          = 0

I'll have a look.

> So I suspect you were a little too aggressive with the #ifdefs around
> the itimers/signal code, or we need to make sure we return an error on
> the setitimer ITIMER_VIRTUAL case as well.

Well, it seemed to me that with POSIX_TIMERS=n, all the code that would 
set up that signal is gone, so there was no point keeping the code to 
deliver it.

Now... would it make more sense to remove itimer support as well when 
POSIX_TIMERS=n?  The same reasoning would apply.


Nicolas