All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Arcangeli <aarcange@redhat.com>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	kirill.shutemov@linux.intel.com, linux-kernel@vger.kernel.org,
	"linux-mm@kvack.org" <linux-mm@kvack.org>
Subject: Re: [BUG] vfio device assignment regression with THP ref counting redesign
Date: Thu, 5 May 2016 17:24:06 +0200	[thread overview]
Message-ID: <20160505152406.GH28755@redhat.com> (raw)
In-Reply-To: <20160505151110.GA13972@node.shutemov.name>

On Thu, May 05, 2016 at 06:11:10PM +0300, Kirill A. Shutemov wrote:
> Hm. How total_mapcount equal to NULL wouldn't lead to NULL-pointer
> dereference inside page_trans_huge_mapcount()?

Sorry for the confusion, this was still work in progress and then I've
seen the email from Alex and I sent the last version I had committed
right away. An earlier version of course had the proper checks for
NULL but they got wiped as I transitioned from one model to another
and back.

> > +				page_move_anon_rmap(old_page, vma, address);
> 
> compound_head() is missing, I believe.

Oh yes, fixed that too.

			if (total_mapcount == 1) {
				/*
				 * The page is all ours. Move it to
				 * our anon_vma so the rmap code will
				 * not search our parent or siblings.
				 * Protected against the rmap code by
				 * the page lock.
				 */
				page_move_anon_rmap(compound_head(old_page),
						    vma, address);
			}


If there's no other issue I can git send-email.

Then we should look into calling page_move_anon_rmap from THP COWs
too, hugetlbfs calls it too. I think we probably need to make
page_move_anon_rmap smarter and optionally let it take the lock for us
after reading page->mapping first to be sure it's really moving it.

The question is then if trylock or lock_page should be used, my
preference would be just trylock.

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: Andrea Arcangeli <aarcange@redhat.com>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	kirill.shutemov@linux.intel.com, linux-kernel@vger.kernel.org,
	"linux-mm@kvack.org" <linux-mm@kvack.org>
Subject: Re: [BUG] vfio device assignment regression with THP ref counting redesign
Date: Thu, 5 May 2016 17:24:06 +0200	[thread overview]
Message-ID: <20160505152406.GH28755@redhat.com> (raw)
In-Reply-To: <20160505151110.GA13972@node.shutemov.name>

On Thu, May 05, 2016 at 06:11:10PM +0300, Kirill A. Shutemov wrote:
> Hm. How total_mapcount equal to NULL wouldn't lead to NULL-pointer
> dereference inside page_trans_huge_mapcount()?

Sorry for the confusion, this was still work in progress and then I've
seen the email from Alex and I sent the last version I had committed
right away. An earlier version of course had the proper checks for
NULL but they got wiped as I transitioned from one model to another
and back.

> > +				page_move_anon_rmap(old_page, vma, address);
> 
> compound_head() is missing, I believe.

Oh yes, fixed that too.

			if (total_mapcount == 1) {
				/*
				 * The page is all ours. Move it to
				 * our anon_vma so the rmap code will
				 * not search our parent or siblings.
				 * Protected against the rmap code by
				 * the page lock.
				 */
				page_move_anon_rmap(compound_head(old_page),
						    vma, address);
			}


If there's no other issue I can git send-email.

Then we should look into calling page_move_anon_rmap from THP COWs
too, hugetlbfs calls it too. I think we probably need to make
page_move_anon_rmap smarter and optionally let it take the lock for us
after reading page->mapping first to be sure it's really moving it.

