Re: [PATCH v2 00/10] evacuate struct page from the block layer, introduce __pfn_t

[Dan Williams]

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