From: Jerome Glisse <jglisse@redhat.com>
To: John Hubbard <jhubbard@nvidia.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Dan Williams <dan.j.williams@intel.com>
Subject: Re: [PATCH v2 10/11] mm/hmm: add helpers for driver to safely take the mmap_sem v2
Date: Thu, 28 Mar 2019 19:24:04 -0400 [thread overview]
Message-ID: <20190328232404.GK13560@redhat.com> (raw)
In-Reply-To: <9e414b8c-0f98-a2f7-4f46-d335c015fc1b@nvidia.com>
On Thu, Mar 28, 2019 at 04:20:37PM -0700, John Hubbard wrote:
> On 3/28/19 4:05 PM, Jerome Glisse wrote:
> > On Thu, Mar 28, 2019 at 03:43:33PM -0700, John Hubbard wrote:
> >> On 3/28/19 3:40 PM, Jerome Glisse wrote:
> >>> On Thu, Mar 28, 2019 at 03:25:39PM -0700, John Hubbard wrote:
> >>>> On 3/28/19 3:08 PM, Jerome Glisse wrote:
> >>>>> On Thu, Mar 28, 2019 at 02:41:02PM -0700, John Hubbard wrote:
> >>>>>> On 3/28/19 2:30 PM, Jerome Glisse wrote:
> >>>>>>> On Thu, Mar 28, 2019 at 01:54:01PM -0700, John Hubbard wrote:
> >>>>>>>> On 3/25/19 7:40 AM, jglisse@redhat.com wrote:
> >>>>>>>>> From: Jérôme Glisse <jglisse@redhat.com>
> >>>> [...]
> >>>> OK, so let's either drop this patch, or if merge windows won't allow that,
> >>>> then *eventually* drop this patch. And instead, put in a hmm_sanity_check()
> >>>> that does the same checks.
> >>>
> >>> RDMA depends on this, so does the nouveau patchset that convert to new API.
> >>> So i do not see reason to drop this. They are user for this they are posted
> >>> and i hope i explained properly the benefit.
> >>>
> >>> It is a common pattern. Yes it only save couple lines of code but down the
> >>> road i will also help for people working on the mmap_sem patchset.
> >>>
> >>
> >> It *adds* a couple of lines that are misleading, because they look like they
> >> make things safer, but they don't actually do so.
> >
> > It is not about safety, sorry if it confused you but there is nothing about
> > safety here, i can add a big fat comment that explains that there is no safety
> > here. The intention is to allow the page fault handler that potential have
> > hundred of page fault queue up to abort as soon as it sees that it is pointless
> > to keep faulting on a dying process.
> >
> > Again if we race it is _fine_ nothing bad will happen, we are just doing use-
> > less work that gonna be thrown on the floor and we are just slowing down the
> > process tear down.
> >
>
> In addition to a comment, how about naming this thing to indicate the above
> intention? I have a really hard time with this odd down_read() wrapper, which
> allows code to proceed without really getting a lock. It's just too wrong-looking.
> If it were instead named:
>
> hmm_is_exiting()
What about: hmm_lock_mmap_if_alive() ?
>
> and had a comment about why racy is OK, then I'd be a lot happier. :)
Will add fat comment.
Cheers,
Jérôme
next prev parent reply other threads:[~2019-03-28 23:24 UTC|newest]
Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-25 14:40 [PATCH v2 00/11] Improve HMM driver API v2 jglisse
2019-03-25 14:40 ` [PATCH v2 01/11] mm/hmm: select mmu notifier when selecting HMM jglisse
2019-03-28 20:33 ` John Hubbard
2019-03-29 21:15 ` Jerome Glisse
2019-03-29 21:42 ` John Hubbard
2019-03-25 14:40 ` [PATCH v2 02/11] mm/hmm: use reference counting for HMM struct v2 jglisse
2019-03-28 11:07 ` Ira Weiny
2019-03-28 19:11 ` Jerome Glisse
2019-03-28 20:43 ` John Hubbard
2019-03-28 21:21 ` Jerome Glisse
2019-03-29 0:39 ` John Hubbard
2019-03-28 16:57 ` Ira Weiny
2019-03-29 1:00 ` Jerome Glisse
2019-03-29 1:18 ` John Hubbard
2019-03-29 1:50 ` Jerome Glisse
2019-03-28 18:21 ` Ira Weiny
2019-03-29 2:25 ` Jerome Glisse
2019-03-29 20:07 ` John Hubbard
2019-03-29 2:11 ` John Hubbard
2019-03-29 2:22 ` Jerome Glisse
2019-03-25 14:40 ` [PATCH v2 03/11] mm/hmm: do not erase snapshot when a range is invalidated jglisse
2019-03-25 14:40 ` [PATCH v2 04/11] mm/hmm: improve and rename hmm_vma_get_pfns() to hmm_range_snapshot() v2 jglisse
2019-03-28 13:30 ` Ira Weiny
2019-03-25 14:40 ` [PATCH v2 05/11] mm/hmm: improve and rename hmm_vma_fault() to hmm_range_fault() v2 jglisse
2019-03-28 13:43 ` Ira Weiny
2019-03-28 22:03 ` Jerome Glisse
2019-03-25 14:40 ` [PATCH v2 06/11] mm/hmm: improve driver API to work and wait over a range v2 jglisse
2019-03-28 13:11 ` Ira Weiny
2019-03-28 21:39 ` Jerome Glisse
2019-03-28 16:12 ` Ira Weiny
2019-03-29 0:56 ` Jerome Glisse
2019-03-28 18:49 ` Ira Weiny
2019-03-25 14:40 ` [PATCH v2 07/11] mm/hmm: add default fault flags to avoid the need to pre-fill pfns arrays jglisse
2019-03-28 21:59 ` John Hubbard
2019-03-28 22:12 ` Jerome Glisse
2019-03-28 22:19 ` John Hubbard
2019-03-28 22:31 ` Jerome Glisse
2019-03-28 22:40 ` John Hubbard
2019-03-28 23:21 ` Jerome Glisse
2019-03-28 23:28 ` John Hubbard
2019-03-28 16:42 ` Ira Weiny
2019-03-29 1:17 ` Jerome Glisse
2019-03-29 1:30 ` John Hubbard
2019-03-29 1:42 ` Jerome Glisse
2019-03-29 1:59 ` Jerome Glisse
2019-03-29 2:05 ` John Hubbard
2019-03-29 2:12 ` Jerome Glisse
2019-03-28 23:43 ` Jerome Glisse
2019-03-25 14:40 ` [PATCH v2 08/11] mm/hmm: mirror hugetlbfs (snapshoting, faulting and DMA mapping) v2 jglisse
2019-03-28 16:53 ` Ira Weiny
2019-03-25 14:40 ` [PATCH v2 09/11] mm/hmm: allow to mirror vma of a file on a DAX backed filesystem v2 jglisse
2019-03-28 18:04 ` Ira Weiny
2019-03-29 2:17 ` Jerome Glisse
2019-03-25 14:40 ` [PATCH v2 10/11] mm/hmm: add helpers for driver to safely take the mmap_sem v2 jglisse
2019-03-28 20:54 ` John Hubbard
2019-03-28 21:30 ` Jerome Glisse
2019-03-28 21:41 ` John Hubbard
2019-03-28 22:08 ` Jerome Glisse
2019-03-28 22:25 ` John Hubbard
2019-03-28 22:40 ` Jerome Glisse
2019-03-28 22:43 ` John Hubbard
2019-03-28 23:05 ` Jerome Glisse
2019-03-28 23:20 ` John Hubbard
2019-03-28 23:24 ` Jerome Glisse [this message]
2019-03-28 23:34 ` John Hubbard
2019-03-28 18:44 ` Ira Weiny
2019-03-25 14:40 ` [PATCH v2 11/11] mm/hmm: add an helper function that fault pages and map them to a device v2 jglisse
2019-04-01 11:59 ` Souptick Joarder
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=20190328232404.GK13560@redhat.com \
--to=jglisse@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=dan.j.williams@intel.com \
--cc=jhubbard@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.