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 27F96E7AD78 for ; Tue, 3 Oct 2023 16:59:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A52AB6B0252; Tue, 3 Oct 2023 12:59:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A031E6B0255; Tue, 3 Oct 2023 12:59:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8CB376B0256; Tue, 3 Oct 2023 12:59:23 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 7D7936B0252 for ; Tue, 3 Oct 2023 12:59:23 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 4C6F91403D9 for ; Tue, 3 Oct 2023 16:59:23 +0000 (UTC) X-FDA: 81304761006.14.99631FB Received: from mail-qk1-f174.google.com (mail-qk1-f174.google.com [209.85.222.174]) by imf24.hostedemail.com (Postfix) with ESMTP id 4C5AC180004 for ; Tue, 3 Oct 2023 16:59:21 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=MQnCUq+u; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf24.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.174 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1696352361; 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=Qjf57nVPafK9Z4I/viF94cG3rR6sZipxHJbMn8qgU2s=; b=L7g+7aDFo8pVJQf40nQTZOtkuluo6I8MFMFkcpSUb423WXk7Q2mMQIAf33pFooJbPwdTs0 nU/qmZKltbnbOlkJvJHp7pTD04LQoZl02hTxJuXOD347Zt7EYOWTkfIQJ8n9xy3yhAyGZN kdUtHbWl7H74YlNwfXtqvLaBonCtvCs= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=MQnCUq+u; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf24.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.174 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1696352361; a=rsa-sha256; cv=none; b=Q5/pSekOKr91DOZkswXAH/pSBEpFICyFqmmeUxwF+VVL5bPQP5/k1EyzyJTCV+RQNfNnvi cKfrnMpFr3+biaB5pvqwwh3hjFyuA+XsvaxWOMWAzoqwAEJ4oVRvHFbOXEOdexL1CKRuGL 3sg+FadIvJ4LsCG6IDAP9q678Ou1bPs= Received: by mail-qk1-f174.google.com with SMTP id af79cd13be357-77428510fe7so936385a.1 for ; Tue, 03 Oct 2023 09:59:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1696352360; x=1696957160; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Qjf57nVPafK9Z4I/viF94cG3rR6sZipxHJbMn8qgU2s=; b=MQnCUq+uEg2w2rjiJVKSOkBfLb9XNIhWLwkoZNzP+5/ErmSgZ4CxWic9Ua8FteucYE kbjkrjG8KSzOE3rhc5VgzWlZrCcSDQRM8ycj9S6UrKWTb9a4dlk95wAL3lfhGBwSZhSL rmNExnOh/wHOp4i2OFmSBgYYVr0xeNSy/SwUphuZIZ0g+d/ZvifjWUBGhKNl/j6DILi/ 4qRMYPyfZDEW5Qhktkrbe/RgO/zV7qa5tKtNw4gC4+qV1XqWvfBF8nT/3r153rdJG5BU DSFISJ+o1GzkWAcHmoSQD9ECHw28gzt9bQVxQiq5QFkF4xvskSB4DuJ9HYOtl2ZQ5azY apew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696352360; x=1696957160; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Qjf57nVPafK9Z4I/viF94cG3rR6sZipxHJbMn8qgU2s=; b=PyW2KClZ6wbRGjHHEZE334mCRfgWN05iUhfHqv3u+77XBDGNab0qk+43uwRMhB+1gz xFmNbmR0ViMW5aTdAwYYTO7djdwIINIR2a85NgkbwQ/oCnZ43PtVaJxe3GkFrbntvgQL d/eMNxCpx/nzIeiwuIwsWhTjT6GJN3bl/ytlwQL6PAhxK0+8199cPwlpToqOeRasHzaV zEzp/6ANLiaxeilVP2n0gznxf4yxPXDPnB/qJRcZKJStEL5s40MI0nzVY/Ottkhw5ovm phNINbFM32lg49UXGCfexF8Ye2EbNtf35dBcf98lAblVogxNwBX1Ue47RDuU9vGnA32y rQCQ== X-Gm-Message-State: AOJu0YwJtuojSG/xm6RR4+3yppUD25TVPwCTTBVJO3f9NZw5FBLIqhkL 2WjgW5sD+lRKvXO1SLUwHGABsw== X-Google-Smtp-Source: AGHT+IEbPCOfzMpQJ91l0JA53AJl5n/TwHFD/8LajVttQ2sjiRlMa9UwFyAepiDMsF+8h0fuH9bv8g== X-Received: by 2002:a05:620a:31a8:b0:76d:a27c:245 with SMTP id bi40-20020a05620a31a800b0076da27c0245mr3474195qkb.7.1696352360273; Tue, 03 Oct 2023 09:59:20 -0700 (PDT) Received: from localhost ([2620:10d:c091:400::5:753d]) by smtp.gmail.com with ESMTPSA id t5-20020a05620a004500b007742ad3047asm600617qkt.54.2023.10.03.09.59.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Oct 2023 09:59:20 -0700 (PDT) Date: Tue, 3 Oct 2023 12:59:19 -0400 From: Johannes Weiner To: Roman Gushchin Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, Michal Hocko , Shakeel Butt , Muchun Song , Dennis Zhou , Andrew Morton , David Rientjes , Vlastimil Babka Subject: Re: [PATCH v1 2/5] mm: kmem: add direct objcg pointer to task_struct Message-ID: <20231003165919.GB20979@cmpxchg.org> References: <20230929180056.1122002-1-roman.gushchin@linux.dev> <20230929180056.1122002-3-roman.gushchin@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230929180056.1122002-3-roman.gushchin@linux.dev> X-Rspamd-Queue-Id: 4C5AC180004 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: phkmgokmosnt13oytciuoats8j96t8b3 X-HE-Tag: 1696352361-867273 X-HE-Meta: U2FsdGVkX18fHvb47tkE5XJvRO18PpX6copEcsv98BEGhi+GYYDQTq38dHqQLH6W6NFwi68rPE4AvjJ2dG4LzDgwfTybXr95PYQrlvtSUMF/sx3InUhhLUQKVJh4vfyVvO+pAfPLKRBcmCwVQBdlGFTNT1aEGmkXMbgDx/G9SHlu/OojnVO5G8z+MKTKrS5GRzJ/u/c5Do2R68OYLc360mhabDnCLhtv/wIs9d/dki1DWOAUMhpsOZ/ZaiLbkT4uW75Dm45nFlBf+P8wKJH/ksp8dG6TUPjCyDFX6JhRu1kmOBM2f8/HQJAt2ePHzXb8ijcWMR2rMAI5bVymD/5XBePDK/ciBIbpu23nARq+vJ33eAzlR+VLdEywvGfrI0Fn+A6vgavVSND9hugzlqT4YIACAoFuSaLGscMol3A0HJ5UbeiQVNv0oZbN/nsruc52e6/P/3PlZ/j6OMhNxe/1/KzasZTHGjM5P4mA3rHUI6O/kTYsvaTt9RE8suYDWjkZiCW4aJVolbNNieIdH3L0Yz2J5QaJxD1xzZP3AsSTkBEPM7eOGyRxB4wEQGn8bnfvC/92zeuH+rU9q6LtY3WdmYZ1lIzAYhnTMMxyOC3h9TXJEkoMsdoxb+HHqOcX6PdNEE2lnhKSdsbg/7AQiASMf/GnCJB+75KM+XYW6meFMw5g9gUt50rooldusyMgP15aqHDrRFhGSX094PMPDaw4l1Iv89TOUpX18cXoNE6jVIkYxqNwB7wAFhkPayPv+8zebMkUrpLAW4mAsZyCp71sP4tTezOSh2FZlgndNekA3VZuraENR4QMTvx54jpvgvJl99xb0p9TtCL4nQl5AhE16UWkROK09wDloQJmXL22Ao7+KIoBL/1jrE8ujfZdDcc6LDyFEFc9PX+AQh6DMbMPZG6xBJLxnM+z2D+evfFBwDgK+NO5i7SvksPE3EsWJPXvBbvZcuv7d8Mbx1icrRF RXr75GN3 vlLNUaw5NFp9lvn7zQODP9lrfQ0e4Gra7fve2uqsHQBpPcJ4SrBnOh4hzQIJ+PWubNlBupwhBEIou4SuBrk+Vool6cDPUapPhvF5yhF//7Nr4JkhD8J9rUSe0whZhmso8XNjAh7Ew1PPp/Q/Ogqt/m3lCaHxurlJ1/NYIgOqy9gp53JjvDjt7DGXFUA55ffrQkfQY9f/kzJ35X0ckg787z3SFIev5Nbq//aH5j92ckmMPgtri2mYdeECkhWhyD6jsCGyYn/JnK74W8Ar/KRraEgIGdY3nGFCm80fW72NcO/2j5C8jTGcYeniEtJz9A6n+imTPXrFBs9VZuz1tzWH++DL30v6h7uwIHEvI5H3lcCqrL6DXnv/UNXa7U3UOaEsUVk4UiIYLkqV7AI2mlcgV4zCTGupvlFWAcZZri382PKMikvO/uAe9r3bYU5gYTbZv64GQhEoElgT0ay/154yzqCoctIB2M3xKuMj6Xi9Z5OW6s8NMPSlPx+WNElKWJpAOVcViua+ul82EWO+xLHN71j74HA== 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: On Fri, Sep 29, 2023 at 11:00:52AM -0700, Roman Gushchin wrote: > @@ -553,6 +553,16 @@ static inline bool folio_memcg_kmem(struct folio *folio) > return folio->memcg_data & MEMCG_DATA_KMEM; > } > > +static inline bool current_objcg_needs_update(struct obj_cgroup *objcg) > +{ > + return (struct obj_cgroup *)((unsigned long)objcg & 0x1); > +} > + > +static inline struct obj_cgroup * > +current_objcg_without_update_flag(struct obj_cgroup *objcg) > +{ > + return (struct obj_cgroup *)((unsigned long)objcg & ~0x1); > +} I would slightly prefer naming the bit with a define, and open-coding the bitops in the current callsites. This makes it clearer that the actual pointer bits are overloaded in the places where the pointer is accessed. > @@ -3001,6 +3001,47 @@ static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg) > return objcg; > } > > +static struct obj_cgroup *current_objcg_update(struct obj_cgroup *old) > +{ > + struct mem_cgroup *memcg; > + struct obj_cgroup *objcg = NULL, *tmp = old; > + > + old = current_objcg_without_update_flag(old); > + if (old) > + obj_cgroup_put(old); > + > + rcu_read_lock(); > + do { > + /* Atomically drop the update bit, */ > + WARN_ON_ONCE(cmpxchg(¤t->objcg, tmp, 0) != tmp); > + > + /* ...obtain the new objcg pointer */ > + memcg = mem_cgroup_from_task(current); > + for (; memcg != root_mem_cgroup; memcg = parent_mem_cgroup(memcg)) { > + objcg = rcu_dereference(memcg->objcg); > + if (objcg && obj_cgroup_tryget(objcg)) > + break; > + objcg = NULL; > + } As per the other thread, it would be great to have a comment here explaining the scenario(s) when the tryget could fail and we'd have to defer to an ancestor. > + > + /* > + * ...and try atomically set up a new objcg pointer. If it > + * fails, it means the update flag was set concurrently, so > + * the whole procedure should be repeated. > + */ > + tmp = 0; > + } while (!try_cmpxchg(¤t->objcg, &tmp, objcg)); > + rcu_read_unlock(); > + > + return objcg; Overall this looks great to me. AFAICS the rcu_read_lock() is needed for the mem_cgroup_from_task() and tryget(). Is it possible to localize it around these operations? Or am I missing some other effect it has? > @@ -6358,8 +6407,27 @@ static void mem_cgroup_move_task(void) > } > #endif > > +#ifdef CONFIG_MEMCG_KMEM > +static void mem_cgroup_fork(struct task_struct *task) > +{ > + /* > + * Set the update flag to cause task->objcg to be initialized lazily > + * on the first allocation. > + */ > + task->objcg = (struct obj_cgroup *)0x1; > +} I like this open-coding! Should this mention why it doesn't need to be atomic? Task is in fork(), no concurrent modifications from allocations or migrations possible... None of the feedback is a blocker, though. Acked-by: Johannes Weiner