On Wed, Apr 15, 2009 at 06:38:47PM -0700, Andrew Morton wrote: > On Wed, 15 Apr 2009 10:25:07 +0200 Nick Piggin <npiggin@xxxxxxx> wrote: > > > so filesystems can avoid LOR conditions with page lock. > > All right, I give up. What's LOR? Lock order reversal. > > - Sage needs this race closed for ceph filesystem. > > - Trond for NFS (http://bugzilla.kernel.org/show_bug.cgi?id=12913). > > I wonder which kernel version(s) we should put this in. > > Going BUG isn't nice, but that report is against 2.6.27. Is the BUG > super-rare, or did we avoid it via other means, or what? Trond? > > @@ -2105,16 +2116,31 @@ unlock: > > * > > * do_no_page is protected similarly. > > */ > > - wait_on_page_locked(dirty_page); > > - set_page_dirty_balance(dirty_page, page_mkwrite); > > + if (!page_mkwrite) { > > + wait_on_page_locked(dirty_page); > > + set_page_dirty_balance(dirty_page, page_mkwrite); > > + } > > put_page(dirty_page); > > + if (page_mkwrite) { > > + struct address_space *mapping = dirty_page->mapping; > > + > > + set_page_dirty(dirty_page); > > + unlock_page(dirty_page); > > + page_cache_release(dirty_page); > > + balance_dirty_pages_ratelimited(mapping); > > hm. I wonder what prevents (prevented) *mapping from vanishing under > our feet here. down_read on mmap_sem should be keeping it pinned. > > + if (dirty_page) { > > + struct address_space *mapping = page->mapping; > > + > > if (vma->vm_file) > > file_update_time(vma->vm_file); > > > > - set_page_dirty_balance(dirty_page, page_mkwrite); > > + if (set_page_dirty(dirty_page)) > > + page_mkwrite = 1; > > + unlock_page(dirty_page); > > put_page(dirty_page); > > + if (page_mkwrite) > > + balance_dirty_pages_ratelimited(mapping); > > + } else { > > + unlock_page(vmf.page); > > + if (anon) > > + page_cache_release(vmf.page); > > } > > > > return ret; > > + > > +unwritable_page: > > + page_cache_release(page); > > + return ret; > > } > > Whoa. Running file_update_time() under lock_page() opens a whole can > of worms, doesn't it? That thing can do journal commits and all sorts > of stuff. And I don't think this ordering is necessary here? Oh good catch. Yes I think we can just move that out to after the put_page no problem. Thanks, Nick -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
