On Mon, Oct 12, 2015 at 11:21:56AM -0600, Ross Zwisler wrote: > On Mon, Oct 12, 2015 at 10:14:43AM +1100, Dave Chinner wrote: > > On Fri, Oct 09, 2015 at 04:02:08PM -0600, Ross Zwisler wrote: > <> > > > +/* > > > + * The lock ordering for ext2 DAX fault paths is: > > > + * > > > + * mmap_sem (MM) > > > + * ext2_inode_info->dax_sem > > > + * sb_start_pagefault (vfs, freeze - taken in DAX) > > > + * address_space->i_mmap_rwsem or page_lock (mutually exclusive in DAX) > > > + * ext2_inode_info->truncate_mutex > > > > This is a different lock order to XFS - it puts the i_mmaplock > > inside sb_start_pagefault(), not outside it. This ordering means the > > timestamp updates during the page fault are also under > > ext2_inode_info->dax_sem... > > Yep - I was trying not not open code dax_fault() yet again, but it looks like > both XFS and ext4 both open-code dax_fault() and call __dax_fault() directly. > > I assume that when we get an analogous lock to i_mmaplock in the ext4 fault > path we'll end up with the same locking order that is found in XFS. > > I'll do the same for v2 of this patch and I'll kill the then-unused > dax_fault(). Great! I'd suggest that you also then rename __dax_fault() to dax_fault() in that case, and also document the requirements for callers in terms of sb_start_pagefault() and timestamp updates... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
