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 1E30DC48BC4 for ; Fri, 16 Feb 2024 15:58:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 946906B009C; Fri, 16 Feb 2024 10:58:02 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8F7B86B009D; Fri, 16 Feb 2024 10:58:02 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7BEAD6B009F; Fri, 16 Feb 2024 10:58:02 -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 6D63B6B009C for ; Fri, 16 Feb 2024 10:58:02 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 17BC2A0540 for ; Fri, 16 Feb 2024 15:58:02 +0000 (UTC) X-FDA: 81798123204.05.8D572CD Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf14.hostedemail.com (Postfix) with ESMTP id 5233D10001A for ; Fri, 16 Feb 2024 15:57:57 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b="C3dk/DW6"; dmarc=none; spf=none (imf14.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708099080; 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=zCyJDd9dKPDmdypQXGurJ7KaNpNOw4+5thN7/7/eyiU=; b=8cVDq7ni9Wiv+gj+ORO4I/G89292xz7LqH/YeQszLnp+cMCDcK7Gp52zXh4wef4tj9Tbns GKiLn7Ba1BaztEQCVAsmJGGgE3KDzbalp8ImRhY9YUgLDIJITe5YnrxmzSZSlRiBOjTBE9 b6wocj/kUmAxjCmFin5So7t0xRJKv/A= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b="C3dk/DW6"; dmarc=none; spf=none (imf14.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708099080; a=rsa-sha256; cv=none; b=fr1eREmpKMlXZQmbePmlI0EDVpB9KMlNEyp6DnMdvmi2D42J+zPxQ8p8WuorXigkLz6Q32 QNLggA/9jXLzdOnZchhEEOy4dvLBPR61U0hrOXOSYq8l/IPOpYE7gM98FFtqf3uJJTDIT6 rf7bO8+6aryLJGYxPTd4GwU/hU80L5o= 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=zCyJDd9dKPDmdypQXGurJ7KaNpNOw4+5thN7/7/eyiU=; b=C3dk/DW6uAoWvTuhYne1bfRsVK AFRFMv8cN14x0aHBmtqjvahsWzQK+FPnbZDYVHperY/+KHBRo1XhXb567u60YCv08qQ8J+MVeSGQ9 1x119URCwWtBQXYV5u3Fp+ktf+Afjf9+nsUaJK0ZIpmn+63y/Im5SUjp7ZE7tKmRq/ZslvmgixOt2 n7hihfuNkMxshd73kzO/yEJcUSTS7FrMe7ixIAu8TEU5eWUaZzyK7H2DC5OzqwyWkPuio4mo83+iB tbNOT59j9CjoFtIoRSKPUX3mgyxqdcuioLt3X5Ufel4z7HraKIzr9R/PBYNEpjMM3ypzTo9Y3duO1 qGjm3Vcw==; Received: from willy by casper.infradead.org with local (Exim 4.97.1 #2 (Red Hat Linux)) id 1rb0as-000000053rW-287p; Fri, 16 Feb 2024 15:57:42 +0000 Date: Fri, 16 Feb 2024 15:57:42 +0000 From: Matthew Wilcox To: Jan Kara Cc: "Liam R. Howlett" , Chuck Lever , viro@zeniv.linux.org.uk, brauner@kernel.org, hughd@google.com, akpm@linux-foundation.org, oliver.sang@intel.com, feng.tang@intel.com, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, maple-tree@lists.infradead.org, linux-mm@kvack.org, lkp@intel.com Subject: Re: [PATCH RFC 7/7] libfs: Re-arrange locking in offset_iterate_dir() Message-ID: References: <170785993027.11135.8830043889278631735.stgit@91.116.238.104.host.secureserver.net> <170786028847.11135.14775608389430603086.stgit@91.116.238.104.host.secureserver.net> <20240215131638.cxipaxanhidb3pev@quack3> <20240215170008.22eisfyzumn5pw3f@revolver> <20240215171622.gsbjbjz6vau3emkh@quack3> <20240215210742.grjwdqdypvgrpwih@revolver> <20240216101546.xjcpzyb3pgf2eqm4@quack3> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240216101546.xjcpzyb3pgf2eqm4@quack3> X-Rspamd-Queue-Id: 5233D10001A X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: kyg88gcbxd51n6ys69su8hf9qnbi8p9u X-HE-Tag: 1708099077-377759 X-HE-Meta: U2FsdGVkX1/zbt8dvm4Qvr01M445gFp6gfVz7f++QUpYtp2XJxqkKxlAeM0guRvoTslewnPnl5XtA4+gT59BuzZmShSTSobRhOAET0aes60jPs+5DSyKRJp7Q4XUsukL6J2zZrPteA4yKxgfE/Uisio+gEeQJ9qXZxijOnINrhyJu7DUIzIHf1t5l0mcc3vWbIo2nuqAiDaHUJNruelj/Tc6F7tcnzeWOsMc4fA1PbtFOSw29D+p33WI7xakx/hQGR0bYzcNHYSCP22K9TjPCthrKp5R8HBjB90PzDsSuN12jYpcB0LTicmqHSp4vnJBLZ4ji798FN8U6vo9A3KSzGpSzkCc4ITSILY0piN/trf7vjL1seKD7T9tb0a1WuZAa+fgMYjL/nff2HHTYTZTj1/byHOfvPtj3VA6UDwwCkgoo2toefWmF32oNT7y12Wu1u9B8PV2Dgvre3BkYTiMyhc5UBH6G7Grn0yBTEkFqfkAv87tUwSg9mjp4fA1RL4os9M8eFEqM+8KnTjFRO2l8CLh0ictKYFF5hM0mIofVxNCA7ewbil2E/d9fyEplJneD4LtFDrsZ+5IynSbMAiymQ/1kaB+g9jtYqQqUb+JaevWtpxshLgFiUMCJBMP9goN7ePtGn56IkRT37nq0/jHqjRmt/HhvV0XARWKFD2++v2GuwMXD/ngfzLeWxnDnHIO29We8zu5eoTCKeT8dyRySBSgmmXn3blg/zmCj77v6UWXmoxK8XeQFpmptV2Hn/ibLJxMY5m6bzi5ytozDHledLRIdozUUBn47pS4wnZSjEbQPvWAhyshoK3/ccbcXzHwU/XVjNOwN1g3tO1EjxOYrRC6Em2USHakSqvMnE89CXGmDIolYHXQ1++55k6nf7m/vt8ono1Z5G9BrwRgaM7Wydr7oZbZELvvMlOZsPH77H4xVFce6Tjado3xJH/Dt1Nioc2zl2CoxPqV5HnMDq1 jIcSiUjV DBUJOcsfEkQU7IfWZLP4NUqmIJwdHZGAcTPAmnEugC5XCWTYk4MGLiE+rVwU1fM9YMqQMuxCX9QI/KGUqABjmk/UaT0GPo4PnJrMO1UMfn6a0lt67X9V6vq+68Sh/BSVkbm5h8Ug1G582uwO0yI6ei66t+uQOKWbDRf4gOqj3Y3/JL1buT//AH5kpvDPuuUrmILvPmTtmM0XXT+S41TKEQ+Vl43tsyq3bk8Ac5NO83esCIkpECSmuoPdoKRYnS7DJPwkb/RalZSz+s3c62Y7yqRYXK7r4JpLJoYkj13sPmlckZI+jpjWqfe662+PF83n9RHvfTFL+h8+ttEOrUI/0+ruak0QhbNNO5n0DSV2aRioBwZA= 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 Fri, Feb 16, 2024 at 11:15:46AM +0100, Jan Kara wrote: > On Thu 15-02-24 16:07:42, Liam R. Howlett wrote: > > The limitations are outlined in the documentation as to how and when to > > lock. I'm not familiar with the xarray users, but it does check for > > locking with lockdep, but the way this is written bypasses the lockdep > > checking as the locks are taken and dropped without the proper scope. > > > > If you feel like this is a trap, then maybe we need to figure out a new > > plan to detect incorrect use? > > OK, I was a bit imprecise. What I wanted to say is that this is a shift in > the paradigm in the sense that previously, we mostly had (and still have) > data structure APIs (lists, rb-trees, radix-tree, now xarray) that were > guaranteeing that unless you call into the function to mutate the data > structure it stays intact. hm, no. The radix tree never guaranteed that to you; I just documented that it wasn't guaranteed for the XArray. > Now maple trees are shifting more in a direction > of black-box API where you cannot assume what happens inside. Which is fine > but then we have e.g. these iterators which do not quite follow this > black-box design and you have to remember subtle details like calling > "mas_pause()" before unlocking which is IMHO error-prone. Ideally, users of > the black-box API shouldn't be exposed to the details of the internal > locking at all (but then the performance suffers so I understand why you do > things this way). Second to this ideal variant would be if we could detect > we unlocked the lock without calling xas_pause() and warn on that. Or maybe > xas_unlock*() should be calling xas_pause() automagically and we'd have > similar helpers for RCU to do the magic for you? If you're unlocking the lock that protects a data structure while still using that data structure, you should always be aware that you're doing something very dangerous! It's no different from calling inode_unlock() inside a filesystem. Sure, you can do it, but you'd better be ready to deal with the consequences. The question is, do we want to be able to defragment slabs or not? My thinking is "yes", for objects where we can ensure there are no current users (at least after an RCU grace period), we want to be able to move them. That does impose certain costs (and subtleties), but just like fast-GUP and lockless page-cache, I think it's worth doing. Of course, we don't have slab defragmentation yet, so we're not getting any benefit from this. The most recent attempt was in 2019: https://lore.kernel.org/linux-mm/20190603042637.2018-1-tobin@kernel.org/ but there were earlier attepts in 2017: https://lore.kernel.org/linux-mm/20171227220636.361857279@linux.com/ and 2008: https://lore.kernel.org/linux-mm/20080216004526.763643520@sgi.com/ so I have to believe it's something we want, just haven't been able to push across the "merge this now" line.