Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752582AbbG2NXu (ORCPT ); Wed, 29 Jul 2015 09:23:50 -0400 Received: from mail-wi0-f170.google.com ([209.85.212.170]:35862 "EHLO mail-wi0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751351AbbG2NXt (ORCPT ); Wed, 29 Jul 2015 09:23:49 -0400 Date: Wed, 29 Jul 2015 15:23:45 +0200 From: Frederic Weisbecker To: Chris Metcalf Cc: LKML , Peter Zijlstra , Thomas Gleixner , Preeti U Murthy , Christoph Lameter , Ingo Molnar , Viresh Kumar , Rik van Riel Subject: Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick dependency mask model Message-ID: <20150729132343.GC11554@lerouge> References: <1437669735-8786-1-git-send-email-fweisbec@gmail.com> <1437669735-8786-9-git-send-email-fweisbec@gmail.com> <55B26E74.5040803@ezchip.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55B26E74.5040803@ezchip.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2404 Lines: 50 On Fri, Jul 24, 2015 at 12:57:24PM -0400, Chris Metcalf wrote: > On 07/23/2015 12:42 PM, Frederic Weisbecker wrote: > >+static void cpu_timer_list_dequeue(struct cpu_timer_list *t) > >+{ > >+ if (!list_empty(&t->entry)) > >+ cpu_timer_dec_tick_dependency(); > >+ list_del_init(&t->entry); > >+} > > Is the list_empty() test necessary? It wasn't in the original posix-timers > code, and it feels like a pretty serious bug if you're doing a list_del on > an empty list. No multiple calls to list_del_init() is fine on a list_head as long as it has been correctly initialized with INIT_LIST_HEAD() and list_del() hasn't been called at some point before. It's necessary because we do that dequeue also when we change the timer, we disarm it in case it was added somewhere before. > At a higher level, is the posix-cpu-timers code here really providing the > right semantics? It seems like before, the code was checking a struct > task-specific state, and now you are setting a global state such that if ANY > task anywhere in the system (even on housekeeping cores) has a pending posix > cpu timer, then nothing can go into nohz_full mode. > > Perhaps what is needed is a task_struct->tick_dependency to go along with > the system-wide and per-cpu flag words? That's an excellent point! Indeed the tick dependency check on posix-cpu-timers was made on task granularity before and now it's a global dependency. Which means that if any task in the system has a posix-cpu-timer enqueued, it prevents all CPUs from shutting down the tick. I need to mention that in the changelog. Now here is the rationale: I expect that nohz full users are not interested in posix cpu timers at all. The only chance for one to run without breaking the isolation is on housekeeping CPUs. So perhaps there is a corner case somewhere but I assume there isn't until somebody reports an issue. Keeping a task level dependency check means that we need to update it on context switch. Plus it's not only about task but also process. So that means two states to update on context switch and to check from interrupts. I don't think it's worth the effort if there is no user at all. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/