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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6558BCD1288 for ; Tue, 2 Apr 2024 00:16:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E42296B0082; Mon, 1 Apr 2024 20:16:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DCA266B0083; Mon, 1 Apr 2024 20:16:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C44C96B009B; Mon, 1 Apr 2024 20:16:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id A36E46B0082 for ; Mon, 1 Apr 2024 20:16:47 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 69A8A1C0795 for ; Tue, 2 Apr 2024 00:16:47 +0000 (UTC) X-FDA: 81962676054.14.6D8984A Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) by imf01.hostedemail.com (Postfix) with ESMTP id CA58240010 for ; Tue, 2 Apr 2024 00:16:45 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=a1qKpXl+; spf=pass (imf01.hostedemail.com: domain of pcc@google.com designates 209.85.214.173 as permitted sender) smtp.mailfrom=pcc@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1712017005; h=from:from:sender: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:dkim-signature; bh=fyW4BCGaQS495oS/zQ0Useus0nHWJbq7KkRShOZB/QA=; b=Svzi/ksdSIQ/skl9+PmQesH9jlXUWe5+V5yOvnpsOrVjX4dlBDxLgeGWktUmAo2/e+lRoc /b5MRZOqChVYTSsrpIjRxXdhL8WzCRroQ1zmd5kZ1oeOyDeHuzj6UfWRUqNakG2sApLYAL ZwkO/0gi0XBzmVTfbigXJUR+JIPaLoY= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712017005; a=rsa-sha256; cv=none; b=rpXyfMB+ydsTOvcrNgnQcFvwwbFyZ+HYQnLw2vpfF4uOWngmNnTFQcsBzqjQ58/hHYdbUk PKrkI4DisJ9oTy2QxVfRdqqKZjozSKh4wtr1jKRqBgQpaXeEC3TfoyprS4pSUJZB7BL1fk QsmcEy6bUga5yW0hQEryAeW7PGLrciY= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=a1qKpXl+; spf=pass (imf01.hostedemail.com: domain of pcc@google.com designates 209.85.214.173 as permitted sender) smtp.mailfrom=pcc@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-1deddb82b43so482195ad.0 for ; Mon, 01 Apr 2024 17:16:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1712017004; x=1712621804; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=fyW4BCGaQS495oS/zQ0Useus0nHWJbq7KkRShOZB/QA=; b=a1qKpXl+TrSv+YRO0S37hMIEKGoINFnyTb4hBQGiADYVJ7JXfjHoIDEsE8M3amSCfj faUx3+11liaTj5DQQVnTzmOl48Y1E9BKFqyg70Ww27RvsJfMCOtzcUNzVjmv5YF999tj pvIobOD9rjSswLoUlPw2sspaAZL+h9Cc6kZzdLH1JiMBUd/3dgVR1teSWsSiLJL6p+Ux ocPh7UIFQF3XdZt7U+MY3AAnoCudR7PW+Mxryf2ovyh1ATFlw47eiDIaY0kxuBuvA4Ir JpRX2kjnqDl41kTe3BU9vJb63HFDb1ZrFx5wfsgP2yBpWRIRG/qGtYhhMYceVMFdLmkn KTWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712017004; x=1712621804; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=fyW4BCGaQS495oS/zQ0Useus0nHWJbq7KkRShOZB/QA=; b=rgm4XH4814sqj9lVSp0UTXO3DhvMdYtg+9JfXsdvzOiEJ2+/Kbd0/0gi88jKyqteaa T+q5/JHvDimkaMPtMNtSEg+Qahf2aPW+nlJJuFDBCK0YP0A4OkFnqD+Hymn7RKp3iCOh gTHYD8OaOKr8+K4UYsCsGrQbwHSp7i4vjs63TvFkCvvPmWAqqYoz7HZjVRJCJL9Sf7II GRK9fdW4fqrGVhrMwIOZS3/1kxuPVpDQzlLxWE5U7YHLHdDg42i3N21qVs65o9MG8ExO jOKZMcLEF3A/9oEB8WV4WQ6oeJlC/qPLaRCExNd4hnSdQ7NTFInIvB9JRbNexMvkeXKg i18g== X-Forwarded-Encrypted: i=1; AJvYcCWL/XJVZbUys65vBjecV3Y80DuhF51eSmrvMO29bQyvEHsQqgLL+YfFt1T8aoDKO2NXMk2muVIyo3DIvYNxiw3/L+A= X-Gm-Message-State: AOJu0YxKIk8Cj2/SwUsYhldhCpqD7TIwF5a1vTECSgCzWCB9i0BqPc9h zqzLfmObvbn6M/hniflrzHHwh4ffIt5Xurad3AKQY5vnWgtIQCTUe8w0My/FZgPM+2jLhbmspRR D7a6KSpukj4PLgQodubrbHk3xWykGuJNGUACG X-Google-Smtp-Source: AGHT+IEmefmlOYptKLrsjrYXMZdGKtoAo0HMnv8wYVRtxPcevsDBQnJ/0sK3/0DKNIlibuR7D02mzHG0kdqh6knJRXg= X-Received: by 2002:a17:902:cec6:b0:1e0:f525:a831 with SMTP id d6-20020a170902cec600b001e0f525a831mr710596plg.28.1712017004291; Mon, 01 Apr 2024 17:16:44 -0700 (PDT) MIME-Version: 1.0 References: <20240215215907.20121-1-osalvador@suse.de> <20240215215907.20121-2-osalvador@suse.de> In-Reply-To: From: Peter Collingbourne Date: Mon, 1 Apr 2024 17:16:31 -0700 Message-ID: Subject: Re: [PATCH v10 1/7] lib/stackdepot: Fix first entry having a 0-handle To: Andrey Konovalov Cc: Oscar Salvador , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Michal Hocko , Vlastimil Babka , Marco Elver , Alexander Potapenko Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: CA58240010 X-Rspam-User: X-Stat-Signature: 15imjdrc84mm6fzihezhe37mragdgqjf X-Rspamd-Server: rspam03 X-HE-Tag: 1712017005-794018 X-HE-Meta: U2FsdGVkX1+W5MbYQRgVOPsG1tr6csfl1HHOFTcEMEmmbgVyALg3oxCWD1YCjj+bbVeOf/tHOQC08Wdu+Rev95elCwAiExtIRic+TKgCLLzBSdcDlBvivZb4+ABDZBMOssoJLlI+CIOe1BfZsiK8b1ST4vP1tqe8a2faLE/oxwMjmlXsKvXq0ZIO0WdaXMo9RZv+QrN/kJDZyCTEH+SO2JWlavUnBTXXw4AsLPxllJAWnRIJPt/u4gyGtYqE3WUl4CcMST+yuYBp5LSxxQC9eIZ9qrTggWahROt57FC/VgWTT88F9Yo7BTZF6l7EdeeEb4SSqLlU32xJ28XhAYLZkVZD/8wbJlhtkAK72WL7LFVdvgsU8RHKh4z5wpe9lvSMxTSm6dB0jsnFHXFHzNBEHQ0vaF+aNvEUmqlGWPCKJUFAiVndW+khq9vI1W6z9FFXS9566iuTOPS94hNVJC1Tyv2eK5uHgH7LBJdjuipkKuU8SvTl4piPvAPpXhQ7PbsdlG809MlV+/WNTW8LfW2AES3TolRT2K8PaMxE/ekF2xbnKhwK1Fe2WTPrDtsaolUGIfBNRT3PoUD7SNVH1IvMPM8TAZRblvkF55Jt6/Kvoyn2bGit/3Y4R9ThJpJJVPdodktVeKodP8q6hRZxq4yde+aRAHci9GDIiwzxzDwRVbVEu2o5nc11RaQZCykeuYNvDnAL9MAEg/p8HUQ/HQ6KHvSVPaEK0ZfKXX1R7wCGBPq5eGYbkvj0obMC6orUM822rCZBzuktENkyB0veA8ws6cVOJfkn3mt6yLyOVXQhG3sG0CB8aqfRYNQpAX0pRbqcV0DbwdmZizJb1T2EAOrPg0KavQb1HCQmnoCZDDyks7Rw1QjnkoL7pbvijkjxPjZkV8QQpQSfCANcGJymookpgODgIDX6pwPpfeoF6XBcoBuc0nHNH54v5Q4ehDBZdQdpO5YWGdvY/xy2iRH/BWB u7A== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, Feb 15, 2024 at 4:25=E2=80=AFPM Peter Collingbourne wrote: > > On Thu, Feb 15, 2024 at 3:37=E2=80=AFPM Andrey Konovalov wrote: > > > > On Thu, Feb 15, 2024 at 10:58=E2=80=AFPM Oscar Salvador wrote: > > > > > > The very first entry of stack_record gets a handle of 0, but this is = wrong > > > because stackdepot treats a 0-handle as a non-valid one. > > > E.g: See the check in stack_depot_fetch() > > > > > > Fix this by adding and offset of 1. > > > > > > This bug has been lurking since the very beginning of stackdepot, > > > but no one really cared as it seems. > > > Because of that I am not adding a Fixes tag. > > > > > > Co-developed-by: Marco Elver > > > Signed-off-by: Marco Elver > > > Signed-off-by: Oscar Salvador > > > Acked-by: Vlastimil Babka > > > --- > > > lib/stackdepot.c | 16 +++++++++------- > > > 1 file changed, 9 insertions(+), 7 deletions(-) > > > > > > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > > > index 4a7055a63d9f..c043a4186bc5 100644 > > > --- a/lib/stackdepot.c > > > +++ b/lib/stackdepot.c > > > @@ -45,15 +45,16 @@ > > > #define DEPOT_POOL_INDEX_BITS (DEPOT_HANDLE_BITS - DEPOT_OFFSET_BITS= - \ > > > STACK_DEPOT_EXTRA_BITS) > > > #define DEPOT_POOLS_CAP 8192 > > > +/* The pool_index is offset by 1 so the first record does not have a= 0 handle. */ > > > #define DEPOT_MAX_POOLS \ > > > - (((1LL << (DEPOT_POOL_INDEX_BITS)) < DEPOT_POOLS_CAP) ? \ > > > - (1LL << (DEPOT_POOL_INDEX_BITS)) : DEPOT_POOLS_CAP) > > > + (((1LL << (DEPOT_POOL_INDEX_BITS)) - 1 < DEPOT_POOLS_CAP) ? \ > > > + (1LL << (DEPOT_POOL_INDEX_BITS)) - 1 : DEPOT_POOLS_CAP) > > > > > > /* Compact structure that stores a reference to a stack. */ > > > union handle_parts { > > > depot_stack_handle_t handle; > > > struct { > > > - u32 pool_index : DEPOT_POOL_INDEX_BITS; > > > + u32 pool_index : DEPOT_POOL_INDEX_BITS; /* pool_inde= x is offset by 1 */ > > Can we rename this, say to pool_index_plus_1? This will make the code > a bit clearer, as well as make it possible for debugging tools such as > drgn [1] to be able to tell when the off-by-one was introduced and > adapt accordingly. > > Peter > > [1] https://github.com/osandov/drgn/pull/376 Unfortunately this message was not acted upon, and it looks like akpm picked up the patch and it made its way into Linus's tree. So I sent a followup to fix this here: https://lore.kernel.org/all/20240402001500.53533-1-pcc@google.com/ Peter