Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934002AbeALOey (ORCPT + 1 other); Fri, 12 Jan 2018 09:34:54 -0500 Received: from mail.kernel.org ([198.145.29.99]:53322 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933757AbeALOew (ORCPT ); Fri, 12 Jan 2018 09:34:52 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D025F2175D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=frederic@kernel.org Date: Fri, 12 Jan 2018 15:34:49 +0100 From: Frederic Weisbecker To: Eric Dumazet Cc: LKML , Levin Alexander , Peter Zijlstra , Linus Torvalds , Hannes Frederic Sowa , "Paul E . McKenney" , Wanpeng Li , Dmitry Safonov , Thomas Gleixner , Radu Rendec , Ingo Molnar , Stanislaw Gruszka , Paolo Abeni , Rik van Riel , Andrew Morton , David Miller Subject: Re: [RFC PATCH 1/2] softirq: Account time and iteration stats per vector Message-ID: <20180112143448.GA1950@lerouge> References: <1515735354-19279-1-git-send-email-frederic@kernel.org> <1515735354-19279-2-git-send-email-frederic@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Thu, Jan 11, 2018 at 10:22:58PM -0800, Eric Dumazet wrote: > > asmlinkage __visible void __softirq_entry __do_softirq(void) > > { > > - unsigned long end = jiffies + MAX_SOFTIRQ_TIME; > > + struct softirq_stat *sstat = this_cpu_ptr(&softirq_stat_cpu); > > unsigned long old_flags = current->flags; > > - int max_restart = MAX_SOFTIRQ_RESTART; > > struct softirq_action *h; > > bool in_hardirq; > > - __u32 pending; > > + __u32 pending, overrun = 0; > > int softirq_bit; > > > > /* > > @@ -262,6 +273,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void) > > __local_bh_disable_ip(_RET_IP_, SOFTIRQ_OFFSET); > > in_hardirq = lockdep_softirq_start(); > > > > + memzero_explicit(sstat, sizeof(*sstat)); > > If you clear sstat here, it means it does not need to be a per cpu > variable, but an automatic one (defined on the stack) That's right. But I thought it was bit large for the stack: struct { u64 time; u64 count; } [NR_SOFTIRQS] although arguably we are either using softirq stack or a fresh task one. > > I presume we need a per cpu var to track cpu usage on last time window. > > ( typical case of 99,000 IRQ per second, one packet delivered per IRQ, > 10 usec spent per packet) So should I account, like, per vector stats in a jiffy window for example? And apply the limits on top of that? > > > > > restart: > > /* Reset the pending bitmask before enabling irqs */ > > set_softirq_pending(0); > > @@ -271,8 +283,10 @@ asmlinkage __visible void __softirq_entry __do_softirq(void) > > h = softirq_vec; > > > > while ((softirq_bit = ffs(pending))) { > > + struct vector_stat *vstat; > > unsigned int vec_nr; > > int prev_count; > > + u64 startime; > > > > h += softirq_bit - 1; > > > > @@ -280,10 +294,18 @@ asmlinkage __visible void __softirq_entry __do_softirq(void) > > prev_count = preempt_count(); > > > > kstat_incr_softirqs_this_cpu(vec_nr); > > + vstat = &sstat->stat[vec_nr]; > > > > trace_softirq_entry(vec_nr); > > + startime = local_clock(); > > h->action(h); > > + vstat->time += local_clock() - startime; > > You might store local_clock() in a variable, so that we do not call > local_clock() two times per ->action() called. So you mean I keep the second local_clock() call for the next first call in the while iteration, right? Yep that sounds possible. > > > > + vstat->count++; > > trace_softirq_exit(vec_nr); > > + > > + if (vstat->time > MAX_SOFTIRQ_TIME || vstat->count > MAX_SOFTIRQ_RESTART) > > If we trust local_clock() to be precise enough, we do not need to > track vstat->count anymore. That's what I was thinking. Should I keep MAX_SOFTIRQ_TIME to 2 ms BTW? It looks a bit long to me. Thanks.