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 9D2F3C48297 for ; Fri, 9 Feb 2024 21:45:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1F7046B0083; Fri, 9 Feb 2024 16:45:04 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1A6F26B0085; Fri, 9 Feb 2024 16:45:04 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0202D6B0087; Fri, 9 Feb 2024 16:45:03 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id E89A86B0083 for ; Fri, 9 Feb 2024 16:45:03 -0500 (EST) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 61C4F1210D9 for ; Fri, 9 Feb 2024 21:45:02 +0000 (UTC) X-FDA: 81773596044.21.099C4A5 Received: from mail-vk1-f181.google.com (mail-vk1-f181.google.com [209.85.221.181]) by imf18.hostedemail.com (Postfix) with ESMTP id 76BFA1C0022 for ; Fri, 9 Feb 2024 21:45:00 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=YiFok1qj; spf=pass (imf18.hostedemail.com: domain of elver@google.com designates 209.85.221.181 as permitted sender) smtp.mailfrom=elver@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=1707515100; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=et8tU5mdHwkCQtGabHsU2uKDCRclXXg4309BFtdcuZo=; b=ZVAVjD5IgQLr4uJV36GcQRMFWbec0hd3lFcJCtC24sCvXg6O/+KnN+ypdLcx5M1zTZ3Fmz YN0LASFI4QO3uJgyYNR5XNzOOBj8F/OLJbu5BkpaJQ+zhMzkuHyetVE/g8HeKP817ki6DV PJBt1lXgD0fw41tKaC/DOBBWUNUR42k= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707515100; a=rsa-sha256; cv=none; b=rqWZ3MtIfL5piWMbfbX0/zemDXRXOp7MKqnkreIVv85z1INPRTc8hgcu9klBC8voDpljdm l0UOx1DM7LqGehJtcdfqSMxgPP8D3akvhUvPYtHiSi1K5An+6X3oCDAkR5IBsPBnJlpPwU 2G1AkLHdQeo0zJt4DEkwaE6cZw5/edo= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=YiFok1qj; spf=pass (imf18.hostedemail.com: domain of elver@google.com designates 209.85.221.181 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-vk1-f181.google.com with SMTP id 71dfb90a1353d-4c03b2ac77aso368653e0c.3 for ; Fri, 09 Feb 2024 13:45:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1707515099; x=1708119899; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=et8tU5mdHwkCQtGabHsU2uKDCRclXXg4309BFtdcuZo=; b=YiFok1qjcFgngewBGKMfHx7lItYU6lyXlsUrcCh0RvVfBH08xC3WnsPGxqyQVi8hG6 URZPTp3oz8T46dUInQhZ6m0933NLG5ETEzDB0D+ftZVn15Yz6IQJKJq4lcNKb1cnS4lh rQHElWJNQSJRjaE3N/311sGUTpWQj3JYDlXA87ySt2Xhm8tJMkw+I4CoMuJVxYclX9NN BmfNxf23XPd7Gx8S9UbzvaCAD3xYIKgXV45HOAvkTA1xcsJ7wichizWE8ZoSVTBS12Z9 PW58u0TGwPP6Uw3ib8z1N0EdNdOYdNc5A1iiW/JavrQmefdPRgkRBalKrbRvOTIxqdcG En1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707515099; x=1708119899; h=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=et8tU5mdHwkCQtGabHsU2uKDCRclXXg4309BFtdcuZo=; b=kj7cexd568xUC5I0WbmcnFLBwneBFvfICGozCTHN7m7EmCKsJUEh3922sfyp+nJDpk DTOZvM+5VGVUhMNYWqm0gzbnY4164zF8bf18uFqocDRloBAxZ6BjFNKleLTkqLmH8SFd Iqr2hyfp2VOzD2ZPm0zvDXKsetHisWgVGRmD/3XNaze3nG3zENfPQmUIUCrv+zO+Kgcs /L/nR7y/LtJHnDlGhR8Dup/cqP9WijVvN1lFtDQmmPC/t267xzaEda19V0kmB28PQdpY 9uCJS0PzvLAdUOfLDRlwkS7PNAccWZpKfnB+cMpyXYQJwWXuWZL7x91Cutuq6drYhL93 837A== X-Forwarded-Encrypted: i=1; AJvYcCXIL91klIx/bqaBGthGvgvQll/1LMrD4M7Es/F7QfsxqYf/SamjajkezS0sQK09BL5H3/ScnBJoAY9N5bjxzP7+S3c= X-Gm-Message-State: AOJu0YyYDiTmudV+mVIami3rR++C6l8Es0Z/EpkuZKSEHjXHv8ySz86/ aEvBTwhzq9z2sAlf1Y+NZqcL2e/TnhUflVwITONwtB7ZKeodN/ekzI9GPzgfEJnToY2QQQVsGrZ BbIk6OTt83Dm5bzty0BnwHAQLy2wwsN0Oh4tH X-Google-Smtp-Source: AGHT+IED4iO9Yz4f9meMJbcQBWw3TJedIDM1/gAdOAcEcQ3P6BcOuVEJ/72AnyHj9NUbD2Vy7AvV7t1wpubEjOwKpk8= X-Received: by 2002:a1f:6281:0:b0:4bd:8926:8e15 with SMTP id w123-20020a1f6281000000b004bd89268e15mr747476vkb.0.1707515099445; Fri, 09 Feb 2024 13:44:59 -0800 (PST) MIME-Version: 1.0 References: <20240208234539.19113-1-osalvador@suse.de> <20240208234539.19113-3-osalvador@suse.de> In-Reply-To: From: Marco Elver Date: Fri, 9 Feb 2024 22:44:23 +0100 Message-ID: Subject: Re: [PATCH v7 2/4] mm,page_owner: Implement the tracking of the stacks count To: Oscar Salvador Cc: Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Michal Hocko , Vlastimil Babka , Andrey Konovalov , Alexander Potapenko Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: qhrcuf9stoxp7q9f7s8w58ao9jnju6kf X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 76BFA1C0022 X-Rspam-User: X-HE-Tag: 1707515100-568366 X-HE-Meta: U2FsdGVkX1/r5lP6M+hRCkrloE1zKUyC0/XhDZXMUle+J717waGU+heUE7XqHKjtxFvmpVuh+darKkL034FIZNtgwlCEAnVyvpA3aQKHNO20woYlshyWLMfA5pZdT2hyAkhABYIQlxJHHuVHiV/so6GlEfWW56DcyT3SfSOA7ttR1tcr/HGpohs95x+DYcNhwpJkJNlrBv+QEMtEoWQWnP319DhGwz05xGUT6y+IdYoOH8ifsSKELX2zNGcPAE4m8aD2bkuDmpGatAU/iFmTAHAyD51hpwUGzVrq2BUwnQOB688V+w2JcQv0qvQg/MjeA2Hg2SzVLJVhA0gddhGmwMDCWY4HaNX00RKfp7+XzNp7JFgBET60XtDy2ZJXJAcRdWLh0gWnt5S0jSzGLzRDhBD6zUyJBa+urlRt685oSqGcvFZy0Pj360gKr0jh4WwIAhVnvUXTMgJjzWFXy2KtFEjqjcQ3nVu84vv2bxHByhO6JkNx9GDLH//cjf+lq6pz+cgmfwXwSewQLwcNVIqUdQOZXuiYRO3FJlXun0MkwwhNqLs79XmTp8q4tqIOXk82BeYRq7B2STihZw/As0Xbr/iIvDwXYsP4/n9cj5Q/VFDVLsxIWv9xN9i9Y/bK7aKVnsWzW+ZeddetdejfBYQZIY3vBsV/icI6SgQ+cV6LeRQhcp2oX3x1Of189NHdsjRrt6jbBPy8UGfALestYnVQSHMr3zZ1V5zJUAO1UwOjhEYUwfP1AVJ0568nw+y3mpAuLgdifHvfKkzoK2T+/POIzYe4ZXz93YpJ81O8GSC7dnXnm+P9wPPsRoEJ3wmQ1THbq03M19Kc9LGzhw3gi3ySXmCvfk2/0AHm4BzSvPzRpjMMvI+YKywDTwXMw3v0BHmUJc3PvK0+VSsRqBU7X/hiXWqJyMXQ+Zs8Xv0g0+6I56EhAIuSwmm771wocDADZlIfRFue11halAUkLJv0l07 BUglD/25 BLStAq10OZQE/Gi8aHJo1aJpHjBci5yBaDjGSvBf0reUXVoz/JZ+6TqFGlNahlM6OHbJg8AZsaPPxoIzwfmF936mAi9CvfPo36tLIBHzb58IikMoIlI+TYtT0pqeW23r9E4pkb9vUdS1liJPU6X9RlU6P6kW5NHZDjuf8F/D9f4SfAOFI3MLnHeCFHs5wXSk0YpRp6lk1CrXUQZK7gPpGdS6l31gNgGDyjYWGdfVYfZYJshLYr5CiTYVqB57xnbPsVDHnMMtrCsSrMs7536Jv/2s3S2sRzyW2MC/LZ8l6rU6RIR4= X-Bogosity: Ham, tests=bogofilter, spamicity=0.018967, 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 Fri, 9 Feb 2024 at 22:42, Marco Elver wrote: > > On Fri, 9 Feb 2024 at 22:37, Oscar Salvador wrote: > > > > On Fri, Feb 09, 2024 at 08:45:00AM +0100, Marco Elver wrote: > > > > +/** > > > > + * stack_depo_get_stack - Get a pointer to a stack struct > > > > > > Typo: "depo" -> depot > > > > > > I would also write "stack_record struct", because "stack struct" does not exist. > > > > Fixed. > > > > > > + * @handle: Stack depot handle > > > > + * > > > > + * Return: Returns a pointer to a stack struct > > > > + */ > > > > +struct stack_record *stack_depot_get_stack(depot_stack_handle_t handle); > > > > > > I don't know what other usecases there are for this, but I'd want to > > > make make sure we give users a big hint to avoid unnecessary uses of > > > this function. > > > > > > Perhaps we also want to mark it as somewhat internal, e.g. by > > > prefixing it with __. So I'd call it __stack_depot_get_stack_record(). > > > > Yes, I went with __stack_depot_get_stack_record(), and I updated its doc > > in stackdepot.h, mentioning that is only for internal purposes. > > > > > > +static void inc_stack_record_count(depot_stack_handle_t handle) > > > > +{ > > > > + struct stack_record *stack = stack_depot_get_stack(handle); > > > > + > > > > + if (stack) > > > > + refcount_inc(&stack->count); > > > > +} > > > > > > In the latest stackdepot version in -next, the count is initialized to > > > REFCOUNT_SATURATED to warn if a non-refcounted entry is suddenly used > > > as a refcounted one. In your case this is intentional and there is no > > > risk that the entry will be evicted, so that's ok. But you need to set > > > the refcount to 1 somewhere here on the initial stack_depot_save(). > > > > Well, I went with something like: > > > > static void inc_stack_record_count(depot_stack_handle_t handle) > > { > > struct stack_record *stack = __stack_depot_get_stack_record(handle); > > > > if (stack) { > > /* > > * New stack_records that do not use STACK_DEPOT_FLAG_GET start > > * with REFCOUNT_SATURATED to catch spurious increments of their > > * refcount. > > * Since we do not use STACK_DEPOT_FLAG_{GET,PUT} API, let us > > There is no FLAG_PUT, only stack_depot_put(). Saying you do not use > the refcount to free any entries should hopefully make it clear that > even if the refcount saturates and you wrap around to 1, nothing > catastrophic will happen. > > > * set a refcount of 1 ourselves. > > */ > > if (refcount_read(&stack->count) == REFCOUNT_SATURATED) > > refcount_set(&stack->count, 1); Do you need to inc the first allocation? Should there be an "else" here instead of always doing refcount_inc()? > > refcount_inc(&stack->count); > > } > > } > > That looks reasonable.