linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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@ml01.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 4/5] dax: fix mapping lifetime handling, convert to __pfn_t + kmap_atomic_pfn_t()
Date: Thu, 13 Aug 2015 11:51:51 -0700	[thread overview]
Message-ID: <CAPcyv4i+gufdFJ7hP4QUyQ_Z3MsjAMsFCC+-2bXtUEWVUZyrAA@mail.gmail.com> (raw)
In-Reply-To: <55CCC6FB.1020901@plexistor.com>

On Thu, Aug 13, 2015 at 9:34 AM, Boaz Harrosh <boaz@plexistor.com> wrote:
> On 08/13/2015 06:21 PM, Dan Williams wrote:
>> On Wed, Aug 12, 2015 at 11:26 PM, Boaz Harrosh <boaz@plexistor.com> wrote:
> <>
>>
>> Hmm, that's not the same block layer I've been working with for the
>> past several years:
>>
>> $ mount /dev/pmem0 /mnt
>> $ echo namespace0.0 > ../drivers/nd_pmem/unbind # succeeds
>>
>> Unbind always proceeds unconditionally.  See the recent kernel summit
>> topic discussion around devm vs unbind [1].  While kmap_atomic_pfn_t()
>> does not implement revoke semantics it at least forces re-validation
>> and time bounded references.  For the unplug case we'll need to go
>> shootdown those DAX mappings in userspace so that they return SIGBUS
>> on access, or something along those lines.
>>
>
> Then fix unbind to refuse. What is the point of unbind when it trashes
> the hot path so badly and makes the code so fat.

What? The DAX hot path avoids the kernel entirely.

> Who uses it and what for?

The device driver core.  We simply can't hold off remove indefinitely.
If the administrator wants the device disabled we need to tear down
and revoke active mappings, or at very least guarantee time bounded
removal.

> First I ever heard of it and I do use Linux a little bit.
>
>> [1]: http://www.spinics.net/lists/kernel/msg2032864.html
>>
> Hm...
>
> OK I hate it. I would just make sure to override and refuse unbinding with an
> elevated ref count. Is not a good reason for me to trash the hotpath.

Again, the current usages are not in hot paths.  If it becomes part of
a hot path *and* shows up in a profile we can look to implement
something with less overhead.  Until then we should plan to honor the
lifetime as defined by ->probe() and ->remove().

In fact I proposed the same as you, but then changed my mind based on
Tejun's response [1].  So please reconsider this idea to solve the
problem by blocking ->remove().  PMEM is new and special, but not
*that* special as to justify breaking basic guarantees.

[1]: https://lkml.org/lkml/2015/7/15/731

  reply	other threads:[~2015-08-13 18:51 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
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 [this message]
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=CAPcyv4i+gufdFJ7hP4QUyQ_Z3MsjAMsFCC+-2bXtUEWVUZyrAA@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@ml01.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;
as well as URLs for NNTP newsgroup(s).