From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Darrick J. Wong" Subject: Re: [PATCH v5 03/78] xarray: Add the xa_lock to the radix_tree_root Date: Tue, 2 Jan 2018 10:01:55 -0800 Message-ID: <20180102180155.GD4857@magnolia> 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> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20171227035815.GD24828@bombadil.infradead.org> Sender: owner-linux-mm@kvack.org To: Matthew Wilcox 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 List-Id: linux-f2fs-devel.lists.sourceforge.net On Tue, Dec 26, 2017 at 07:58:15PM -0800, Matthew Wilcox wrote: > On Tue, Dec 26, 2017 at 07:43:40PM -0800, Matthew Wilcox wrote: > > Also add the xa_lock() and xa_unlock() family of wrappers to make it > > easier to use the lock. If we could rely on -fplan9-extensions in > > the compiler, we could avoid all of this syntactic sugar, but that > > wasn't added until gcc 4.6. > > Oh, in case anyone's wondering, here's how I'd do it with plan9 extensions: > > struct xarray { > spinlock_t; > int xa_flags; > void *xa_head; > }; > > ... > spin_lock_irqsave(&mapping->pages, flags); > __delete_from_page_cache(page, NULL); > spin_unlock_irqrestore(&mapping->pages, flags); > ... > > The plan9 extensions permit passing a pointer to a struct which has an > unnamed element to a function which is expecting a pointer to the type > of that element. The compiler does any necessary arithmetic to produce > a pointer. It's exactly as if I had written: > > spin_lock_irqsave(&mapping->pages.xa_lock, flags); > __delete_from_page_cache(page, NULL); > spin_unlock_irqrestore(&mapping->pages.xa_lock, 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 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. :) --D > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v5,03/78] xarray: Add the xa_lock to the radix_tree_root From: "Darrick J. Wong" Message-Id: <20180102180155.GD4857@magnolia> Date: Tue, 2 Jan 2018 10:01:55 -0800 To: Matthew Wilcox 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 List-ID: T24gVHVlLCBEZWMgMjYsIDIwMTcgYXQgMDc6NTg6MTVQTSAtMDgwMCwgTWF0dGhldyBXaWxjb3gg d3JvdGU6Cj4gT24gVHVlLCBEZWMgMjYsIDIwMTcgYXQgMDc6NDM6NDBQTSAtMDgwMCwgTWF0dGhl dyBXaWxjb3ggd3JvdGU6Cj4gPiAgICAgQWxzbyBhZGQgdGhlIHhhX2xvY2soKSBhbmQgeGFfdW5s b2NrKCkgZmFtaWx5IG9mIHdyYXBwZXJzIHRvIG1ha2UgaXQKPiA+ICAgICBlYXNpZXIgdG8gdXNl IHRoZSBsb2NrLiAgSWYgd2UgY291bGQgcmVseSBvbiAtZnBsYW45LWV4dGVuc2lvbnMgaW4KPiA+ ICAgICB0aGUgY29tcGlsZXIsIHdlIGNvdWxkIGF2b2lkIGFsbCBvZiB0aGlzIHN5bnRhY3RpYyBz dWdhciwgYnV0IHRoYXQKPiA+ICAgICB3YXNuJ3QgYWRkZWQgdW50aWwgZ2NjIDQuNi4KPiAKPiBP aCwgaW4gY2FzZSBhbnlvbmUncyB3b25kZXJpbmcsIGhlcmUncyBob3cgSSdkIGRvIGl0IHdpdGgg cGxhbjkgZXh0ZW5zaW9uczoKPiAKPiBzdHJ1Y3QgeGFycmF5IHsKPiAgICAgICAgIHNwaW5sb2Nr X3Q7Cj4gICAgICAgICBpbnQgeGFfZmxhZ3M7Cj4gICAgICAgICB2b2lkICp4YV9oZWFkOwo+IH07 Cj4gCj4gLi4uCj4gICAgICAgICBzcGluX2xvY2tfaXJxc2F2ZSgmbWFwcGluZy0+cGFnZXMsIGZs YWdzKTsKPiAgICAgICAgIF9fZGVsZXRlX2Zyb21fcGFnZV9jYWNoZShwYWdlLCBOVUxMKTsKPiAg ICAgICAgIHNwaW5fdW5sb2NrX2lycXJlc3RvcmUoJm1hcHBpbmctPnBhZ2VzLCBmbGFncyk7Cj4g Li4uCj4gCj4gVGhlIHBsYW45IGV4dGVuc2lvbnMgcGVybWl0IHBhc3NpbmcgYSBwb2ludGVyIHRv IGEgc3RydWN0IHdoaWNoIGhhcyBhbgo+IHVubmFtZWQgZWxlbWVudCB0byBhIGZ1bmN0aW9uIHdo aWNoIGlzIGV4cGVjdGluZyBhIHBvaW50ZXIgdG8gdGhlIHR5cGUKPiBvZiB0aGF0IGVsZW1lbnQu ICBUaGUgY29tcGlsZXIgZG9lcyBhbnkgbmVjZXNzYXJ5IGFyaXRobWV0aWMgdG8gcHJvZHVjZSAK PiBhIHBvaW50ZXIuICBJdCdzIGV4YWN0bHkgYXMgaWYgSSBoYWQgd3JpdHRlbjoKPiAKPiAgICAg ICAgIHNwaW5fbG9ja19pcnFzYXZlKCZtYXBwaW5nLT5wYWdlcy54YV9sb2NrLCBmbGFncyk7Cj4g ICAgICAgICBfX2RlbGV0ZV9mcm9tX3BhZ2VfY2FjaGUocGFnZSwgTlVMTCk7Cj4gICAgICAgICBz cGluX3VubG9ja19pcnFyZXN0b3JlKCZtYXBwaW5nLT5wYWdlcy54YV9sb2NrLCBmbGFncyk7Cj4g Cj4gTW9yZSBkZXRhaWxzIGhlcmU6IGh0dHBzOi8vOXAuaW8vc3lzL2RvYy9jb21waWxlci5odG1s CgpJIHJlYWQgdGhlIGxpbmssIGFuZCBJIHVuZGVyc3RhbmQgKGZyb20gc2VjdGlvbiAzLjMpIHRo YXQgcmVwbGFjaW5nCmZvby5iYXIuYmF6LmdvbyB3aXRoIGZvby5nb28gaXMgbGVzcyB0eXBpbmcs IGJ1dCBvdG9oIHRoZSBmaXJzdCB0aW1lIEkKcmVhZCB5b3VyIGV4YW1wbGUgYWJvdmUgSSB0aG91 Z2h0ICJ3ZSdyZSBwYXNzaW5nIChhbiBhcnJheSBvZiBwYWdlcyB8CnNvbWV0aGluZyB0aGF0IGRv ZXNuJ3QgaGF2ZSB0aGUgd29yZCAnbG9jaycgaW4gdGhlIG5hbWUpIHRvCnNwaW5fbG9ja19pcnFz YXZlPyB3dGY/IgoKSSBzdXBwb3NlIGl0IGRvZXMgZm9yY2UgbWUgdG8gZ28gZGlnIGludG8gd2hh dGV2ZXIgbWFwcGluZy0+cGFnZXMgaXMgdG8KZmlndXJlIG91dCB0aGF0IHRoZXJlJ3MgYW4gdW5u YW1lZCBzcGlubG9ja190IGFuZCB0aGF0IHRoZSBjb21waWxlciBjYW4KaW5zZXJ0IHRoZSBhcHBy b3ByaWF0ZSBwb2ludGVyIGFyaXRobWV0aWMsIGJ1dCBub3cgbXkgYnJhaW4gdHJpcHMgb3Zlcgon cGFnZXMnIGJlaW5nIGF0IHRoZSBlbmQgb2YgdGhlIHNlbGVjdG9yIGZvciBwYXJhbWV0ZXIgMSB3 aGljaCBzbG93cwpkb3duIG15IHJldmlldyByZWFkaW5nLi4uCgpPVE9IIEkgZ3Vlc3MgaXQgL2Rp ZC8gbW90aXZhdGUgbWUgdG8gY2xpY2sgdGhlIGxpbmssIHNvIHdlbGwgcGxheWVkLApzaXIuIDop CgotLUQKCj4gLS0KPiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0aGUgbGlu ZSAidW5zdWJzY3JpYmUgbGludXgteGZzIiBpbgo+IHRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBt YWpvcmRvbW9Admdlci5rZXJuZWwub3JnCj4gTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cDov L3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sCi0tLQpUbyB1bnN1YnNjcmliZSBm cm9tIHRoaXMgbGlzdDogc2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtdXNiIiBpbgp0 aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZwpNb3JlIG1h am9yZG9tbyBpbmZvIGF0ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0 bWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from aserp2120.oracle.com ([141.146.126.78]:55338 "EHLO aserp2120.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751024AbeABSIn (ORCPT ); Tue, 2 Jan 2018 13:08:43 -0500 Date: Tue, 2 Jan 2018 10:01:55 -0800 From: "Darrick J. Wong" Subject: Re: [PATCH v5 03/78] xarray: Add the xa_lock to the radix_tree_root Message-ID: <20180102180155.GD4857@magnolia> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171227035815.GD24828@bombadil.infradead.org> Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Matthew Wilcox 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 On Tue, Dec 26, 2017 at 07:58:15PM -0800, Matthew Wilcox wrote: > On Tue, Dec 26, 2017 at 07:43:40PM -0800, Matthew Wilcox wrote: > > Also add the xa_lock() and xa_unlock() family of wrappers to make it > > easier to use the lock. If we could rely on -fplan9-extensions in > > the compiler, we could avoid all of this syntactic sugar, but that > > wasn't added until gcc 4.6. > > Oh, in case anyone's wondering, here's how I'd do it with plan9 extensions: > > struct xarray { > spinlock_t; > int xa_flags; > void *xa_head; > }; > > ... > spin_lock_irqsave(&mapping->pages, flags); > __delete_from_page_cache(page, NULL); > spin_unlock_irqrestore(&mapping->pages, flags); > ... > > The plan9 extensions permit passing a pointer to a struct which has an > unnamed element to a function which is expecting a pointer to the type > of that element. The compiler does any necessary arithmetic to produce > a pointer. It's exactly as if I had written: > > spin_lock_irqsave(&mapping->pages.xa_lock, flags); > __delete_from_page_cache(page, NULL); > spin_unlock_irqrestore(&mapping->pages.xa_lock, 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 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. :) --D > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html