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 71317E77184 for ; Fri, 20 Dec 2024 00:40:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 58F1B6B007B; Thu, 19 Dec 2024 19:39:59 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5182F6B0083; Thu, 19 Dec 2024 19:39:59 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 392F46B0085; Thu, 19 Dec 2024 19:39:59 -0500 (EST) 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 161DB6B007B for ; Thu, 19 Dec 2024 19:39:59 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id B1DB0C0500 for ; Fri, 20 Dec 2024 00:39:58 +0000 (UTC) X-FDA: 82913479782.14.02D4921 Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) by imf02.hostedemail.com (Postfix) with ESMTP id 882F080006 for ; Fri, 20 Dec 2024 00:38:52 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=NmVuwkdr; spf=pass (imf02.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.221.45 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1734655181; 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=04DMK2CBlaYgVmHyWh5D3Gtkvb9UdRG0+YSphVQgFvw=; b=cy3odm/cvmzmNG4L456/o7fphFjlXDhSIW7fA/aaH8c2BvNwLbqKNOA6b0S5oojUbCuWqO ckBtK69T3iUN1kQ3zgcTehYRLWrOfN95tnsqW+9ZfZfPh9xkWLsncTX4A55i0+dvSXK65U 4CFeR0tckmCuFRl8iZIWatXCzg7hak4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1734655181; a=rsa-sha256; cv=none; b=VXw2b+8+dsz22xR9bc6xHRJuD5ojjBnk/ab43pvSIbopX7cZnUHN3R/pn+esUUzMVN8coY 5Owmg9diYMX9vwzWct1ZIUx6OmZswH81FlP/hbAVF36leAmcCag+jZHN20N01cQAEAkVWa 0sMndtvScnlzCgKmTZW6YnkugbGTpdM= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=NmVuwkdr; spf=pass (imf02.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.221.45 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-wr1-f45.google.com with SMTP id ffacd0b85a97d-38634c35129so1019848f8f.3 for ; Thu, 19 Dec 2024 16:39:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1734655195; x=1735259995; 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=04DMK2CBlaYgVmHyWh5D3Gtkvb9UdRG0+YSphVQgFvw=; b=NmVuwkdrgmjEOhpqAmP+EU1t+wrB16XmBlV5SHnHEdybzIZEUcunT7keGwpXDZIcIQ NbuOqU/37PnaiizQDdWzKW/j5V9C9CgcFMt1AL8VVNwYbfp8061+XAIwTNuzyuqz/FPt XZrxlpnSx74USuQWvRnrhYdRxMxLj9YHANz//FJChz8KQD9xKtHMGvbAsFvkvzrYZ1wt lllyX1s6gfXpJzVMNH5otcOXHyVNZQ8LVQ8j4Tgn8sAIvV2HrxOUudY1HvkJVUs2CJ7Y 3LPXM7sNZzUcscXZ5TsQXwo4ZVh76dGXBktbQZyRBlSNI/dA4y90zWoaMFpPoToVpXP4 Cv1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734655195; x=1735259995; 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=04DMK2CBlaYgVmHyWh5D3Gtkvb9UdRG0+YSphVQgFvw=; b=d5g2dL4q1ZtcImXU2uKoY/TT1GR4u59SfeB0d2vnsSw0YuRNBDhBaaxb6wgSnBOKW3 iH4YTcYAAcOuO0EbcPkPJAwu2/lsNF1dkRi6RVcesFLCvyP2rnftMY8W7oBooLKWZw0q oWL36ErriGrEzcZPAa87mGoFcThbqqd8x2sYD3zSUsU7Bh441EUSuDOotw4rGygKpIR+ l+O4zHwmYkTUTbzSwI5/jJd1tL7H9aXN1PGi5clcDbJrSyhiWCPCs64RmABmpxt7H5TF rXMT+mM5Cx0PpDsnBHAJ2IS+tx4S0X8VJ2dS0Q2V6GFs3LrK2yxcWxbAKLKs9s2DJoMF zWSQ== X-Forwarded-Encrypted: i=1; AJvYcCXETI4Cmhl2Sico+Q8+6K3EPKOh4Dr7zn6pNvOlVPUb3D1frVWUA34nRymQ3+E6RpCDLYDkNjSIpg==@kvack.org X-Gm-Message-State: AOJu0Yyli+jTNQ/byBkTWTGVf0+gjfN4k6q07svEGcWDSGy4/lcB3Lg8 UrdkHhdoYIx6DPWE7o1FHsS9NEHUF4HIzy1fho93cb5TnziFO6aH6DLnBFj0L79T/XKYDplY/DG Pn8bH8d8t+WHdINBBRLBv8eXBsv8= X-Gm-Gg: ASbGnctDvPVF6JcsYEPsgI9/aF0IdA4TjdS5jA1ywXhnPswLs5WR0SDx+KsLPRY3qGt 8zuhG4Yw+r/naYu6Y9MZHhfPt3HKhANw+nEKPtw== X-Google-Smtp-Source: AGHT+IFDryDjaUKXZCODMa0NSyOxFIrLFC1sf5j4TOLkzW4dPJsrH5UZoPI/WEYgmMzo2wmrfC0Ri/2Yxfd+h7Ydi3U= X-Received: by 2002:a5d:6d09:0:b0:385:e43a:4ded with SMTP id ffacd0b85a97d-38a223ff460mr742092f8f.57.1734655195080; Thu, 19 Dec 2024 16:39:55 -0800 (PST) MIME-Version: 1.0 References: <20241218030720.1602449-1-alexei.starovoitov@gmail.com> <20241218030720.1602449-5-alexei.starovoitov@gmail.com> In-Reply-To: From: Alexei Starovoitov Date: Thu, 19 Dec 2024 16:39:43 -0800 Message-ID: Subject: Re: [PATCH bpf-next v3 4/6] memcg: Use trylock to access memcg stock_lock. To: Michal Hocko Cc: bpf , Andrii Nakryiko , Kumar Kartikeya Dwivedi , Andrew Morton , Peter Zijlstra , Vlastimil Babka , Sebastian Sewior , Steven Rostedt , Hou Tao , Johannes Weiner , Shakeel Butt , Matthew Wilcox , Thomas Gleixner , Jann Horn , Tejun Heo , linux-mm , Kernel Team Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 882F080006 X-Stat-Signature: aaheofmw1m7o3gwabfqda49xmedonqdn X-Rspam-User: X-HE-Tag: 1734655132-997089 X-HE-Meta: U2FsdGVkX1+Fn9bd7VN6gOHJbg57eT1hi0q4dSxfDi6pdR9reJxLk9JtM2ju/AxfUR/7AVzRmmusStTqwErGg/Vd9Ut9RV2W9vXPB6kR1VmQBrKptC3wUuaIo1P/2R6QKI8RXn4gCRkBTosy9tufkR+FhMSpS7Y/0RPcvn5EXlLJOTSMs8QyJywZsx1dgKTj4wuoIP0Prio+jUtfJfoIAoRzCl2ibEnh0rl2esLLEjVg4x8nuOC2uc2mlFS/DLXrhDaOq5sSEhPi1AQ9Gshia699TPDBbSCCgjj0e0BlJxIK+oVx/CJbPqSurhhlYwniNAg6tT4Fdhi2QcGU4WzgBRrITLe5JlbXQWQ5Eh+HUkpz6Gjq8en5YYznnNW6jCPXt6MUS5+l81LgJlP7xnIoDY/59cIRJzwPur7ZFeFeDCXWuo1W7ElSKkNqk9tVrnUtlNVI+psu6mPE6uKkrliH+O8r3h3F9vqNzEYOW0foo2XXu1rbL/Fn+a7Fp5Pso36f0YodkanMc9Ru9iW8HtAWnLDoLuus6nJJX91lFZmjEjgAca0dw2WRCFPm5w4n7DZsaHAsYro6TrdDdcCLwkEGjmwCKJYFHMbwkoMQYqC6nsR1cUkqhBz7/z9wlAGIEiOhbOeNuCmAzklJpwdK3av50s39StRwLdpwmdYTpGTlELOJyC+7vgFp0V2jt5zfKjjlNS09lqZLvhhsr3lstK6ILTxS84XbppolYSzHCwZ5Sxk2kTFKCG5s1goFzCfA9C+RqvU9oEyxCIOyRep94Apg10eYyGU53hAck5iNYU3raXOckUkZpFoW4gIWX5Jjy1bYmrqGHfr7xwy/uxv9T5Th39adGGpDThoK4DzdOwmKDvkWE/rxOI3QqzLFEbJ9i2hpzgFrGQ/gaEaMkKpc2G5SqS+4wS47ASfUzla+4lWVaG/B3pMM+S+EvCnKbeie6rH8Q1+XJigXrQ3eapFkkEq kbLy0D53 veAVqoyUEGPg2t7Q2/sMFLcPO+nIx8Cc9QdJBtWweQ5zjAklFhKba8ReHfORJlcdPc96DQ8BTN8mH+1Jw13z6JQVJQtbIXjp5dDgBaHH+WraqtHWMK1phwm8hA8IITGk3+od1+n/mE6W77nzEWJniDJ0wBO3bPA98ysIjEwBKPrHNp5ehE8FDpQOiKdn55nCHPf2oEw9BUkLgsBjG3zRM3eE7f9EJ9D6nV5/fLyIqxPGBErYKvQjA8oZ0rHDejj8GB3bFr3NCDif4hNxx/AKizluWifkZfx5Wujpsw1eVTaxjRUimvV6nCcDc/0e8SxW1kCzYsAUXrzeJEBGgCQGffpniwYbVfZOoS3yFjBb0TM2ZNLj/YJO1iv1p8QIhGvSMDMsBwimkHrDDzXVDgyRmPR0GblWY/iE3XKqy8gR3hpswBrhdXjlv61Aoq3lebD0Cjwt9bDc/BJPXvaY= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000023, 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 Wed, Dec 18, 2024 at 11:52=E2=80=AFPM Michal Hocko wro= te: > > On Thu 19-12-24 08:27:06, Michal Hocko wrote: > > On Thu 19-12-24 08:08:44, Michal Hocko wrote: > > > All that being said, the message I wanted to get through is that atom= ic > > > (NOWAIT) charges could be trully reentrant if the stock local lock us= es > > > trylock. We do not need a dedicated gfp flag for that now. > > > > And I want to add. Not only we can achieve that, I also think this is > > desirable because for !RT this will be no functional change and for RT > > it makes more sense to simply do deterministic (albeit more costly > > page_counter update) than spin over a lock to use the batch (or learn > > the batch cannot be used). > > So effectively this on top of yours > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index f168d223375f..29a831f6109c 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1768,7 +1768,7 @@ static bool consume_stock(struct mem_cgroup *memcg,= unsigned int nr_pages, > return ret; > > if (!local_trylock_irqsave(&memcg_stock.stock_lock, flags)) { > - if (gfp_mask & __GFP_TRYLOCK) > + if (!gfpflags_allow_blockingk(gfp_mask)) > return ret; > local_lock_irqsave(&memcg_stock.stock_lock, flags); I don't quite understand such a strong desire to avoid the new GFP flag especially when it's in mm/internal.h. There are lots of bits left. It's not like PF_* flags that are limited, but fine let's try to avoid GFP_TRYLOCK_BIT. You're correct that in !RT the above will work, but in RT spin_trylock vs spin_lock might cause spurious direct page_counter charge instead of batching. It's still correct and unlikely to cause performance issues, so probably fine, but in other places like slub.c gfpflags_allow_blocking() is too coarse. All of GFP_NOWAIT will fall into such 'trylock' category, more slub bits will be trylock-ing and potentially returning ENOMEM for existing GPF_NOWAIT users which is not great. I think we can do better, though it's a bit odd to indicate trylock gfp mode by _absence_ of flags instead of presence of __GFP_TRYLOCK bit. How about the following: diff --git a/include/linux/gfp.h b/include/linux/gfp.h index ff9060af6295..f06131d5234f 100644 --- a/include/linux/gfp.h +++ b/include/linux/gfp.h @@ -39,6 +39,17 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags) return !!(gfp_flags & __GFP_DIRECT_RECLAIM); } +static inline bool gfpflags_allow_spinning(const gfp_t gfp_flags) +{ + /* + * !__GFP_DIRECT_RECLAIM -> direct claim is not allowed. + * !__GFP_KSWAPD_RECLAIM -> it's not safe to wake up kswapd. + * All GFP_* flags including GFP_NOWAIT use one or both flags. + * try_alloc_pages() is the only API that doesn't specify either fl= ag. + */ + return !(gfp_flags & __GFP_RECLAIM); +} + #ifdef CONFIG_HIGHMEM #define OPT_ZONE_HIGHMEM ZONE_HIGHMEM #else diff --git a/mm/memcontrol.c b/mm/memcontrol.c index f168d223375f..545d345c22de 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1768,7 +1768,7 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages, return ret; if (!local_trylock_irqsave(&memcg_stock.stock_lock, flags)) { - if (gfp_mask & __GFP_TRYLOCK) + if (!gfpflags_allow_spinning(gfp_mask)) return ret; local_lock_irqsave(&memcg_stock.stock_lock, flags); } If that's acceptable then such an approach will work for my slub.c reentrance changes too. GPF_NOWAIT users will not be affected. The slub's trylock mode will be only for my upcoming try_kmalloc() that won't use either gfp_*_reclaim flag.