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 5B6FEE7718A for ; Fri, 20 Dec 2024 08:25:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A7F566B007B; Fri, 20 Dec 2024 03:25:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A2EFC6B0083; Fri, 20 Dec 2024 03:25:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 91DD86B0085; Fri, 20 Dec 2024 03:25:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 738096B007B for ; Fri, 20 Dec 2024 03:25:00 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 24176A085C for ; Fri, 20 Dec 2024 08:25:00 +0000 (UTC) X-FDA: 82914650952.26.E6B210D Received: from mail-ed1-f52.google.com (mail-ed1-f52.google.com [209.85.208.52]) by imf17.hostedemail.com (Postfix) with ESMTP id 227E140002 for ; Fri, 20 Dec 2024 08:24:31 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=SmunD39m; spf=pass (imf17.hostedemail.com: domain of mhocko@suse.com designates 209.85.208.52 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1734683062; 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=Urqu9iSmfH6SROfO9Gb6Yy0aeXqtUTT+yKgW/YW4rGs=; b=APY1EFyi2I7njnSl4ZsNeiHz2FsIlpLRqJHuiXNw/l3ywSbnfrh4PuOJNaeDCONsvnAKkg RgZZmSLj+/DTuGM/qwjrPEFdQj9eEmtXQEHrv2IcrBPW5g6Z7DCQZZ/mQNk8K/fx4DD5L7 Vt5tqxOVeW1QWEW3UN+80n9iQ3MCycI= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=SmunD39m; spf=pass (imf17.hostedemail.com: domain of mhocko@suse.com designates 209.85.208.52 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1734683062; a=rsa-sha256; cv=none; b=kD8Xx3XMe21SQvTnYaMCRYeE/+3OlabmR5T/+rhbEyBRNgDXL9bP0D0o4Ptl7xECEGBpPZ mnLlOGxTvlUJ4DckejW2vEWDG22ElcKao4/eUnWD0c5jlVKsEGKFXiTEZVudS0iyd5JzT0 p2CVVmfEve8VUbhl1NxzuQaniqqa7DI= Received: by mail-ed1-f52.google.com with SMTP id 4fb4d7f45d1cf-5d3f57582a2so4926448a12.1 for ; Fri, 20 Dec 2024 00:24:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1734683096; x=1735287896; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=Urqu9iSmfH6SROfO9Gb6Yy0aeXqtUTT+yKgW/YW4rGs=; b=SmunD39mFYh6VKUhib6S1Sk8UboSAf3wdgGkN4E12j8zPfoL3x2L7bi5jOzYOaVOpI R2J+Xrd3H3ST5Ggb7xQW/3+iV4OSjSKtdohel/IKUAn5N+Dm3Dh9/0p3f1M8dRdRqy1r AkEFE5Lm40cTfWA9S45xC/I6DSQMaubkceMnMV01k6o9jsO5uNp2vC/I8Fmf8FQusAqm ecAPlQbQlrj4WJZiW1XTx9XcDl9ClN2ZhNBw0e8/JzT8apTo0K92oN/OYADCqV9DGAPc OyMxnb31okEOLCBMEd+KR3FsWDIU2Hs7sppUYPrv69szV2B4PFXqZmsb/kNK430UPJhI /xaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734683096; x=1735287896; h=in-reply-to:content-transfer-encoding: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=Urqu9iSmfH6SROfO9Gb6Yy0aeXqtUTT+yKgW/YW4rGs=; b=qDC8K5K/RO5/VFwasaam7u8V6YdAaZH0bjPIIztVQ6l0ZyNfZnqdBj4lpcgRI9XLcQ YldPhjS6WvIO5EAsKW1n5mTzU4DRKrZdroRD8/8cRU618z4I1DRRlXB+UNQ3DfMWF3HH lk4RnHT3mdnQGWGe/Y0KVVBwVZxrZ1B4ZOGOua8oOobae1Tho71e469a8MaMRFFdgL0G gFYGYgX7o21UIWexGcFrTnM5LPJQNB+GsycTZbmyQg49MjONZqHsVUzvGuoT4RfBB+Mz ujS3q5GDZwWpZ1S4w4QBpqZiNnyMBLc0OdQibu2K/wDgtu+Wr+TOKEbeT2/M/GCHqDvZ dmuA== X-Forwarded-Encrypted: i=1; AJvYcCUfnabpEX5cNnOkU++lE1RlIY7vaEw8V9X/wj33KBSEb9esyMq13C931Lefwf66Mo9SlrJcNAC7VA==@kvack.org X-Gm-Message-State: AOJu0YxfVwJzwHVNolAohciWUfu8kUwSFJxP+/T4RqANl9fZ/SMBr1+O Vn1Fbj6EXMpXdW2knegvcUrvSApOVCjypZwIhA+kX+H7OMLWLYmv+5MK8PEXZ5w= X-Gm-Gg: ASbGncvRYnrYT6ipc9pANvRawPQ3OHupwO1jQri+rkBXiS/jXKhkiucAdKWuI3YKu9E XfjV3coZwr1aSlLnc08QbWHcskFUkp/rOcMEdI2dEfGdH/6VfImgpe/t7aGB3Jv1aUyvJpulRx6 L2MDWVDfvmwtcKbehmUkV8glelP4hoduGdb4OmsiNYOO5I9gHkP5VQ0FEmhknCawwkXTIpnmDZV 0ak7FGS8JW26X7PpTuC/wnbUpzLsfUa1XheYZWfHLKy7E0= X-Google-Smtp-Source: AGHT+IEEJw1PFfgfh9Omb3EZ32kpp5CtPbHnxjC3BlcrCvpiIwXYmAD3OK1KKBMHhPcIP797MTnnTQ== X-Received: by 2002:a17:906:f5aa:b0:aa5:53d4:8876 with SMTP id a640c23a62f3a-aac08228224mr538509966b.20.1734683096271; Fri, 20 Dec 2024 00:24:56 -0800 (PST) Received: from localhost ([193.86.92.181]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-aac0efe35d4sm151853766b.108.2024.12.20.00.24.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 20 Dec 2024 00:24:55 -0800 (PST) Date: Fri, 20 Dec 2024 09:24:55 +0100 From: Michal Hocko To: Alexei Starovoitov 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 Subject: Re: [PATCH bpf-next v3 4/6] memcg: Use trylock to access memcg stock_lock. Message-ID: References: <20241218030720.1602449-1-alexei.starovoitov@gmail.com> <20241218030720.1602449-5-alexei.starovoitov@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 227E140002 X-Rspam-User: X-Stat-Signature: xj5zpco6b4xu84jtyg31jajnqz434z3g X-HE-Tag: 1734683071-180374 X-HE-Meta: U2FsdGVkX1+PAOQuiYJQS6Gxw0Q5DOGhHSPUYJ3/2JWbRZuWbrQw2vY/oYpr6wO1yakmBdtz22lYX6AEe3it9xDU4iTP1mrQcXDiDMwPK0xOnezRvL9lTmcfdgWThYyAashaBdSAN7uUwp+G+G3Vqtz/QJAxi4K0W4ZZtxriU4qiHQv0yVx+eknfW8PYp8XiVP1FOvmACCjAmsCDxm587iLTXxtMkv9ithln1u+pesIFFFJi3pKoPPVKtS+HozDjCu1bW+7geQpu6XIffztXVwPKhND9fVe6roftpUGyODtg6D3emN4ma1ZLCcewR6HkN8j5MTeB2s2wL0tP6/mPF5ICyhAEI2UdJLpAX9Biccau5U1JZUfVDuroKZhQzOdVaV5Puq9Bj+4x+Rux/nFdqfxynFpepq8hCFm0EdJzEXzFeopG3kRd0t3OTIxmGHZO8F9R/RVi5cLNZIZjv29bCdaSbChOk1nG7qsFxDRHhp0d7RsnbcYaz2MVHqqcst9CVvEyx9JVEmuqG+kCUTfgJRTXmfwbGy8yJNULA2Abx0UvHN+XaAOMJdKJkqT+CIvsjxvOOA2F6CWVz2BG/ForCnZIlyUej75el88YDSifpZ4WT5boui31+5zO06+fk+i7aXimB2yDNTbWU6AGR5/mAyzaVba9sibJclskGtu3RnWOhHdFG+BvuomCZKq4+KqOwq7SsqNdNTb7Ne5syGzq51DiDrZP9TGMfCIHwSE9ZTRk9epM8pw35c6RHSdKsAfTvk+E64vG8RcmIT8PDQOWmIeq3RJX6wrT+TkJw61JR9XGBUed+2P2SDNK4JDbTNYqXw+9DUWzqz00hb5Dfuyv7NUSsV+gMOFmLz8xJR+9oyHXLztljVoruqkcdEUBKFfcNfESBkVqlTf2wt58yaSQjRfMGjkyVTLCAO7MWn1moR7Km+1IXgEHL10TWJD0kLvUtF3fAjwwmvqFg1fOYuC L02qtZV/ C7vd1CY6aO7QLC+560srBvXSHz+1tkLkmANq4H9FTktu+bffkUrlJfyD2SoY3820IsZkle0xwpGTA9gMe1fLEnJ4hhU4jwKD7myLhoG3fttDjb5EhQII8esr1YHDNt6k2ss93zdDDPoLueE1pqXzdTnTsrNn95ddb3S03vddlk5WN/kY8a9vMeMCzRoYDB6ZfOJFdI2lvKKbJszxORC1sN1zlSdef1Ut42I2CmP8byndQfvpmHGWXYFZN9dZKJZunbmZUN6Yb+JGVc2dsvkRaO3vcK1ceSNVhJXebOzzsKlVXJ9H8TgiHjk76NSbRiGJJVyzgTHDtOiS/eP5jfP3WxsEtS1Qw9zzWOR4mCYaSmfx0w0quYL7aYskWAXz1x5ijcBnnaHpbhVf9khkrvl+0Q5fvjnnUqtu492X8dAtXNhkNxD0haeHA+i1SLbcYqpw5+eMY98l/d3Ho40gHNKJweLciBKQc+2krJ5RvEczzLyYfa85+K64fZHAYXRneWZHWd5vu 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 19-12-24 16:39:43, Alexei Starovoitov wrote: > On Wed, Dec 18, 2024 at 11:52 PM Michal Hocko wrote: > > > > 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 atomic > > > > (NOWAIT) charges could be trully reentrant if the stock local lock uses > > > > 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. Because historically this has proven to be a bad idea that usually backfires. As I've said in other email I do care much less now that this is mostly internal (one can still do that but would need to try hard). But still if we _can_ avoid it and it makes the code generally _sensible_ then let's not introduce a new flag. [...] > 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 flag. I wouldn't be surprised if we had other allocations like that. git grep is generally not very helpful as many/most allocations use gfp argument of a sort. I would slightly reword this to be more explicit. /* * This is stronger than GFP_NOWAIT or GFP_ATOMIC because * those are guaranteed to never block on a sleeping lock. * Here we are enforcing that the allaaction doesn't ever spin * on any locks (i.e. only trylocks). There is no highlevel * GFP_$FOO flag for this use try_alloc_pages as the * regular page allocator doesn't fully support this * allocation mode. > + */ > + 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. It certainly is acceptable for me. Do not forget to add another hunk to avoid charging the full batch in this case. Thanks! -- Michal Hocko SUSE Labs