From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 2 Jan 2018 14:41:37 -0800 From: Matthew Wilcox To: "Darrick J. Wong" Cc: "Kirill A. Shutemov" , linux-kernel@vger.kernel.org, Matthew Wilcox , Ross Zwisler , David Howells , Shaohua Li , Jens Axboe , Rehas Sachdeva , Marc Zyngier , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-nilfs@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-xfs@vger.kernel.org, linux-usb@vger.kernel.org, linux-raid@vger.kernel.org Subject: Re: [PATCH v5 03/78] xarray: Add the xa_lock to the radix_tree_root Message-ID: <20180102224137.GC20405@bombadil.infradead.org> References: <20171215220450.7899-1-willy@infradead.org> <20171215220450.7899-4-willy@infradead.org> <20171226165440.tv6inwa2fgk3bfy6@node.shutemov.name> <20171227034340.GC24828@bombadil.infradead.org> <20171227035815.GD24828@bombadil.infradead.org> <20180102180155.GD4857@magnolia> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180102180155.GD4857@magnolia> Sender: owner-linux-mm@kvack.org List-ID: On Tue, Jan 02, 2018 at 10:01:55AM -0800, Darrick J. Wong wrote: > On Tue, Dec 26, 2017 at 07:58:15PM -0800, Matthew Wilcox wrote: > > spin_lock_irqsave(&mapping->pages, flags); > > __delete_from_page_cache(page, NULL); > > spin_unlock_irqrestore(&mapping->pages, flags); > > > > More details here: https://9p.io/sys/doc/compiler.html > > I read the link, and I understand (from section 3.3) that replacing > foo.bar.baz.goo with foo.goo is less typing, but otoh the first time I > read your example above I thought "we're passing (an array of pages | > something that doesn't have the word 'lock' in the name) to > spin_lock_irqsave? wtf?" I can see that being a bit jarring initially. If you think about what object-oriented languages were offering in the nineties, this is basically C++ multiple-inheritance / Java interfaces. So when I read the above example, I think "lock the mapping pages, delete from page cache, unlock the mapping pages", and I don't have a wtf moment. It's just simpler to read than "lock the mapping pages lock", and less redundant. > I suppose it does force me to go dig into whatever mapping->pages is to > figure out that there's an unnamed spinlock_t and that the compiler can > insert the appropriate pointer arithmetic, but now my brain trips over > 'pages' being at the end of the selector for parameter 1 which slows > down my review reading... > > OTOH I guess it /did/ motivate me to click the link, so well played, > sir. :) Now if only I can trick you into giving your ACK on patch 1, "xfs: Rename xa_ elements to ail_" -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org