On Mon, 2011-04-11 at 13:41 -0400, Chris Mason wrote: > Excerpts from Jeff Layton's message of 2011-04-11 12:42:29 -0400: > > > @@ -5839,6 +5844,15 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) > > > if (ret < 0) > > > goto out_unlock; > > > ret = 0; > > > + > > > + /* > > > + * write_begin/end might have created a dirty page and someone > > > + * could wander in and start the IO. Make sure that hasn't > > > + * happened. > > > + */ > > > + lock_page(page); > > > + wait_on_page_writeback(page); > > > + unlock_page(page); > > I am little puzzled here. if someone wander in and start the IO, the page is up-to-date (dirtied by this page_mkwrite). We shouldn't see the checksum inconsistancy, right? > > nit: > > > > The callers of page_mkwrite always lock the page afterward if you > > return from page_mkwrite with it unlocked. If you plan to take page > > lock anyway, it's probably slightly more efficient not to unlock it and > > instead return VM_FAULT_LOCKED. > > > > Actually this isn't a nit. Keeping the page locked closes an important > hole where it can become writeback again. It might fix the last > remaining problem. > Oh, right. Currently ext4_page_mkwrite drops the page lock before calling it's dirty the page (by write_begin() and write_end(). I suspect regrab the lock() after write_end() (with your proposed change) and returning with locked still leave the dirty by ext4_page_mkwrite unlocked. We probably should to keep the page locked the page during the entire ext4_page_mkwrite() call. Any reason to drop the page lock() before calling aops->write_begin()? Mingming -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
