Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 13 Oct 2021 17:06:58 -0700
From:   Roman Gushchin <guro@fb.com>
To:     Shakeel Butt <shakeelb@google.com>
CC:     Johannes Weiner <hannes@cmpxchg.org>,
        Michal Hocko <mhocko@kernel.org>,
        Mel Gorman <mgorman@techsingularity.net>,
        Uladzislau Rezki <urezki@gmail.com>,
        Vasily Averin <vvs@virtuozzo.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Cgroups <cgroups@vger.kernel.org>, Linux MM <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] memcg: page_alloc: skip bulk allocator for __GFP_ACCOUNT
Message-ID: <YWd0ohzK7RScx9TO@carbon.DHCP.thefacebook.com>
References: <20211013194338.1804247-1-shakeelb@google.com>
 <YWdXv+RBjXvdmsK+@carbon.DHCP.thefacebook.com>
 <CALvZod6ZppPNk2XfvKFfdPhrsSF6NbSBKrOOOc6UyJMfDEfKoQ@mail.gmail.com>
 <YWdoj9FZy2B4oLj8@carbon.DHCP.thefacebook.com>
 <CALvZod7oYyGvHAQVO5fg5eCJefeU1J70iUS6-9k0gQ2S8-y7NQ@mail.gmail.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <CALvZod7oYyGvHAQVO5fg5eCJefeU1J70iUS6-9k0gQ2S8-y7NQ@mail.gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?k3uVahuG8Ql9Yqbh66iCi0uKEFrnR3FCjv2f8+bIWqdBcPGbzYm/HhBg1gXw?=
 =?us-ascii?Q?tOgyoJ3tc9gdMtd52sqNYVdqFs8jgbttKclSRq34j1oK9v/4z0yMd2eX3GeQ?=
 =?us-ascii?Q?ArmlqPfKaX6GBS8xAc1NfCbtxhdoKscsXy4Oa3t+qEIonKHgOgBKrbwrwTwO?=
 =?us-ascii?Q?dJM2VJ1Mu2m00BsMj66XodsOhV1ELnu4kGkxfqUfxR2MFNyEcMzV2FSQOsuC?=
 =?us-ascii?Q?WkQfzRRt9y02mwCyari6XsbaT6WYWEIU0ZGvF1hZqkrTiyaUXh5CuXilVZ3h?=
 =?us-ascii?Q?tKIcD7LHe1nh/4foFkrvhEBRx9qmvpFH25VzIYrj5rXcthTJWrZWud+VXjic?=
 =?us-ascii?Q?aS4W1phN9k/6hUoE0VYGl2QNKu3nyofEp5z9mWYKNRuITImO1a20m6FPJsj+?=
 =?us-ascii?Q?Go68YUtPcOt/wrgmerrW62sds9MIa0BOgqML+T//vM37lBwExlY51xTqHn6c?=
 =?us-ascii?Q?1wCA2naaMcGA370beMmsxZdGtcaaCSwGZjsjzyebR4DTBhBek2t7W1zbCCY7?=
 =?us-ascii?Q?1JYs85cV42yoRreYuz1JuNcHDm+mdeZLMn2Ct5dtcuzYKeINDglUZuuTkOhj?=
 =?us-ascii?Q?4msJ1Wh+RcWpA347GbDo1FcWQmMnloAdvvmdwmzyU1pwXHaCz2FbZ2IA4krp?=
 =?us-ascii?Q?GHDAHvegmESxNICsBacltV3LWXXz8udFsN6GLFtWYQ5xSzsx69whHz1WRnLC?=
 =?us-ascii?Q?azyDeuFxhqdJRxkVmNEY3eQ//scyFV0/UvfWVMeUyjmVrc4FEr38dbIJUEbw?=
 =?us-ascii?Q?vwgMWgKl87jtdzi6QLNx3VlloU1UOftv+UtLIsBS1x4loH5OiMMlTxpcpp24?=
 =?us-ascii?Q?WG8M52tTHCALpzk/Sd9UHBjBbczgxbG0cmMY5kaEC3yTGU30psTKQ8n1k0RS?=
 =?us-ascii?Q?3gJd90z+Xo0IASjfYmfppivKzdoH/4sKyRZ7pQbgi4cOmBfevcu3ohTRbox8?=
 =?us-ascii?Q?bDbbUdEQskTfkXpjivZ9gtRrvbyQqduZd6TIrazWRUi2c1MndZMh0fT7U8mJ?=
 =?us-ascii?Q?DeOtejIa0xEP/O+idGIEjqFHSi9hDh5ItkZbqShLadqb/zQRcZjmnoHO5QUK?=
 =?us-ascii?Q?/dGEH3oGfA3RyKtwoK6rTGJ1rf2STuWn5pfgriktwu4NY5hNgWkqrQOHBDq0?=
 =?us-ascii?Q?dwah0zIzZnSveLHAYt0QJZV3y2rFZI95kHru9RcRJyfpUifgeJQzB3GaPcoo?=
 =?us-ascii?Q?EkhEPeBoxmCQ5bgIDbZ1Yjt2ciuMZK18QGTri80ygeS0GoIPOcAO5K20Udgw?=
 =?us-ascii?Q?Sj8pS1Al8YIUHuwlSASYaRpylYp8Mes7nW+LdOJloU+3SCATmrK7njS7sNzv?=
 =?us-ascii?Q?W5t4GICCnta0x7S1GrEsVa/7qFA1hWjDfZEZK/luEKwIFeg3ccx29l1Z5nwF?=
 =?us-ascii?Q?UfTaMSA=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2456f236-6802-4ce5-0dc3-08d98ea68cce
