On Fri, Aug 28, 2009 at 10:26:56PM -0400, Theodore Tso wrote: > On Wed, Aug 26, 2009 at 10:53:16AM +0530, Aneesh Kumar K.V wrote: > > Inorder to check whether the buffer_heads are mapped we need > > to hold page lock. Otherwise a reclaim can cleanup the attached > > buffer_heads. Instead of taking page lock and check whether > > buffer_heads are mapped we let the write_begin/write_end callback > > does the equivalent. It does have a performance impact in that we > > are doing more work if we the buffer_heads are already mapped. > > So I started looking at all of the work that we need to do in > write_begin/write_end; did you check both write paths depending on > whether we are using delayed allocation or not? It would seem to me > that it might be better to use lock_page() and unlock_page() around > the check, since in many work loads the buffer heads will already be > mapped often, and it appears to me that write_begin() will end up > locking the page anyway. > > Am I missing something? > Below are the possibilities i looked at a) mmap with no parallel write to the same offset. That would mean we don't have attached buffer heads because nobody attach buffer heads to the page. b) mmap happening to the hole. The buffer heads are not mapped. c) mmap with parallel write to the same offset. The parallel write did attach mapped buffer heads to the same page. So we should find all buffer heads mapped in the above case. if we will find buffer heads already be mapped in many workloads then i guess it make sense to add page lock. It will also avoid the journal_start that we do in write_begin. I will redo the patch -aneesh -- 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
