From: Dan Williams <dan.j.williams@intel.com>
To: Boaz Harrosh <boaz@plexistor.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Jens Axboe <axboe@kernel.dk>, Rik van Riel <riel@redhat.com>,
"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
Linux MM <linux-mm@kvack.org>, Mel Gorman <mgorman@suse.de>,
"torvalds@linux-foundation.org" <torvalds@linux-foundation.org>,
Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v5 2/5] allow mapping page-less memremaped areas into KVA
Date: Thu, 13 Aug 2015 05:57:26 -0700 [thread overview]
Message-ID: <CAPcyv4gwFD5F=k_qQyf68z74Opzf1t4DMqY+A9D2w_Fwsbzvew@mail.gmail.com> (raw)
In-Reply-To: <55CC3222.5090503@plexistor.com>
On Wed, Aug 12, 2015 at 10:58 PM, Boaz Harrosh <boaz@plexistor.com> wrote:
> On 08/13/2015 06:01 AM, Dan Williams wrote:
[..]
>> +void *kmap_atomic_pfn_t(__pfn_t pfn)
>> +{
>> + struct page *page = __pfn_t_to_page(pfn);
>> + resource_size_t addr;
>> + struct kmap *kmap;
>> +
>> + rcu_read_lock();
>> + if (page)
>> + return kmap_atomic(page);
>
> Right even with pages I pay rcu_read_lock(); for every access?
>
>> + addr = __pfn_t_to_phys(pfn);
>> + list_for_each_entry_rcu(kmap, &ranges, list)
>> + if (addr >= kmap->res->start && addr <= kmap->res->end)
>> + return kmap->base + addr - kmap->res->start;
>> +
>
> Good god! This loop is a real *joke*. You have just dropped memory access
> performance by 10 fold.
>
> The all point of pages and memory_model.h was to have a one to one
> relation-ships between Kernel-virtual vs physical vs page *
>
> There is already an object that holds a relationship of physical
> to Kernel-virtual. It is called a memory-section. Why not just
> widen its definition?
>
> If you are willing to accept this loop. In current Linux 2015 Kernel
> Then I have nothing farther to say.
>
> Boaz - go mourning for the death of the Linux Kernel alone in the corner ;-(
>
This is explicitly addressed in the changelog, repeated here:
> The __pfn_t to resource lookup is indeed inefficient walking of a linked list,
> but there are two mitigating factors:
>
> 1/ The number of persistent memory ranges is bounded by the number of
> DIMMs which is on the order of 10s of DIMMs, not hundreds.
>
> 2/ The lookup yields the entire range, if it becomes inefficient to do a
> kmap_atomic_pfn_t() a PAGE_SIZE at a time the caller can take
> advantage of the fact that the lookup can be amortized for all kmap
> operations it needs to perform in a given range.
DAX as is is races against pmem unbind. A synchronization cost must
be paid somewhere to make sure the memremap() mapping is still valid.
--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2015-08-13 12:57 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-13 3:00 [PATCH v5 0/5] introduce __pfn_t for unmapped pfn I/O and DAX lifetime Dan Williams
2015-08-13 3:01 ` [PATCH v5 1/5] mm: move __phys_to_pfn and __pfn_to_phys to asm/generic/memory_model.h Dan Williams
2015-08-13 3:01 ` [PATCH v5 2/5] allow mapping page-less memremaped areas into KVA Dan Williams
2015-08-13 5:58 ` Boaz Harrosh
2015-08-13 12:57 ` Dan Williams [this message]
2015-08-13 13:23 ` Boaz Harrosh
2015-08-13 14:41 ` Christoph Hellwig
2015-08-13 15:01 ` Boaz Harrosh
2015-08-13 14:37 ` Christoph Hellwig
2015-08-13 14:48 ` Boaz Harrosh
2015-08-13 15:29 ` Boaz Harrosh
2015-08-13 17:37 ` Dave Hansen
2015-08-13 17:35 ` Matthew Wilcox
2015-08-13 18:15 ` Dan Williams
2015-08-13 3:01 ` [PATCH v5 3/5] dax: drop size parameter to ->direct_access() Dan Williams
2015-08-13 3:01 ` [PATCH v5 4/5] dax: fix mapping lifetime handling, convert to __pfn_t + kmap_atomic_pfn_t() Dan Williams
2015-08-13 6:26 ` Boaz Harrosh
2015-08-13 15:21 ` Dan Williams
2015-08-13 16:34 ` Boaz Harrosh
2015-08-13 18:51 ` Dan Williams
2015-08-13 3:01 ` [PATCH v5 5/5] scatterlist: convert to __pfn_t Dan Williams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPcyv4gwFD5F=k_qQyf68z74Opzf1t4DMqY+A9D2w_Fwsbzvew@mail.gmail.com' \
--to=dan.j.williams@intel.com \
--cc=axboe@kernel.dk \
--cc=boaz@plexistor.com \
--cc=hch@lst.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-nvdimm@lists.01.org \
--cc=mgorman@suse.de \
--cc=riel@redhat.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox