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 5CAA9C4829A for ; Tue, 13 Feb 2024 11:35:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DDC656B009A; Tue, 13 Feb 2024 06:35:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D8DA38D000E; Tue, 13 Feb 2024 06:35:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BDEA66B009D; Tue, 13 Feb 2024 06:35:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id A75036B009A for ; Tue, 13 Feb 2024 06:35:00 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 84F3AA0B56 for ; Tue, 13 Feb 2024 11:35:00 +0000 (UTC) X-FDA: 81786573960.10.E1E91B6 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by imf12.hostedemail.com (Postfix) with ESMTP id 07F854000A for ; Tue, 13 Feb 2024 11:34:57 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=qlN8MDQH; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=iKWAPuNK; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=qlN8MDQH; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=iKWAPuNK; spf=pass (imf12.hostedemail.com: domain of vbabka@suse.cz designates 195.135.223.130 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707824098; 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=ghGDITH5a4ZoXoh0yiMTLBa3Ptq5jxOlMFTWcfo631s=; b=2HygC81jMbmNgATRe7pFzZckb9T5mscn8UEk+OQD7Gfyz+TapPtunEdCE07kZivQV9OTDm jVMPDG/yP9ukyHolITx9RmVEPX9aJEW59OWtwcL70837imwdI63qkFMuCQ4L5fWKq5uKkv F+AvXtpoOf3eSjkw7w7PPDwEotBmkl4= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=qlN8MDQH; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=iKWAPuNK; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=qlN8MDQH; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=iKWAPuNK; spf=pass (imf12.hostedemail.com: domain of vbabka@suse.cz designates 195.135.223.130 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707824098; a=rsa-sha256; cv=none; b=K796p6wYejf7dqCdOpThRDsXLLH5jXvhm5Tia2s8RVoSofHyhWwp2G7F1286TvQLD/fbYH kNdpEpW38P3V+ptAZiHcUTMPPlYP4+6umdesWUQOr8jQf2CzjlfA70Wj1etXgx3mUc/BQ+ 5YR7sPLqcmODJHl5DRxl2TMDouXBub4= Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 5529B21E40; Tue, 13 Feb 2024 11:34:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1707824096; h=from:from:reply-to: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=ghGDITH5a4ZoXoh0yiMTLBa3Ptq5jxOlMFTWcfo631s=; b=qlN8MDQHXyl133ejSoFs03hc66Q5AOAJBTfn5cLTwITPEGtSoKzRfEL1G6lMpI1aIsVAzI kQFBPSVzxaLg7cZRUrDDASIDVY7lqvGrGsjk5UEROwaesP5Xh4vWYeyF+T5iSCQgR0Ueep 2zuTVRCD+FYHi28LR2KfB+wKQXwpc7Q= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1707824096; h=from:from:reply-to: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=ghGDITH5a4ZoXoh0yiMTLBa3Ptq5jxOlMFTWcfo631s=; b=iKWAPuNKsQIqMownxqV9qhJZ0/ynyCdvG2O9M5BDCYqG6/BP2pGi3Oru1KGCPEUIEV1xV5 8e8N+Ip71RBe6bAw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1707824096; h=from:from:reply-to: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=ghGDITH5a4ZoXoh0yiMTLBa3Ptq5jxOlMFTWcfo631s=; b=qlN8MDQHXyl133ejSoFs03hc66Q5AOAJBTfn5cLTwITPEGtSoKzRfEL1G6lMpI1aIsVAzI kQFBPSVzxaLg7cZRUrDDASIDVY7lqvGrGsjk5UEROwaesP5Xh4vWYeyF+T5iSCQgR0Ueep 2zuTVRCD+FYHi28LR2KfB+wKQXwpc7Q= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1707824096; h=from:from:reply-to: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=ghGDITH5a4ZoXoh0yiMTLBa3Ptq5jxOlMFTWcfo631s=; b=iKWAPuNKsQIqMownxqV9qhJZ0/ynyCdvG2O9M5BDCYqG6/BP2pGi3Oru1KGCPEUIEV1xV5 8e8N+Ip71RBe6bAw== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 3AE1C1370C; Tue, 13 Feb 2024 11:34:56 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id OaKNDeBTy2ULegAAD6G6ig (envelope-from ); Tue, 13 Feb 2024 11:34:56 +0000 Message-ID: <11cb2ac2-102f-4acd-aded-bbfd29f7269a@suse.cz> Date: Tue, 13 Feb 2024 12:34:55 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v8 2/5] mm,page_owner: Implement the tracking of the stacks count Content-Language: en-US To: Marco Elver Cc: Oscar Salvador , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Michal Hocko , Andrey Konovalov , Alexander Potapenko References: <20240212223029.30769-1-osalvador@suse.de> <20240212223029.30769-3-osalvador@suse.de> From: Vlastimil Babka In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 07F854000A X-Rspam-User: X-Stat-Signature: 7u74in1jwhggsna844cq4rozjhjix61j X-Rspamd-Server: rspam01 X-HE-Tag: 1707824097-35749 X-HE-Meta: U2FsdGVkX19iHzNAyfMX9CT7pvsUY/I/p2l7A227K/iB4/3dqCvtr+cY/G8zqcQ0NvJeEASLS6J92kPkAd0fstmm+UAGFJ6647QmvxrSWtB7Ye3MZ8a53149j5A3e6foMIMc2/99atfqfbBFOUWEx92tGQeXxm5MyLKy6IgKxIcgAT7T9wX1JifKP2wTZkxdRGm/fsOhmmZVRD7CLrKH1Txi9C/d3KudT+4wA11A3uQifErf7sZV4VS+38caLA6DjKNG9fx9fQAcUyrB83iV3zRiX6XPc0z9Bnj5O3zI2fZnQ5zDUnXDlD86T8OMtPqoAoTVQaqRXz6JoyqKyIPsE8N0Au9amiYpOCxaL/D7ZoBfUOmMt446fS8eBoPwsK/9mfT1z8/rtbQ8QI7X47Cb71vDDCsO3sBy4mu2SF8m2VOVr+fXiAzqjAuD2IeQScdYZ0bUmZIKxI9zL8T70uOO8PCBUhiVmQ0Z8benKks0nCKbnYN/IVPmW9LY02iLafaaUdGdW9cjHxl0oscFemNsyKaYLT6NyI3Lt7M5lDRgSAstAtpTjbMaesrUWIvLBKIn2GdbQ7pmdG6DhchddXQqFR6tQuK1WpUJwbEiJWvYU5zK6d5+GapWgE/j9RLTCtnu+1e5Y3/aCzEjGx2asppP3SwOPJTFcbnYFTCa2YMJg5P+2g4EiXAvJjuR1jk0M2SpvPTNv812kOoaUQNS4mjFSGD89o+rOXHDBNyKDNsHQeL5GZHvYh+LJ4jZP3O70GWWPP70j4KxtGIEB8pJh5qyeFu1aDI6hAtL5wYj2aUFMsemHFpVzl0d70iZN5NPTwR0f4NHYl9bks6TC3Jj3w1JQ8BBJ9fHIRfZGbbC0Umh8ZUyxntwbFfo8uS1qDWLkmNtVQ32CWz7pulOOZ+Y4mLlqeOGF+fRbryESN0VTkWYFfcSg/MQxKJDVbQOO9JKTLYRkRVzQycPOe1kaViMahC elTSSt2j QskWYX6b+p/PqJw+2fvqs9ruUaEKR5WeJ3bMKTZZWIPm/UtqR4gveIZXrZX5rtSQ5eVgObmNTAKhLCFziMjHTLk/DH6kzZV14pwOjkXnDlcnP+OrHIL4MfzJ9XFJ/iryC7SD28IvAwfAz2hTQHtUP9xIAuuQUuNFtkgGK7oVWE7u+qkNNsb/fxWsfAbatIA3dv+RClQBexIxoSN3uLs7IcSmcOjpbbJZDk0iKO7BGjJtYKVLKodqFnLJJclQeb15gmFudUgrcYYw7f9Y91XBN/D2U3Q7crdHREEQPnLavi0cLZEJzwNXPRJgW+g== 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 2/13/24 10:21, Marco Elver wrote: > On Tue, 13 Feb 2024 at 10:16, Vlastimil Babka wrote: >> >> On 2/12/24 23:30, Oscar Salvador wrote: >> > page_owner needs to increment a stack_record refcount when a new allocation >> > occurs, and decrement it on a free operation. >> > In order to do that, we need to have a way to get a stack_record from a >> > handle. >> > Implement __stack_depot_get_stack_record() which just does that, and make >> > it public so page_owner can use it. >> > >> > Also implement {inc,dec}_stack_record_count() which increments >> > or decrements on respective allocation and free operations, via >> > __reset_page_owner() (free operation) and __set_page_owner() (alloc >> > operation). >> > >> > Traversing all stackdepot buckets comes with its own complexity, >> > plus we would have to implement a way to mark only those stack_records >> > that were originated from page_owner, as those are the ones we are >> > interested in. >> > For that reason, page_owner maintains its own list of stack_records, >> > because traversing that list is faster than traversing all buckets >> > while keeping at the same time a low complexity. >> > inc_stack_record_count() is responsible of adding new stack_records >> > into the list stack_list. >> > >> > Modifications on the list are protected via a spinlock with irqs >> > disabled, since this code can also be reached from IRQ context. >> > >> > Signed-off-by: Oscar Salvador >> > --- >> > include/linux/stackdepot.h | 9 +++++ >> > lib/stackdepot.c | 8 +++++ >> > mm/page_owner.c | 73 ++++++++++++++++++++++++++++++++++++++ >> > 3 files changed, 90 insertions(+) >> >> ... >> >> >> > --- a/mm/page_owner.c >> > +++ b/mm/page_owner.c >> > @@ -36,6 +36,14 @@ struct page_owner { >> > pid_t free_tgid; >> > }; >> > >> > +struct stack { >> > + struct stack_record *stack_record; >> > + struct stack *next; >> > +}; >> > + >> > +static struct stack *stack_list; >> > +static DEFINE_SPINLOCK(stack_list_lock); >> > + >> > static bool page_owner_enabled __initdata; >> > DEFINE_STATIC_KEY_FALSE(page_owner_inited); >> > >> > @@ -61,6 +69,57 @@ static __init bool need_page_owner(void) >> > return page_owner_enabled; >> > } >> > >> > +static void add_stack_record_to_list(struct stack_record *stack_record) >> > +{ >> > + unsigned long flags; >> > + struct stack *stack; >> > + >> > + stack = kmalloc(sizeof(*stack), GFP_KERNEL); >> >> I doubt you can use GFP_KERNEL unconditionally? Think you need to pass down >> the gfp flags from __set_page_owner() here? >> And what about the alloc failure case, this will just leave the stack record >> unlinked forever? Can we somehow know which ones we failed to link, and try >> next time? Probably easier by not recording the stack for the page at all in >> that case, so the next attempt with the same stack looks like the very first >> again and attemps the add to list. >> Still not happy that these extra tracking objects are needed, but I guess >> the per-users stack depot instances I suggested would be a major change. >> >> > + if (stack) { >> > + stack->stack_record = stack_record; >> > + stack->next = NULL; >> > + >> > + spin_lock_irqsave(&stack_list_lock, flags); >> > + if (!stack_list) { >> > + stack_list = stack; >> > + } else { >> > + stack->next = stack_list; >> > + stack_list = stack; >> > + } >> > + spin_unlock_irqrestore(&stack_list_lock, flags); >> > + } >> > +} >> > + >> > +static void inc_stack_record_count(depot_stack_handle_t handle) >> > +{ >> > + struct stack_record *stack_record = __stack_depot_get_stack_record(handle); >> > + >> > + if (stack_record) { >> > + /* >> > + * New stack_record's 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 >> > + * set a refcount of 1 ourselves. >> > + */ >> > + if (refcount_read(&stack_record->count) == REFCOUNT_SATURATED) { >> > + refcount_set(&stack_record->count, 1); >> >> Isn't this racy? Shouldn't we use some atomic cmpxchg operation to change >> from REFCOUNT_SATURATED to 1? > > If 2 threads race here, both will want to add it to the list as well > and take the lock. So this could just be solved with double-checked > locking: > > if (count == REFCOUNT_SATURATED) { > spin_lock(...); Yeah probably stack_list_lock could be taken here already. But then the kmalloc() of struct stack must happen also here, before taking the lock. > if (count == REFCOUNT_SATURATED) { > refcount_set(.., 1); > .. add to list ... > } > spin_unlock(..); > }