On Wed, Jul 01, 2015 at 03:59:44PM -0700, Darrick J. Wong wrote: > On Wed, Jul 01, 2015 at 11:58:43AM +1000, Dave Chinner wrote: > > I would have thought you only need to check the inode flag here > > because the only time it will be set is on a reflink enabled > > filesystem. i.e. that flag being set implies we've already done > > all the "reflink is supported in this filesystem and it's not a > > realtime file" checks when setting the flag. > > Sure. The reason for so many ASSERTs everywhere is to help me check my > own sanity while cobbling together the first version. I imagine I could > eliminate a lot of them, but since they all compile out on !XFS_DEBUG && > !XFS_WARN, I didn't think it was a serious problem. :) Generally it's not, but we try to keep performance of the debug kernel within a few percent of a non-debug build, just so that it behaves roughly the same w.r.t. CPU and memory overhead, scalability and race conditions. Hence I'd much prefer to see strong validation of the parameters at the highest layer possible so that they don't need to be constantly checked in lower layers that have a single context. e.g. AG-specific modification functions shouldn't need to check the agno is valid, as they wouldn't have been called if someone tried to perform the operation on an invalid agno. Same goes for block numbers, etc. And for printk debugging to tell you whow functions a being called and what oeprations they are doing, you should replace all that with tracepoints. Addition of trace points at the entry and exit of functions gives sufficient information for verifying this during debugging, but has almost no overhead in the code or at runtime. They can also be switched on dynamically in production machines, which you can't do with compile option debug code like xfs_debug and ASSERT statements. i.e. tracepoints = good, debug printk = bad ;) Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
