[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200226155727.GA22036@iweiny-DESK2.sc.intel.com>
Date: Wed, 26 Feb 2020 07:57:28 -0800
From: Ira Weiny <ira.weiny@...el.com>
To: Jan Kara <jack@...e.cz>
Cc: Dave Chinner <david@...morbit.com>, Christoph Hellwig <hch@....de>,
linux-kernel@...r.kernel.org,
Alexander Viro <viro@...iv.linux.org.uk>,
"Darrick J. Wong" <darrick.wong@...cle.com>,
Dan Williams <dan.j.williams@...el.com>,
"Theodore Y. Ts'o" <tytso@....edu>, linux-ext4@...r.kernel.org,
linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH V4 07/13] fs: Add locking for a dynamic address space
operations state
On Wed, Feb 26, 2020 at 12:17:40PM +0100, Jan Kara wrote:
> On Tue 25-02-20 11:09:37, Dave Chinner wrote:
> > /me wonders if the best thing to do is to add a ->fault callout to
> > tell the filesystem to lock/unlock the inode right up at the top of
> > the page fault path, outside even the mmap_sem. That means all the
> > methods that the page fault calls are protected against S_DAX
> > changes, and it gives us a low cost method of serialising page
> > faults against DIO (e.g. via inode_dio_wait())....
>
> Well, that's going to be pretty hard. The main problem is: you cannot
> lookup VMA until you hold mmap_sem and the inode is inside the VMA. And
> this is a fundamental problem because until you hold mmap_sem, the address
> space can change and thus the virtual address you are faulting can be
> changing inode it is mapped to. So you would have to do some dance like:
>
> lock mmap_sem
> lookup vma
> get inode reference
> drop mmap_sem
> tell fs about page fault
> lock mmap_sem
> is the vma still the same?
>
> And I'm pretty confident the overhead will be visible in page fault
> intensive workloads...
I did not get to this level of detail... Rather I looked at it from a high
level perspective and thought "does the mode need to change while someone has
the mmap?" My thought is, that it does not make a lot of sense. Generally the
user has mmaped with some use case in mind (either DAX or non-DAX) and it seems
reasonable to keep that mode consistent while the map is in place.
So I punted and restricted the change.
Ira
>
> Honza
>
> --
> Jan Kara <jack@...e.com>
> SUSE Labs, CR
Powered by blists - more mailing lists