From: Jan Kara <jack@suse.cz>
To: "lixinhai.lxh@gmail.com" <lixinhai.lxh@gmail.com>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>,
"linux-mm@kvack.org" <linux-mm@kvack.org>, jack <jack@suse.cz>
Subject: Re: [PATCH] mm/memory.c: avoid repeated set_page_dirty in fault_dirty_shared_page
Date: Fri, 13 Dec 2019 18:50:17 +0100 [thread overview]
Message-ID: <20191213175017.GC15331@quack2.suse.cz> (raw)
In-Reply-To: <2019121315283073596252@gmail.com>
On Fri 13-12-19 15:28:32, lixinhai.lxh@gmail.com wrote:
> On 2019-12-13 at 00:55 Kirill A. Shutemov wrote:
> >On Thu, Dec 12, 2019 at 11:21:18PM +0800, Li Xinhai wrote:
> >> When vm_ops->page_mkwrite is defined, and called from wp_page_shared and
> >> do_shared_fault, the set_page_dirty must already called by page_mkwrite.
> >
> >Must? Do all ->page_mkwrite implementation do this?
>
> My understanding is that set_page_dirty need be called before PTE is set
> to allow writing. If not in this sequence, other thread will see a
> writable PTE and dirty the page before current thread set_page_dirty.
Yes, filesystems effectively do rely on this.
> In ->page_mkwrite, FS can decide if set_page_dirty should be called or
> not. I checked a few FS, ext4/xfs/btrsfs/ceph and generic
> filemap_page_mkwrite, they called it. If FS provide ->page_mkwrite and
> decide don't call set_page_dirty, why fault_dirty_shared_page call this
> function unconditionally? or, I missed something?
Well, generally the responsibility for dirtying the page has been on the
generic MM code (i.e., fault_dirty_shared_page()). Now you're right that
lots of filesystems will end up dirtying the page because they are reusing
generic helpers for handling ->page_mkwrite() and that happens to dirty the
page. But that is mostly a coincidence and not guarantee. So to safely
remove page dirtying from fault_dirty_shared_page(), you'd need to audit
*all* ->page_mkwrite() implementations, make sure they all dirty the page,
and then document this requirement somewhere. Overall I don't think the
effort is really worth it since redirtying already dirty page is very
cheap.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
prev parent reply other threads:[~2019-12-13 21:44 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-12 15:21 [PATCH] mm/memory.c: avoid repeated set_page_dirty in fault_dirty_shared_page Li Xinhai
2019-12-12 15:35 ` David Hildenbrand
2019-12-12 16:55 ` Kirill A. Shutemov
2019-12-13 7:28 ` lixinhai.lxh
2019-12-13 17:50 ` Jan Kara [this message]
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=20191213175017.GC15331@quack2.suse.cz \
--to=jack@suse.cz \
--cc=kirill@shutemov.name \
--cc=linux-mm@kvack.org \
--cc=lixinhai.lxh@gmail.com \
/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.