From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.9 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,HK_RANDOM_FROM,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6925BC433FE for ; Thu, 10 Dec 2020 17:12:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2E05E23D23 for ; Thu, 10 Dec 2020 17:12:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404049AbgLJRMQ (ORCPT ); Thu, 10 Dec 2020 12:12:16 -0500 Received: from mga18.intel.com ([134.134.136.126]:17797 "EHLO mga18.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2392581AbgLJRLt (ORCPT ); Thu, 10 Dec 2020 12:11:49 -0500 IronPort-SDR: d41Y+XWzAeTMKrgzKEl09M9eSv7KI2jSvgh145xZvFm066gfrrjUZuEYHduJ/hP2oCbV9PXPSn wY37elf7r3vA== X-IronPort-AV: E=McAfee;i="6000,8403,9831"; a="162051068" X-IronPort-AV: E=Sophos;i="5.78,408,1599548400"; d="scan'208";a="162051068" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Dec 2020 09:09:43 -0800 IronPort-SDR: WE0hWg/82iCdDqPN3YvhJW4jlnyplmaYkBa3XkZQjjL3reiaF42Zga7gdGwHogLRW60Z0eWbWH 4ViCmjePZsXg== X-IronPort-AV: E=Sophos;i="5.78,408,1599548400"; d="scan'208";a="320011341" Received: from nabuhijl-mobl.ger.corp.intel.com (HELO [10.251.185.230]) ([10.251.185.230]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Dec 2020 09:09:36 -0800 Subject: Re: [Intel-gfx] [PATCH v3 2/4] drm/i915/pmu: Use kstat_irqs to get interrupt count To: Thomas Gleixner , Joonas Lahtinen , Jerry Snitselaar , linux-kernel@vger.kernel.org, Tvrtko Ursulin , Chris Wilson Cc: Jason Gunthorpe , Peter Zijlstra , intel-gfx@lists.freedesktop.org, Matthew Garrett , James Bottomley , David Airlie , Jarkko Sakkinen , dri-devel@lists.freedesktop.org, linux-integrity@vger.kernel.org, Peter Huewe References: <20201205014340.148235-1-jsnitsel@redhat.com> <20201205014340.148235-3-jsnitsel@redhat.com> <875z5e99ez.fsf@nanos.tec.linutronix.de> <160758677957.5062.15497765500689083558@jlahtine-mobl.ger.corp.intel.com> <87v9d9k49q.fsf@nanos.tec.linutronix.de> From: Tvrtko Ursulin Organization: Intel Corporation UK Plc Message-ID: Date: Thu, 10 Dec 2020 17:09:33 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <87v9d9k49q.fsf@nanos.tec.linutronix.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/12/2020 16:35, Thomas Gleixner wrote: > On Thu, Dec 10 2020 at 10:45, Tvrtko Ursulin wrote: >> On 10/12/2020 07:53, Joonas Lahtinen wrote: >>> I think later in the thread there was a suggestion to replace this with >>> simple counter increment in IRQ handler. >> >> It was indeed unsafe until recent b00bccb3f0bb ("drm/i915/pmu: Handle >> PCI unbind") but now should be fine. >> >> If kstat_irqs does not get exported it is easy enough for i915 to keep a >> local counter. Reasoning was very infrequent per cpu summation is much >> cheaper than very frequent atomic add. Up to thousands of interrupts per >> second vs "once per second" PMU read kind of thing. > > Why do you need a atomic_add? It's ONE interrupt which can only be > executed on ONE CPU at a time. Interrupt handlers are non-reentrant. > > The core code function will just return an accumulated counter nowadays > which is only 32bit wide, which is what the interface provided forever. > That needs to be fixed first. > > Aside of that the accounting is wrong when the interrupt line is shared > because the core accounts interrupt per line not per device sharing the > line. Don't know whether you care or not. > > I'll send out a series addressing irq_to_desc() (ab)use all over the > place shortly. i915 is in there... Yep we don't need atomic, my bad. And we would care about the shared interrupt line. And without atomic the extra accounting falls way below noise. So in the light of it all, it sounds best I just quickly replace our abuse with private counting and then you don't have to deal with it in your series. Regards, Tvrtko From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.9 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,HK_RANDOM_FROM,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BCD69C4167B for ; Thu, 10 Dec 2020 17:09:48 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6795523D3C for ; Thu, 10 Dec 2020 17:09:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6795523D3C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C33DA89D77; Thu, 10 Dec 2020 17:09:46 +0000 (UTC) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3085289129; Thu, 10 Dec 2020 17:09:45 +0000 (UTC) IronPort-SDR: Rdo++ORfBoNE+QyZxceI/aOVE7JSAiseGtalv4nl0svq07OdrqG/PLQRoo/oR6lahNB/Xf3uBd LRw4OKqKC9lQ== X-IronPort-AV: E=McAfee;i="6000,8403,9831"; a="174437052" X-IronPort-AV: E=Sophos;i="5.78,408,1599548400"; d="scan'208";a="174437052" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Dec 2020 09:09:43 -0800 IronPort-SDR: WE0hWg/82iCdDqPN3YvhJW4jlnyplmaYkBa3XkZQjjL3reiaF42Zga7gdGwHogLRW60Z0eWbWH 4ViCmjePZsXg== X-IronPort-AV: E=Sophos;i="5.78,408,1599548400"; d="scan'208";a="320011341" Received: from nabuhijl-mobl.ger.corp.intel.com (HELO [10.251.185.230]) ([10.251.185.230]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Dec 2020 09:09:36 -0800 Subject: Re: [Intel-gfx] [PATCH v3 2/4] drm/i915/pmu: Use kstat_irqs to get interrupt count To: Thomas Gleixner , Joonas Lahtinen , Jerry Snitselaar , linux-kernel@vger.kernel.org, Tvrtko Ursulin , Chris Wilson References: <20201205014340.148235-1-jsnitsel@redhat.com> <20201205014340.148235-3-jsnitsel@redhat.com> <875z5e99ez.fsf@nanos.tec.linutronix.de> <160758677957.5062.15497765500689083558@jlahtine-mobl.ger.corp.intel.com> <87v9d9k49q.fsf@nanos.tec.linutronix.de> From: Tvrtko Ursulin Organization: Intel Corporation UK Plc Message-ID: Date: Thu, 10 Dec 2020 17:09:33 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <87v9d9k49q.fsf@nanos.tec.linutronix.de> Content-Language: en-US X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , Peter Zijlstra , intel-gfx@lists.freedesktop.org, Matthew Garrett , James Bottomley , Jason Gunthorpe , Jarkko Sakkinen , dri-devel@lists.freedesktop.org, linux-integrity@vger.kernel.org, Peter Huewe Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 10/12/2020 16:35, Thomas Gleixner wrote: > On Thu, Dec 10 2020 at 10:45, Tvrtko Ursulin wrote: >> On 10/12/2020 07:53, Joonas Lahtinen wrote: >>> I think later in the thread there was a suggestion to replace this with >>> simple counter increment in IRQ handler. >> >> It was indeed unsafe until recent b00bccb3f0bb ("drm/i915/pmu: Handle >> PCI unbind") but now should be fine. >> >> If kstat_irqs does not get exported it is easy enough for i915 to keep a >> local counter. Reasoning was very infrequent per cpu summation is much >> cheaper than very frequent atomic add. Up to thousands of interrupts per >> second vs "once per second" PMU read kind of thing. > > Why do you need a atomic_add? It's ONE interrupt which can only be > executed on ONE CPU at a time. Interrupt handlers are non-reentrant. > > The core code function will just return an accumulated counter nowadays > which is only 32bit wide, which is what the interface provided forever. > That needs to be fixed first. > > Aside of that the accounting is wrong when the interrupt line is shared > because the core accounts interrupt per line not per device sharing the > line. Don't know whether you care or not. > > I'll send out a series addressing irq_to_desc() (ab)use all over the > place shortly. i915 is in there... Yep we don't need atomic, my bad. And we would care about the shared interrupt line. And without atomic the extra accounting falls way below noise. So in the light of it all, it sounds best I just quickly replace our abuse with private counting and then you don't have to deal with it in your series. Regards, Tvrtko _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.9 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,HK_RANDOM_FROM,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BF393C4361B for ; Thu, 10 Dec 2020 17:09:47 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 41917230FC for ; Thu, 10 Dec 2020 17:09:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 41917230FC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 905C089D40; Thu, 10 Dec 2020 17:09:46 +0000 (UTC) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3085289129; Thu, 10 Dec 2020 17:09:45 +0000 (UTC) IronPort-SDR: Rdo++ORfBoNE+QyZxceI/aOVE7JSAiseGtalv4nl0svq07OdrqG/PLQRoo/oR6lahNB/Xf3uBd LRw4OKqKC9lQ== X-IronPort-AV: E=McAfee;i="6000,8403,9831"; a="174437052" X-IronPort-AV: E=Sophos;i="5.78,408,1599548400"; d="scan'208";a="174437052" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Dec 2020 09:09:43 -0800 IronPort-SDR: WE0hWg/82iCdDqPN3YvhJW4jlnyplmaYkBa3XkZQjjL3reiaF42Zga7gdGwHogLRW60Z0eWbWH 4ViCmjePZsXg== X-IronPort-AV: E=Sophos;i="5.78,408,1599548400"; d="scan'208";a="320011341" Received: from nabuhijl-mobl.ger.corp.intel.com (HELO [10.251.185.230]) ([10.251.185.230]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Dec 2020 09:09:36 -0800 To: Thomas Gleixner , Joonas Lahtinen , Jerry Snitselaar , linux-kernel@vger.kernel.org, Tvrtko Ursulin , Chris Wilson References: <20201205014340.148235-1-jsnitsel@redhat.com> <20201205014340.148235-3-jsnitsel@redhat.com> <875z5e99ez.fsf@nanos.tec.linutronix.de> <160758677957.5062.15497765500689083558@jlahtine-mobl.ger.corp.intel.com> <87v9d9k49q.fsf@nanos.tec.linutronix.de> From: Tvrtko Ursulin Organization: Intel Corporation UK Plc Message-ID: Date: Thu, 10 Dec 2020 17:09:33 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <87v9d9k49q.fsf@nanos.tec.linutronix.de> Content-Language: en-US Subject: Re: [Intel-gfx] [PATCH v3 2/4] drm/i915/pmu: Use kstat_irqs to get interrupt count X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , Peter Zijlstra , intel-gfx@lists.freedesktop.org, Matthew Garrett , James Bottomley , Jason Gunthorpe , Jarkko Sakkinen , dri-devel@lists.freedesktop.org, linux-integrity@vger.kernel.org, Peter Huewe Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On 10/12/2020 16:35, Thomas Gleixner wrote: > On Thu, Dec 10 2020 at 10:45, Tvrtko Ursulin wrote: >> On 10/12/2020 07:53, Joonas Lahtinen wrote: >>> I think later in the thread there was a suggestion to replace this with >>> simple counter increment in IRQ handler. >> >> It was indeed unsafe until recent b00bccb3f0bb ("drm/i915/pmu: Handle >> PCI unbind") but now should be fine. >> >> If kstat_irqs does not get exported it is easy enough for i915 to keep a >> local counter. Reasoning was very infrequent per cpu summation is much >> cheaper than very frequent atomic add. Up to thousands of interrupts per >> second vs "once per second" PMU read kind of thing. > > Why do you need a atomic_add? It's ONE interrupt which can only be > executed on ONE CPU at a time. Interrupt handlers are non-reentrant. > > The core code function will just return an accumulated counter nowadays > which is only 32bit wide, which is what the interface provided forever. > That needs to be fixed first. > > Aside of that the accounting is wrong when the interrupt line is shared > because the core accounts interrupt per line not per device sharing the > line. Don't know whether you care or not. > > I'll send out a series addressing irq_to_desc() (ab)use all over the > place shortly. i915 is in there... Yep we don't need atomic, my bad. And we would care about the shared interrupt line. And without atomic the extra accounting falls way below noise. So in the light of it all, it sounds best I just quickly replace our abuse with private counting and then you don't have to deal with it in your series. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx