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 6BC7CC4332F for ; Thu, 14 Dec 2023 11:30:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DCA928D00AA; Thu, 14 Dec 2023 06:30:46 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D79868D00A2; Thu, 14 Dec 2023 06:30:46 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C683A8D00AA; Thu, 14 Dec 2023 06:30:46 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id B2CCD8D00A2 for ; Thu, 14 Dec 2023 06:30:46 -0500 (EST) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 754E4A1E5A for ; Thu, 14 Dec 2023 11:30:46 +0000 (UTC) X-FDA: 81565206492.20.3A907DA Received: from mail-wr1-f41.google.com (mail-wr1-f41.google.com [209.85.221.41]) by imf19.hostedemail.com (Postfix) with ESMTP id 89C451A0026 for ; Thu, 14 Dec 2023 11:30:44 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=linaro.org header.s=google header.b=KqZ8mIA7; dmarc=pass (policy=none) header.from=linaro.org; spf=pass (imf19.hostedemail.com: domain of dan.carpenter@linaro.org designates 209.85.221.41 as permitted sender) smtp.mailfrom=dan.carpenter@linaro.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702553444; 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=ZGyR5IM589pNwJshXT2QlygyEFYgsSWq45p1PM9PHw8=; b=IDl+eg6DnSX5bsXn5FYTf/PwuWcwCxBFAwFZbB7PUjc53MRuTVXBn75A/JUCMzX2okipev dLBfWjY4Rjhp85OmE32EqOfaarLbdqLcJB83jyjw8gvB5BZpmmN6PBXRShkziWpIoWtNr6 /PdDjj4xyYrIXFVvJbWcI7GYQNJem8Q= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=linaro.org header.s=google header.b=KqZ8mIA7; dmarc=pass (policy=none) header.from=linaro.org; spf=pass (imf19.hostedemail.com: domain of dan.carpenter@linaro.org designates 209.85.221.41 as permitted sender) smtp.mailfrom=dan.carpenter@linaro.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702553444; a=rsa-sha256; cv=none; b=IcRk0y7u83NxwHangxI3w8LN745XIelXkc0aydt4v+jRVYLAeNUr4lmGP7ohEXzMP/JMxC vs+mAxVDmcMvd8oR6FkwB1sGubkUllin44VgJvjxf4H81YdoaZJxLMleH1Sp8kIBgXGyl8 KqN3hGdkGw5WqANjDSDfat1p9ZQ8m4g= Received: by mail-wr1-f41.google.com with SMTP id ffacd0b85a97d-3363aa1b7d2so1443262f8f.0 for ; Thu, 14 Dec 2023 03:30:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1702553443; x=1703158243; 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=ZGyR5IM589pNwJshXT2QlygyEFYgsSWq45p1PM9PHw8=; b=KqZ8mIA7hkukBQvLWMxmx8Ibdf3BSwxW9eyYVSEsA1rlcoLbaeCemoEZkgDZa1o7De qI1VGbYm5JOx2nsLqoRstawMnfsRNfVBSUMZTfyqOZI7lrKJYnOnndHmNe5HFc3RNcdZ 42eAehyIwGBXPg5O9MOMjo7Nfx/Sq/ow7oRKJLtY4NdxjFcbpXYBeRblC+lVPMljs0QC px8Q7lZ2GPkP9pvtzAMe6ABVA7M2h+ZpkbZj3+kIIQU+qkfMcd8R85o8jmP5hqbEBcA9 lM9rBh19fGZYXy+1RvN7SWcVmahbv9YNvcBuUXooyeo1DqDRamh1VoFTonM5t0+DCzac DpIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702553443; x=1703158243; 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=ZGyR5IM589pNwJshXT2QlygyEFYgsSWq45p1PM9PHw8=; b=EPDAdGiZbAdqdRVewIYEWg/Ljku2ui3/x2fiaftv05oJzZ06x2IZ5iaG0r+wyfbVoc aCF1NK8ryVbFUafaHsGKcfgfX5PG3WVAqEGc5c8wv/Ir/K6SGCVnR2hVWslwB8DT8NgY Ydzu9UDxqSFpNzZgQ6TemyfD4fBm80GO/mBE/KhhPsFvzOyZwd0pX1JeZinE7kyPilZa s9L05FkDI3/NESDjXUgfQsQiIKxVibJ0s53Wm2mOyVFXWfqdqF3cbqZr7IsuQnZIqAl6 GsSy01DlojoFhZRbjpSyLwywWjS/zEtS5JZwX7NsGgDU501J58aio01HMd54sirAeQBl dqxw== X-Gm-Message-State: AOJu0YxaXzGnPzzqPFylGLvrSUKey36fIj46/GudZbUzgao25vcu5AUa /WLYnt+rkyn0ImVMe9Xr59WJgw== X-Google-Smtp-Source: AGHT+IGlunMuhMf+wJNo9In2hayzFruWkeU7cG8jgxWDtUBf1ufk3oh/44gl3xD2yS1qQogfw0HhwQ== X-Received: by 2002:a05:6000:11c4:b0:333:2fd2:3be2 with SMTP id i4-20020a05600011c400b003332fd23be2mr3801476wrx.155.1702553443052; Thu, 14 Dec 2023 03:30:43 -0800 (PST) Received: from localhost ([102.140.209.237]) by smtp.gmail.com with ESMTPSA id a4-20020a056000100400b00333371c7382sm15875496wrx.72.2023.12.14.03.30.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Dec 2023 03:30:42 -0800 (PST) Date: Thu, 14 Dec 2023 14:30:39 +0300 From: Dan Carpenter To: Ryan Roberts Cc: Andrew Morton , Matthew Wilcox , Yin Fengwei , David Hildenbrand , Yu Zhao , Catalin Marinas , Anshuman Khandual , Yang Shi , "Huang, Ying" , Zi Yan , Luis Chamberlain , Itaru Kitayama , "Kirill A. Shutemov" , John Hubbard , David Rientjes , Vlastimil Babka , Hugh Dickins , Kefeng Wang , Barry Song <21cnbao@gmail.com>, Alistair Popple , linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v9 04/10] mm: thp: Support allocation of anonymous multi-size THP Message-ID: References: <20231207161211.2374093-1-ryan.roberts@arm.com> <20231207161211.2374093-5-ryan.roberts@arm.com> <43a8bfff-f939-4f2d-a8cd-97306d5e44c9@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <43a8bfff-f939-4f2d-a8cd-97306d5e44c9@arm.com> X-Rspamd-Queue-Id: 89C451A0026 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: r7dgb67q1y5ec1pii7huka6e9c1jdeur X-HE-Tag: 1702553444-122145 X-HE-Meta: U2FsdGVkX1+fFujDCFthuCAgZNvPC7nSg71GDZ/alQbUoUddvHhNhH7sgJMIwzzEMxGr+hbjajOR07ae7oVg1+4eoL0Et4GTCDUQtey4wOxR7qcWZGDFJSf9xaNHLLjQDhIOKpIkmrj1u79A1J03AdZlTkwIvj6aWEKqd45OVnHO2XChhOFSKs6ZfTh8kLW2/z8w55VX4CfzFtGaqNkP6ZFBqtTWv8lyR6aq0px+qdLrV0mn+ozOiHg+vFqUOppSX5jOZsZgRZDHBEmxJr8jcKBXHNRlhZZW5JCkbv1xqyTIoDvJHkrIZCYgho5OUUVEU63ttkeFbSWbVWkCGY242bPLBr2IJIoOTTp4CZgU3ysAfQedc+Ch/dIpaPyjhmRJgxuSaEkvNen7inrqXIqcpxt9mW+l2aVrBIk7FD0ddT9v7HQt4R8jUg9IyVrb371OTEV+kiW65/t0tpVVp9M7ffdXEayxvqslSYkL0lW27zmCiFncr/u6MlbtOmXwCeNiwmToMsyCHr6iXlYJmAKlKCBATaVuHsPFiOThCseFRhjzhccXUC1whqCIO+xcnyLA4mgBdEROcL49uaa8TpGhn0024yaA0g2eBgGRGlfsCwroqE0qe957ZCH+PbDB4vpbhT9oQ3Ux1hseEFkfmRWGBnYqJSVTHXMAQxVgn3fhnR+7t5yYeGlVuXWtLW/52Fr37/RtidkJjxeNFdzKTpwGWJS/XXjKxTjXD9Eds0myiz6MuT72nNPumHK43x9ic0R5ZWbEoF4pe6P56znqDznAD32h1zGgMFxL9glWbXreWudjdxrB6cz67imdOlEuIPBxanRwR9yDUmUkBs7yGeqp7m3K5DYkD2iHU6N7rBNWm1BTaO0sS8IK+CP87UN0wHkXlq44ha/ut0/t5kOwe2aHq9KTQykoQbpdIqtYSHMNm4+ALYf42AvkSv8HfzQ7paQZsSmMGmQSKduVDwlAV4C rguMdl+d zObnLn51ulkMU++AL5MaaNsL7WftiOPQbkAkZMs3P3BGeRJEQrzQkjoTobDFlF1JH8xZ/rGEwx4emlxKhe7AMmVhpj+rMLjNvbVedS322NkoPF9VZaU0bGZgs5QtDwsLNUyerrpXS4jewgYtrQOsuSA0w31mtFrYwiJTA0tgI7BOgbHXUca+L1iWZRT+Mq9vwRULp31Hg5cmWnbgacTSBGUgDBbRHyolHYSU8GsCXPSnNzFfQqVjJ0/Sj+gsWe1xnVYhYhlFantB+jRiAyG0aOD3byYkCu/q8PJSDWFcazCy/oKHXzYAPIeei9wS0UvqqhYJO3GEmv5SGLBTFMiuF2xiQGJ7Cx9g5h7Ac+7efPmhkM+Pn1ncJDNXJJ4gzwxkTTt/Rh2/yr2lJ1Yg5WLykiX4LIaE1Y3CJe2j/ZKSLLJi6SjHRQYVKVnTNWw== 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, Dec 14, 2023 at 10:54:19AM +0000, Ryan Roberts wrote: > On 13/12/2023 07:21, Dan Carpenter wrote: > > On Thu, Dec 07, 2023 at 04:12:05PM +0000, Ryan Roberts wrote: > >> @@ -4176,10 +4260,15 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf) > >> /* Allocate our own private page. */ > >> if (unlikely(anon_vma_prepare(vma))) > >> goto oom; > >> - folio = vma_alloc_zeroed_movable_folio(vma, vmf->address); > >> + folio = alloc_anon_folio(vmf); > >> + if (IS_ERR(folio)) > >> + return 0; > >> if (!folio) > >> goto oom; > > > > Returning zero is weird. I think it should be a vm_fault_t code. > > It's the same pattern that the existing code a little further down this function > already implements: > > vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, addr, &vmf->ptl); > if (!vmf->pte) > goto release; > > If we fail to map/lock the pte (due to a race), then we return 0 to allow user > space to rerun the faulting instruction and cause the fault to happen again. The > above code ends up calling "return ret;" and ret is 0. > Ah, okay. Thanks! > > > > This mixing of error pointers and NULL is going to cause problems. > > Normally when we have a mix of error pointers and NULL then the NULL is > > not an error but instead means that the feature has been deliberately > > turned off. I'm unable to figure out what the meaning is here. > > There are 3 conditions that the function can return: > > - folio successfully allocated > - folio failed to be allocated due to OOM > - fault needs to be tried again due to losing race > > Previously only the first 2 conditions were possible and they were indicated by > NULL/not-NULL. The new 3rd condition is only possible when THP is compile-time > enabled. So it keeps the logic simpler to keep the NULL/not-NULL distinction for > the first 2, and use the error code for the final one. > > There are IS_ERR() and IS_ERR_OR_NULL() variants so I assume a pattern where you > can have pointer, error or NULL is somewhat common already? People are confused by this a lot so I have written a blog about it: https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/ The IS_ERR_OR_NULL() function should be used like this: int blink_leds() { led = get_leds(); if (IS_ERR_OR_NULL(led)) return PTR_ERR(led); <-- NULL means zero/success return led->blink(); } In the case of alloc_anon_folio(), I would be tempted to create a wrapper around it where NULL becomes ERR_PTR(-ENOMEM). But this is obviously fast path code and I haven't benchmarked it. Adding a comment is the other option. regards, dan carpenter