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 2886CC47258 for ; Thu, 25 Jan 2024 20:22:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B11976B0095; Thu, 25 Jan 2024 15:22:41 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id AC16F8D0002; Thu, 25 Jan 2024 15:22:41 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9894A6B0098; Thu, 25 Jan 2024 15:22:41 -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 88DE16B0095 for ; Thu, 25 Jan 2024 15:22:41 -0500 (EST) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 6A9751A0E38 for ; Thu, 25 Jan 2024 20:22:41 +0000 (UTC) X-FDA: 81718956522.18.999C22D Received: from mail-ej1-f51.google.com (mail-ej1-f51.google.com [209.85.218.51]) by imf08.hostedemail.com (Postfix) with ESMTP id 42B42160022 for ; Thu, 25 Jan 2024 20:22:39 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=ffwll.ch header.s=google header.b=ddHc5csb; spf=none (imf08.hostedemail.com: domain of daniel@ffwll.ch has no SPF policy when checking 209.85.218.51) smtp.mailfrom=daniel@ffwll.ch; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706214159; 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=cgx7mklBpB9KounVQy5eGpWd3i905pv75d99Q4ggPTM=; b=Gl7/1oZeucGYWL6L+BVPMi/CyZtO9f6jC4vwsCF0T1dUPX4I7LkloobDhS3OQQCK4qXt3A JeEAsJrK987v0kQ0880/DAvhnWa29KFvky8ZtvXXmYnwwSk2n7qZH2K4mJA0AeUqXbXHJt sKMbn/hIAxNUl1d5jmnb4xPMQdSL6WA= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=ffwll.ch header.s=google header.b=ddHc5csb; spf=none (imf08.hostedemail.com: domain of daniel@ffwll.ch has no SPF policy when checking 209.85.218.51) smtp.mailfrom=daniel@ffwll.ch; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706214159; a=rsa-sha256; cv=none; b=TdlHRlqwnLxXhaYDQ6dY4NOF3GTvuf+UOtyxcC2KOOLxlV1IpzfgdaDu72rcFxItKagI5c Y7MYGgZsX+vlLwZmUJNbLP7KnqUJh4gYMvnO7fsyHVOIz7lIK0ip30nNOQidxQNkpcxEY+ g2NXNpM20VXNeAXQZyJB9oaFo1X3/ZE= Received: by mail-ej1-f51.google.com with SMTP id a640c23a62f3a-a28cfca3c45so168730766b.1 for ; Thu, 25 Jan 2024 12:22:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; t=1706214157; x=1706818957; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc :subject:date:message-id:reply-to; bh=cgx7mklBpB9KounVQy5eGpWd3i905pv75d99Q4ggPTM=; b=ddHc5csbJOGzdVWBeBHejol/elCaxe7fSMwleWHgCtavdEq7RdwY1NrmBtwzCeO+Bs Mqp1gp9R4cy9uRPfrxVkYaXFc0UxsBNHbmfGy+FzhP29LMhPIvaUHE7+d0sds6QxxOh9 nTnhoPWM2wuCIcF4f2Bedhx1jWIf0OU4uzVC8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706214157; x=1706818957; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=cgx7mklBpB9KounVQy5eGpWd3i905pv75d99Q4ggPTM=; b=AqZTdGS0v900S+o4aTro7QIPa7D1HGGe+Ohs5Yk+KML35pbUBH5Gq2iASyiFAX4f8A i4mZXCz8Q8ELXc7TgWCIxJ41MWpuSK7EaLU1/lb+fKdNHOEp/tQigcmZpBfrmL3hSN8g 94nh19SHTsTWmC5oiULz1mKPXtNmS2BGlZEgFlILTFtrA3kNUGTme6H+ZzbAv48kYUw5 T90QNU/Td/bYyAwIQ+5u13YT681PpcbqSE3g1dzTWpHUqa5b5mAN9D4FSqQmRv7FpRca PJMU9sa5O/wA7dR9siPDC35H71SGpYmKVK54FaOTs5MRl/yNMWPz0PHipAy5ZcNJAVNJ 0akg== X-Forwarded-Encrypted: i=0; AJvYcCUayDzlaoKHTf8l9MlsQ77gwPyHizw/rq5+Z7TS/eSNLTuvM0/qz/tnK94uJGbo1mnafpVkFGxS7k5yUn4fcYMHVSM= X-Gm-Message-State: AOJu0YwQ8ZB59EDvwGbTZ34T9p58Wd3yW3I45FnKucTqQjhX2gIsJSZ4 +vzB5PT0syZcJMXMhktFkiiwY4hegR5DyqEV0a+rHNR/KZL3cwahYQn9NBvMpKw= X-Google-Smtp-Source: AGHT+IFaTRwxaZ3kossB3H1YRQdr9l0tTF1yAxPBdIIOdK1uRobh4zyGMgDW9fgGjUXK1GGr2MpHAQ== X-Received: by 2002:a17:906:f804:b0:a30:ff7f:e583 with SMTP id kh4-20020a170906f80400b00a30ff7fe583mr148157ejb.2.1706214157490; Thu, 25 Jan 2024 12:22:37 -0800 (PST) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id ps12-20020a170906bf4c00b00a31225fed97sm1372278ejb.104.2024.01.25.12.22.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Jan 2024 12:22:36 -0800 (PST) Date: Thu, 25 Jan 2024 21:22:34 +0100 From: Daniel Vetter To: Matthew Wilcox Cc: Helge Deller , Thomas Zimmermann , Jaya Kumar , Daniel Vetter , linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-mm@kvack.org Subject: Re: fb_defio and page->mapping Message-ID: Mail-Followup-To: Matthew Wilcox , Helge Deller , Thomas Zimmermann , Jaya Kumar , linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-mm@kvack.org References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux phenom 6.6.11-amd64 X-Rspamd-Queue-Id: 42B42160022 X-Rspam-User: X-Stat-Signature: 8cdih8g8my9zcb75xmngaoao5s73ma4f X-Rspamd-Server: rspam01 X-HE-Tag: 1706214158-59365 X-HE-Meta: U2FsdGVkX1+Q0L7GGDsw4aXiFaIyZR10fvmBIwFEL4P9HcJSM/TZKm/pv3ipHg34y+ajp536+QaVWqky6QR7LgewvJRETzwkKH9CuEpIJ/S0Y+gpZ04Jt9SYr+KbAe9JSvmef+pdTag4HDWc7fxVi9XxkpqBvMpOZHpWMdlEsi/LHdcW0HEnND9Lw6THp96zcn6DGd2CnP0RcCd5bp110iXKb18Lu4DVcu24xO3bTVpBAzLP5NOa5eod01na63PBqpsEIA9MVc947Nk0NJyHtp4+GclhwAqBmYui7mUUBj5ohj1AsriQeAX23CW1E6ubrNk/XVIV99u8TEOgqTpiUx3Nyo2U2K+WlVua7Emsj56VM4xLDP9SQ7GEH7C8G79YjKNascyvnHWiWybeUhWO0WFkz78kosMJj3VugfSKzRrCU1C9aRURZtQ7YRtH/I0JutU8vp+vcf3ZHkB1vW5t5EuKhVGq8I3C5QClSZfZWxCPkQ6oIt2dewkfdZnIZv+YreTkIpCrYVrI9Cvdk9yhU7gnoqHQl0NEwmZuFPRVMsXAf/TCm0579eEiLYAfi7r9PdQ75Z6nTAN4S6jGUEgklKCgPEDjFSn+gO3hpAu3xy9JX7FVLBPL5aawZH42ZzYzRoMheLLmvok/pTBlUZ4j+Sy9fVjVNJOapuUpmu5LYlqyfZ2+1uff+Pp4R0q8Lr1LlPhP6xECIQOtqlpReDhgXxoTS1Y+WMIvM0bC79j5aZCsskJuG0vEt6MsYgOckWWIn5eK2xrifTNW4nLBuJWsKof259NKbGdlGdJuiyYor/sKY6+J5pVccvg82nr/kC/aNSgC36aqA8HVBDEPKI8x16EgLyIyEaT/6egmr6KEE7NhoDwX6xXbMgG/msRPVU5I9wmQXMDuVa85cmqnRcsViO8qeEebuY6S/RqRGoiBttAvzjcXROjJDioeN4zo8rHYnyo5slbtwi9poASvGh3 mLnwpXuL JCSpEftOlCLL6C+NorMRDQaTiMIfKXhmXXbJ68NfnNBX8wKUw1KDiw68um/IuWh7HCtYNycO2tjhNt4K7Cu5Z4UdeZFCw8yQarq+xZcruq46R4a1ZOn3tUwTUSh/0/AFLkVeprDkCahZwdniLWSk9LKk8GWGXrROCmog+4tUwBx8YhTtkjgKeB3sXEPCG8QyjZnmbOkalB+4OIOolvnK1Jb0s2lrSwqodSwYxuq0F2aFH21N5S6bc1WCpoVci8oMMp5khrLbRENbLo/X92KNGgJ2MsJD5Yi6b83Q/TM99wJG9Q7iTdkwo7NVkmAQ3qqtTTkQ24FRBFlZiCEukcfZdfOnGYPL8GiPLXHXRfzwmnaoDhsaZbGcoveS6+1kJ+lhKumHS X-Bogosity: Ham, tests=bogofilter, spamicity=0.000041, 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 Tue, Jan 23, 2024 at 05:20:55PM +0000, Matthew Wilcox wrote: > We're currently trying to remove page->mapping from the entire kernel. > This has me interested in fb_defio and since I made such a mess of it > with commits ccf953d8f3d6 / 0b78f8bcf495, I'd like to discuss what to > do before diving in. > > Folios continue to have a mapping. So we can effectively do > page_folio(page)->mapping (today that's calling compound_head() to get > to the head page; eventually it's a separate allocation). > > But now I look at commit 56c134f7f1b5, I'm a little scared. > Apparently pages are being allocated from shmem and being mapped by > fb_deferred_io_fault()? This line: > > page->mapping = vmf->vma->vm_file->f_mapping; > > initially appears harmless for shmem files (because that's presumably > a noop), but it's only a noop for head pages. If shmem allocates a > compound page (ie a 2MB THP today), you'll overlay some information > stored in the second and third pages; looks like _entire_mapcount > and _deferred_list.prev (but we do shift those fields around without > regard to what the fb_defio driver is doing). Even if you've disabled > THP today, setting page->mapping to NULL in fb_deferred_io_lastclose() > for a shmem page is a really bad idea. > > I'd like to avoid fb_defio playing with page->mapping at all. > As I understand it, the only reason to set page->mapping is so that > page_mkclean() works. But there are all kinds of assumptions in > page_mkclean() (now essentially folio_mkclean()) that we're dealing with > file-backed or anonymous memory. I wonder if we might be better off > calling pfn_mkclean_range() for each VMA which maps these allocations? > You'd have to keep track of each VMA yourself (I think?) but it would > avoid messing with page->mapping. > > Anyway, I don't know enough about DRM. There might be something > unutterably obvious we could do to fix this. It's just really old code that's been barely touched to keep it going. The issue is that the entire defio stuff is pretty bad layering violation. Not sure what the cleanest way to do this really would be if it only touches the ptes and nothing else. Not sure what's the right function for a bit of pte walking for that. That would still potentially mess with the mapping by the gpu memory allocator in bad ways, but I think at least for all current ones it should avoid problems. Definitely agree that messing with struct page in any way is really bad, we simply didn't get that far yet. I think the cleanest way would be if we have a fb_mmap only for drm drivers in drm_fbdev_generic.c and sunset fb_deferred_io_mmap and use that to just replicate the ptes from the kernel's vmap into one that is ok for userspace. The fbdev implementation in drm_fbdev_generic.c is the only one left in drm that supports fb_defio, so that would catch all of them. To my knowledge all the other defio implementations in native fbdev drivers aren't problematic since none use shmem. For I while we pondered with proxying the vma to the driver's drm native mmap implementation, but that gets real messy plus there's no benefit because fbdev assumes a bit too much that the memory is permanently pinned. So we need the pinned kernel vmap anyway. Cheers, Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch