On 05/31/2010 04:15 PM, Nick Piggin wrote: > On Mon, May 31, 2010 at 01:17:35PM +0300, Boaz Harrosh wrote: >> On 05/31/2010 12:50 PM, Boaz Harrosh wrote: >>> On 05/30/2010 11:49 PM, Christoph Hellwig wrote: >>>> This series is something that I see as the first major step towards >>>> a broad switch to the new truncate sequence. The patches get rid >>>> of the _newtrunc variant of blockdev_direct_IO & friends and >>>> *_write_begin, and clean up some bits in that area that make the >>>> switch easier. After this we have all vmtruncate instances except >>>> for inode_setattr in filesystem code. A second series to deal >>>> with ->setattr will follow and after that we can easily switch >>>> over one filesystem after another. >>>> >>>> I think this is still 2.6.34 material as it will make the fs >>>> switches a lot easier and avoid introducing the _newtrunc variants >>>> for one kernel release. >>>> -- >>>> 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 >>> >>> >>> Christoph, Nick hi. >>> >>> I'm attempting a truncate conversion for exofs, will need a review. >>> >>> I'm basing the first attempt on Al's next tree that has Nicks last >>> patches. Will rebase on these when they get into that tree. Please >>> advise on the best tree to use? >>> (BTW did you mean 2.6.35 above, I guess) >>> >>> So one minor thing ext2_setsize can be static: > > Thanks. > > >>> (It used to be used by the struct inode_operations) >>> >> >> One more thing. ext2_setsize is only used by ext2_setattr do we still >> need this code? (form ext2_setsize) half of it is done in ext2_setattr >> already) >> >> inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC; >> if (inode_needs_sync(inode)) { >> sync_mapping_buffers(inode->i_mapping); >> ext2_sync_inode (inode); >> } else { >> mark_inode_dirty(inode); >> } >> >> And generic_setattr does some more of that, No? > > Not sure. Some callers appear not to set ATTR_CTIME/ATTR_MTIME when > making ATTR_SIZE changes. And I don't know the history of the required > sync semantics here either. > > I think these would be good questions to raise with the ext? > maintainers, though. ext3/4 have similar code. > OK, I've rechecked and and no one is doing: inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC; So this one is needed as is. But the: if (inode_needs_sync(inode)) { sync_mapping_buffers(inode->i_mapping); ext2_sync_inode (inode); } else { mark_inode_dirty(inode); } Is just stupid given that mark_inode_dirty(inode); is again done in ext2_setattr() right after the call to _setsize. Perhaps if needed, then move it into _setattr (Please also have a look at my exofs: truncate patch I would love your input) Boaz -- 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
