Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751853AbbEGCgT (ORCPT ); Wed, 6 May 2015 22:36:19 -0400 Received: from mail-wi0-f181.google.com ([209.85.212.181]:37639 "EHLO mail-wi0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751483AbbEGCgP (ORCPT ); Wed, 6 May 2015 22:36:15 -0400 MIME-Version: 1.0 In-Reply-To: References: <20150506200219.40425.74411.stgit@dwillia2-desk3.amr.corp.intel.com> Date: Wed, 6 May 2015 19:36:13 -0700 Message-ID: Subject: Re: [PATCH v2 00/10] evacuate struct page from the block layer, introduce __pfn_t From: Dan Williams To: Linus Torvalds Cc: Linux Kernel Mailing List , Boaz Harrosh , Jan Kara , Mike Snitzer , Neil Brown , Benjamin Herrenschmidt , Dave Hansen , Heiko Carstens , Chris Mason , Paul Mackerras , "H. Peter Anvin" , Christoph Hellwig , Alasdair Kergon , "linux-nvdimm@lists.01.org" , Ingo Molnar , Mel Gorman , Matthew Wilcox , Ross Zwisler , Rik van Riel , Martin Schwidefsky , Jens Axboe , "Theodore Ts'o" , "Martin K. Petersen" , Julia Lawall , Tejun Heo , linux-fsdevel , Andrew Morton Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4052 Lines: 77 On Wed, May 6, 2015 at 5:19 PM, Linus Torvalds wrote: > On Wed, May 6, 2015 at 4:47 PM, Dan Williams wrote: >> >> Conceptually better, but certainly more difficult to audit if the fake >> struct page is initialized in a subtle way that breaks when/if it >> leaks to some unwitting context. > > Maybe. It could go either way, though. In particular, with the > "dynamically allocated struct page" approach, if somebody uses it past > the supposed lifetime of the use, things like poisoning the temporary > "struct page" could be fairly effective. You can't really poison the > pfn - it's just a number, and if somebody uses it later than you think > (and you have re-used that physical memory for something else), you'll > never ever know. True, but there's little need to poison a _pfn_t because it's permanent once discovered via ->direct_access() on the hosting struct block_device. Sure, kmap_atomic_pfn_t() may fail when the pmem driver unbinds from a device, but the __pfn_t is still valid. Obviously, we can only support atomic kmap(s) with this property, and it would be nice to fault if someone continued to use the __pfn_t after the hosting device was disabled. To be clear, DAX has this same problem today. Nothing stops whomever called ->direct_access() to continue using the pfn after the backing device has been disabled. > I'd *assume* that most users of the dynamic "struct page" allocation > have very clear lifetime rules. Those things would presumably normally > get looked-up by some extended version of "get_user_pages()", and > there's a clear use of the result, with no longer lifetime. Also, you > do need to have some higher-level locking when you do this, to make > sure that the persistent pages don't magically get re-assigned. We're > presumably talking about having a filesystem in that persistent > memory, so we cannot be doing IO to the pages (from some other source > - whether RDMA or some special zero-copy model) while the underlying > filesystem is reassigning the storage because somebody deleted the > file. > > IOW, there had better be other external rules about when - and how > long - you can use a particular persistent page. No? So the whole > "when/how to allocate the temporary 'struct page'" is just another > detail in that whole thing. > > And yes, some uses may not ever actually see that. If the whole of > persistent memory is just assigned to a database or something, and the > DB just wants to do a "flush this range of persistent memory to > long-term disk storage", then there may not be much of a "lifetime" > issue for the persistent memory. But even then you're going to have IO > completion callbacks etc to let the DB know that it has hit the disk, > so.. > > What is the primary thing that is driving this need? Do we have a very > concrete example? My pet concrete example is covered by __pfn_t. Referencing persistent memory in an md/dm hierarchical storage configuration. Setting aside the thrash to get existing block users to do "bvec_set_page(page)" instead of "bvec->page = page" the onus is on that md/dm implementation and backing storage device driver to operate on __pfn_t. That use case is simple because there is no use of page locking or refcounting in that path, just dma_map_page() and kmap_atomic(). The more difficult use case is precisely what Al picked up on, O_DIRECT and RDMA. This patchset does nothing to address those use cases outside of not needing a struct page when they eventually craft a bio. I know Matthew Wilcox has explored the idea of "get_user_sg()" and let the scatterlist hold the reference count and locks, but I'll let him speak to that. I still see __pfn_t as generally useful for the simple in-kernel stacked-block-i/o use case. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/