The question is then if trylock or lock_page should be used, my
preference would be just trylock.

  reply	other threads:[~2016-05-05 15:24 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-28 16:20 [BUG] vfio device assignment regression with THP ref counting redesign Alex Williamson
2016-04-28 16:20 ` Alex Williamson
2016-04-28 18:17 ` Kirill A. Shutemov
2016-04-28 18:17   ` Kirill A. Shutemov
2016-04-28 18:58   ` Alex Williamson
2016-04-28 18:58     ` Alex Williamson
2016-04-28 23:21     ` Andrea Arcangeli
2016-04-28 23:21       ` Andrea Arcangeli
2016-04-29  0:44       ` Alex Williamson
2016-04-29  0:44         ` Alex Williamson
2016-04-29  0:51       ` Kirill A. Shutemov
2016-04-29  0:51         ` Kirill A. Shutemov
2016-04-29  2:45         ` Alex Williamson
2016-04-29  2:45           ` Alex Williamson
2016-04-29  7:06           ` Kirill A. Shutemov
2016-04-29  7:06             ` Kirill A. Shutemov
2016-04-29 15:12             ` Alex Williamson
2016-04-29 15:12               ` Alex Williamson
2016-04-29 16:34             ` Andrea Arcangeli
2016-04-29 16:34               ` Andrea Arcangeli
2016-04-29 22:34               ` Alex Williamson
2016-04-29 22:34                 ` Alex Williamson
2016-05-02 10:41               ` Kirill A. Shutemov
2016-05-02 10:41                 ` Kirill A. Shutemov
2016-05-02 11:15                 ` Jerome Glisse
2016-05-02 11:15                   ` Jerome Glisse
2016-05-02 12:14                   ` GUP guarantees wrt to userspace mappings redesign Kirill A. Shutemov
2016-05-02 12:14                     ` Kirill A. Shutemov
2016-05-02 13:39                     ` Jerome Glisse
2016-05-02 13:39                       ` Jerome Glisse
2016-05-02 15:00                       ` GUP guarantees wrt to userspace mappings Kirill A. Shutemov
2016-05-02 15:00                         ` Kirill A. Shutemov
2016-05-02 15:22                         ` Jerome Glisse
2016-05-02 15:22                           ` Jerome Glisse
2016-05-02 16:12                           ` Kirill A. Shutemov
2016-05-02 16:12                             ` Kirill A. Shutemov
2016-05-02 19:14                             ` Andrea Arcangeli
2016-05-02 19:14                               ` Andrea Arcangeli
2016-05-02 19:11                           ` Andrea Arcangeli
2016-05-02 19:11                             ` Andrea Arcangeli
2016-05-02 19:02                         ` Andrea Arcangeli
2016-05-02 19:02                           ` Andrea Arcangeli
2016-05-02 14:15                     ` GUP guarantees wrt to userspace mappings redesign Oleg Nesterov
2016-05-02 14:15                       ` Oleg Nesterov
2016-05-02 16:21                       ` Kirill A. Shutemov
2016-05-02 16:21                         ` Kirill A. Shutemov
2016-05-02 16:22                         ` Oleg Nesterov
2016-05-02 16:22                           ` Oleg Nesterov
2016-05-02 18:03                           ` Kirill A. Shutemov
2016-05-02 18:03                             ` Kirill A. Shutemov
2016-05-02 17:41                             ` Oleg Nesterov
2016-05-02 17:41                               ` Oleg Nesterov
2016-05-02 18:56                     ` Andrea Arcangeli
2016-05-02 18:56                       ` Andrea Arcangeli
2016-05-02 15:23                 ` [BUG] vfio device assignment regression with THP ref counting redesign Andrea Arcangeli
2016-05-02 15:23                   ` Andrea Arcangeli
2016-05-02 16:00                   ` Kirill A. Shutemov
2016-05-02 16:00                     ` Kirill A. Shutemov
2016-05-02 18:03                     ` Andrea Arcangeli
2016-05-02 18:03                       ` Andrea Arcangeli
2016-05-05  1:19                       ` Alex Williamson
2016-05-05  1:19                         ` Alex Williamson
2016-05-05 14:39                         ` Andrea Arcangeli
2016-05-05 14:39                           ` Andrea Arcangeli
2016-05-05 15:09                           ` Andrea Arcangeli
2016-05-05 15:09                             ` Andrea Arcangeli
2016-05-05 15:11                           ` Kirill A. Shutemov
2016-05-05 15:11                             ` Kirill A. Shutemov
2016-05-05 15:24                             ` Andrea Arcangeli [this message]
2016-05-05 15:24                               ` Andrea Arcangeli
2016-05-06  7:29                               ` Kirill A. Shutemov
2016-05-06  7:29                                 ` Kirill A. Shutemov

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=20160505152406.GH28755@redhat.com \
    --to=aarcange@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kirill@shutemov.name \
    --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.