X-MS-Exchange-CrossTenant-AuthSource: BYAPR15MB4136.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Oct 2021 00:07:02.8794
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: tQcWawxBkFqExkJA90DJPRWzNoZmbdtYPFEpr2QN09SRtI7V5Br9aG0TWlHTZDvQ
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR15MB2519
X-OriginatorOrg: fb.com
X-Proofpoint-GUID: lmM-v9Uro8Dq40K-DSuZQYso06W0QR6l
X-Proofpoint-ORIG-GUID: lmM-v9Uro8Dq40K-DSuZQYso06W0QR6l
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.182.1,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.0.607.475
 definitions=2021-10-13_09,2021-10-13_02,2020-04-07_01
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 phishscore=0
 lowpriorityscore=0 spamscore=0 malwarescore=0 adultscore=0 clxscore=1015
 suspectscore=0 bulkscore=0 mlxlogscore=732 priorityscore=1501
 impostorscore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2109230001 definitions=main-2110130146
X-FB-Internal: deliver
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Oct 13, 2021 at 04:45:35PM -0700, Shakeel Butt wrote:
> On Wed, Oct 13, 2021 at 4:15 PM Roman Gushchin <guro@fb.com> wrote:
> >
> [...]
> > > >
> > > > Isn't it a bit too aggressive?
> > > >
> > > > How about
> > > >     if (WARN_ON_ONCE(gfp & __GFP_ACCOUNT))
> > >
> > > We actually know that kvmalloc(__GFP_ACCOUNT) users exist and can
> > > trigger bulk page allocator through vmalloc, so I don't think the
> > > warning would be any helpful.
> > >
> > > >        gfp &= ~__GFP_ACCOUNT;
> > >
> > > Bulk allocator is best effort, so callers have adequate fallbacks.
> > > Transparently disabling accounting would be unexpected.
> >
> > I see...
> >
> > Shouldn't we then move this check to an upper level?
> >
> > E.g.:
> >
> > if (!(gfp & __GFP_ACCOUNT))
> >    call_into_bulk_allocator();
> > else
> >    call_into_per_page_allocator();
> >
> 
> If we add this check in the upper level (e.g. in vm_area_alloc_pages()
> ) then I think we would need WARN_ON_ONCE(gfp & __GFP_ACCOUNT) in the
> bulk allocator to detect future users.
> 
> At the moment I am more inclined towards this patch's approach. Let's
> say in future we find there is a __GFP_ACCOUNT allocation which can
> benefit from bulk allocator and we decide to add such support in bulk
> allocator then we would not need to change the bulk allocator callers
> at that time just the bulk allocator.

Ok, no objections from me.

Thanks!