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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0663AC531DC for ; Thu, 17 Aug 2023 19:03:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1354643AbjHQTCa (ORCPT ); Thu, 17 Aug 2023 15:02:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41530 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1354639AbjHQTCB (ORCPT ); Thu, 17 Aug 2023 15:02:01 -0400 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E74852D59 for ; Thu, 17 Aug 2023 12:01:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=RJpdT2TuLX3rRzXEHlG8PsweP6fxFNpA/3TiJp72Sck=; b=EyNymHEPUnKYceBO6ggRNe0dHk SRAjEmv1T0PLwVNYmgMS60Wyu2dVgl5IAQAkhoP0RF4cXv+yk3KGFvARoXCaJG9rxr2gDazAolFC5 T1USF2yfPy2/5CX9R2MUeBYgYcyfM1dqeZkcryawIY5HdFurvfMWp5z0EBR8PzXBAhAYLE0Nd1ETz 0bTUzNCOxa/Kc+rWXVkvoJVPZ7V3kboycx+ZfU/xAwQTKXlYpv6e+1nywDB/8ozyIfVaJ7AwNd7xe Yqe7Kv/csfiF8A1hf/c2s79i0npK20cbzmgxPaQLZrxWpSUF+EokHACH8uhH05wJ3zVxGS6PQeOf/ cucPPK4w==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1qWiFg-004uCD-0q; Thu, 17 Aug 2023 19:01:48 +0000 Date: Thu, 17 Aug 2023 20:01:47 +0100 From: Matthew Wilcox To: Zach O'Keefe Cc: Saurabh Singh Sengar , Dan Williams , "linux-mm@kvack.org" , Yang Shi , "linux-kernel@vger.kernel.org" Subject: Re: [EXTERNAL] [PATCH] mm/thp: fix "mm: thp: kill __transhuge_page_enabled()" Message-ID: References: <20230812210053.2325091-1-zokeefe@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Aug 17, 2023 at 11:13:36AM -0700, Zach O'Keefe wrote: > > > IIUC then, there is a bug in smaps THPeligible code when > > > CONFIG_READ_ONLY_THP_FOR_FS is not set. Not obvious, but apparently > > > this config is (according to it's Kconfig desc) khugepaged-only, so it > > > should be fine for it to be disabled, yet allow > > > do_sync_mmap_readahead() to install a pmd for file-backed memory. > > > hugepage_vma_check() will need to be patched to fix this. > > > > I guess so ... > > The easiest and most satisfying way to handle this -- and I think we > talked about this before -- is relaxing that complicated > file_thp_enabled() check when the file's mapping supports large > folios. I think that makes sense to me, though I don't know all the > details fs-side. Will we need any hook to give fs the chance to update > any internal state on collapse? If the filesystem has per-folio metadata, we need to give the filesystem the chance to set that up. I've vaguaely been wondering about using the ->migrate_folio callback for it. At the moment, I think it just refuses to work if the folio isn't order-0. > > > But I have a larger question for you: should we care about > > > /sys/kernel/mm/transparent_hugepage/enabled for file-fault? We > > > currently don't. Seems weird that we can transparently get a hugepage > > > when THP="never". Also, if THP="always", we might as well skip the > > > VM_HUGEPAGE check, and try the final pmd install (and save khugepaged > > > the trouble of attempting it later). > > > > I deliberately ignored the humungous complexity of the THP options. > > They're overgrown and make my brain hurt. [..] > > Same > > > [..] Instead, large folios are > > adaptive; they observe the behaviour of the user program and choose based > > on history what to do. This is far superior to having a sysadmin tell > > us what to do! > > I had written a bunch on this, but I arrived to the conclusion that > (a) pmd-mapping here is ~ a free win, and (b) I'm not the best person > to argue for these knobs, given MADV_COLLAPSE ignores them entirely :P > > ..But (sorry) what about MMF_DISABLE_THP? Yeah, we ignore that too. My rationale is -- as you said -- using the PMDs is actually free, and it's really none of the app's business how the page cache chooses to cache things.