Message-ID: <CAPcyv4ic-NY4KEtVknDQEdbF243aqdWdei2kTnn_tcw=7fUYyQ@mail.gmail.com>
Date:	Wed, 6 May 2015 19:36:13 -0700
From:	Dan Williams <dan.j.williams@...el.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Boaz Harrosh <boaz@...xistor.com>, Jan Kara <jack@...e.cz>,
	Mike Snitzer <snitzer@...hat.com>, Neil Brown <neilb@...e.de>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	Chris Mason <clm@...com>, Paul Mackerras <paulus@...ba.org>,
	"H. Peter Anvin" <hpa@...or.com>, Christoph Hellwig <hch@....de>,
	Alasdair Kergon <agk@...hat.com>,
	"linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
	Ingo Molnar <mingo@...nel.org>, Mel Gorman <mgorman@...e.de>,
	Matthew Wilcox <willy@...ux.intel.com>,
	Ross Zwisler <ross.zwisler@...ux.intel.com>,
	Rik van Riel <riel@...hat.com>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Jens Axboe <axboe@...nel.dk>, "Theodore Ts'o" <tytso@....edu>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Julia Lawall <Julia.Lawall@...6.fr>, Tejun Heo <tj@...nel.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v2 00/10] evacuate struct page from the block layer,
 introduce __pfn_t

On Wed, May 6, 2015 at 5:19 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Wed, May 6, 2015 at 4:47 PM, Dan Williams <dan.j.williams@...el.com> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists