From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 58CE230F95C for ; Mon, 1 Dec 2025 14:13:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764598387; cv=none; b=WbsgjJIcjXnnmc2VbmhMlsHSpdBE8UHYpGwDk5jreI1hfTwfzu5iYHR8TUMFJmxk42nST58tPhLYcXA3WfwTl6NLuyCRxfQYqrIjQERLArVcjZeFqd/RBZAtDdyrtKKQ1+gC/gJEf+8lcARgaxxM36YfZle3L4D5/Fv83PXXc/w= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764598387; c=relaxed/simple; bh=Rc8tQOb4KW598mjC8YsSgSNwBXwy5J7HyE2s6WHzBK8=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=YwFaroi8o0qMo32AvyK4vzzXKcjKxGMCV04s5WN3er5tPP15VqruErPCqpmENWxLrEFqd+N/AX3e565bXgZQ4l/bONjfZeXYuDKthbLjEPlmeWU97uncYvoYBl+joVl2T967KSvMS4Y6wAy6ofqp+tEJSrgjz48phiFtuxNEe6A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=N7gUrqSI; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=gcCT9rYb; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="N7gUrqSI"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="gcCT9rYb" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1764598385; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=61JYzq1X4An4pJbqjhEXU3J3+I6bDh/T5IuX0+k4kE8=; b=N7gUrqSIZ/J77bsebfKkVt8El0VYE369WmzUe7EBdA17x73qmKkyvxu2QfgQ7ZtSfICgWM c6Igb5JRHAxBd/dNM7KMsWT7R1tvBiFRM4vYTQprA7MdkeVrUL4ypIMsayw2oTEy+QMrwL IwmeuSzwoJEKeSgYZhDA4WJtBCvC17w= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-326-rj7Bd36HP3asWlUh40zDRg-1; Mon, 01 Dec 2025 09:13:04 -0500 X-MC-Unique: rj7Bd36HP3asWlUh40zDRg-1 X-Mimecast-MFC-AGG-ID: rj7Bd36HP3asWlUh40zDRg_1764598384 Received: by mail-qk1-f199.google.com with SMTP id af79cd13be357-8b2e19c8558so754757985a.2 for ; Mon, 01 Dec 2025 06:13:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1764598383; x=1765203183; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=61JYzq1X4An4pJbqjhEXU3J3+I6bDh/T5IuX0+k4kE8=; b=gcCT9rYbVrkf9XrwAUgJ2RemyaQOsOeolJC3kRZnPq0YuIQIJh6iO/7FqPBG0X7cRo HzOIqwTxPlZtF6I5XuEIMDWLRUhwSjv8UToYseBxZiHhrBgK2JGt7WBMqOg2EUEeMKfc zskaMjuDJK2+ugCLTJIDzKSV+JErW/Xe0OMIeucDtQ6knvekQWYDIBhIXYehpInCNacT ZB6FoJOSRomI4RxlGbi176IZnkfrppZE1FNooZ/83gPm9/pZ0V38FPE8bYBrV92PWXQJ Y14pfHNc8VCLnglfxyWL+6ajxcDWY9xuXpnewnyfMCrSbNXt/IC7sjR9UhBd1+OD6+Nr kRUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764598383; x=1765203183; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=61JYzq1X4An4pJbqjhEXU3J3+I6bDh/T5IuX0+k4kE8=; b=Bk4v3eiEiHTL3TLGSPT0IyqlMRfM+KroOLTEB/yHPftuHYqm4riyRuwftTOEK1WitG w0oeU1LVUQnjqXwbiS+M1XuHHMa5VO3lysAlrhFMJ7ifddfrjYvvHudyYWCmsIkLUgWO fSq+PhrcIsXxyxTYxEUW3YmiJSRz7Fesk2ieXfjoRDN0C8hcZIMro1uw2utwfnbZGgI5 joewFyUzmzNjPdEmW1302BRcnbum8eJ2UzAiZfxL9Rv2/V6oEIcnayMU2qTI41paB4Ty l7OCKIGrWzNMyOO/IlMxL1626Eugyict4h0avriKUdV+0kHKH1h+iwzJeVtISCXAGfZu n9nA== X-Forwarded-Encrypted: i=1; AJvYcCV/eUoEUwSzAdxs2etYwJC4rc5M5HKz2wFU6fBLwy+wrDKKobU4sWTBza8CB65HTmw8Z9YHYLbfjItfvww=@vger.kernel.org X-Gm-Message-State: AOJu0YzVnidEZFRS0HXys6RBhwje7Q3bdtpBiTOoenRp4iklKHG6+36g ClqfqH6NfT6XEDZLLfALtXTcQj1mqoIKg3E3vTqUQZkud1q+ftJQVJSTuXuxRMrCxR1XPIhv6Mt Cjys5C52I9XPycKsYwJ5w/cNB4MacacVkcoUSvJB8WsRCQc5pINY8zbE3qpMg3mFYCyElwkF4NA == X-Gm-Gg: ASbGncueQOtCyDotT0THxnEr4TBqghpjPzRJ/l1pseJfZz6rqquIJT49WCdYTwETOYT e/Ku13x7f5ON15tQZUjo7uX4WuPZbomt5NCiFQOEB+5Ew+xnkQtJ48l5Lsj6mqIbhPUl0vJS6o2 aiVZKJTbMWTnL5Mm5r606lVne67tMTmN26VHhVtiB6CJZEoHiwsGwbw3vF2uvGGvKwfVJT7/7HF Ecyq7uxA5ZjpuKJooxoXJ/+xhT2LAnOzBLkdk26pU/01MgAH5vhw8gYl73rkJ+55VzJ8RlHWGIC Ktchw0GtrCy1Fqry5Zr+MAlEfqI+cFwZwwXHT/d0mt+Z200pqM6u2UBhULJajYwfDfweQ1IOOnF tDli15vpoZw== X-Received: by 2002:a05:620a:708a:b0:89f:db05:1643 with SMTP id af79cd13be357-8b33d48e2f2mr5244009485a.89.1764598383226; Mon, 01 Dec 2025 06:13:03 -0800 (PST) X-Google-Smtp-Source: AGHT+IEQ1J6QLX6Tv54HUvnqgzshcUPHK7E1VpPS4qLWxfkDn4ZOdMBygaWC5SDcdU9pi/1XIBeWfQ== X-Received: by 2002:a05:620a:708a:b0:89f:db05:1643 with SMTP id af79cd13be357-8b33d48e2f2mr5244003785a.89.1764598382738; Mon, 01 Dec 2025 06:13:02 -0800 (PST) Received: from [10.26.1.94] ([66.187.232.136]) by smtp.gmail.com with ESMTPSA id af79cd13be357-8b529993c92sm868303485a.1.2025.12.01.06.13.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 01 Dec 2025 06:13:02 -0800 (PST) Message-ID: Date: Mon, 1 Dec 2025 09:13:01 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/3] tools/power turbostat: avoid segfault referencing fd_instr_count_percpu To: Len Brown Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org References: <20251118155813.533424-1-darcari@redhat.com> <20251118155813.533424-2-darcari@redhat.com> Content-Language: en-US From: David Arcari In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit So get_instr_count_fd() calls open_perf_counter() which in turn calls perf_event_open() which returns the value from syscall(). From the documentation this seems to return -1 in the case of a failure. Looking at get_instr_count_fd() I see: int get_instr_count_fd(int cpu) { if (fd_instr_count_percpu[cpu]) return fd_instr_count_percpu[cpu]; fd_instr_count_percpu[cpu] = open_perf_counter(cpu, PERF_TYPE_HARDWARE, PERF_COUNT_HW_INSTRUCTIONS, -1, 0); return fd_instr_count_percpu[cpu]; } So open_perf_counter() is only called when fd_instr_count_percpu[cpu] is 0. In that case the return value is stored in fd_instr_count_percpu[cpu]. So in the case of an error this value would be -1; otherwise, it should be a valid file descriptor. In fact, I don't think the function should ever return 0. As far as I can tell fd_instr_count_percpu[] is initialized to zero so that get_instr_count_fd() can discern whether or not open_perf_counter() needs to be called. Am I missing something? I do see that free_fd_instr_count_percpu() has a bug as I think the code should be: if (fd_instr_count_percpu[i] > 0) instead of: if (fd_instr_count_percpu[i] != 0) Thanks, -DA On 11/25/25 2:11 PM, Len Brown wrote: > not your fault, but looking at this code, it seems that > get_instr_count_fd(base_cpu) > assumes that 0 is an invalid FD. Fine, but based on that you'd think > we'd use zero for invalid > and non-zero for valid as return for the function call... > > On Tue, Nov 18, 2025 at 10:58 AM David Arcari wrote: >> >> The problem is that fd_instr_count_percpu is allocated based on >> the value of has_aperf. If has_aperf=0 then fd_instr_count_percpu >> remains NULL. However, get_instr_count_fd() is called from >> turbostat_init() based on the value of has_aperf_access. >> >> On some VM systems has_aperf can be 0, while has_aperf_access can be >> 1. In order to resolve the issue simply check for to see if >> fd_instr_count_percpu is NULL and return -1 if it is. Accordingly, >> the has_aperf_access check can be removed from turbostat_init. >> >> Signed-off-by: David Arcari >> Cc: Len Brown >> Cc: linux-kernel@vger.kernel.org >> --- >> tools/power/x86/turbostat/turbostat.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c >> index f2512d78bcbd..584b0f7f9067 100644 >> --- a/tools/power/x86/turbostat/turbostat.c >> +++ b/tools/power/x86/turbostat/turbostat.c >> @@ -2463,6 +2463,9 @@ static long open_perf_counter(int cpu, unsigned int type, unsigned int config, i >> >> int get_instr_count_fd(int cpu) >> { >> + if (!fd_instr_count_percpu) >> + return -1; >> + >> if (fd_instr_count_percpu[cpu]) >> return fd_instr_count_percpu[cpu]; >> >> @@ -10027,7 +10030,7 @@ void turbostat_init() >> for_all_cpus(get_cpu_type, ODD_COUNTERS); >> for_all_cpus(get_cpu_type, EVEN_COUNTERS); >> >> - if (BIC_IS_ENABLED(BIC_IPC) && has_aperf_access && get_instr_count_fd(base_cpu) != -1) >> + if (BIC_IS_ENABLED(BIC_IPC) && get_instr_count_fd(base_cpu) != -1) >> BIC_PRESENT(BIC_IPC); >> >> /* >> -- >> 2.51.0 >> >> > >