On Oct 17, 2008 08:25 -0400, Theodore Ts'o wrote: > What I added was a dead-simple per-journal commit callback, with no > additional memory allocations (and requirement to do error handling if > the memory allocation fails), no need to take a spinlock before > manually adding the call back to each transaction handle, no need to > search the linked list to see if we have an entry on the linked list > already, etc. > > If in the future we need a true per-transaction handle commit > callback, we can add this; but I think it still makes more sense to > keep the per-journal commit callback. After all, as it stands the > current patch results in a net reduction of 46 lines of code. Adding > all of this machinery would erase take far more than the savings by > removing ext4_mb_poll_new_transaction(). The problem with the mechanism you've implemented is that it isn't possible to add any other kind of callback to the journal. There is only a single callback function, and the entries in the "t_private_list" are all assumed to be "ext4_free_data" structures so even if other users wanted to add callbacks they would only be handled by the release_blocks_on_commit() function. Is there any reason not to make this more generic and have the callback function pointer embedded in the "ext4_free_data" struct in some way that other callbacks could be registered? This would still avoid the need to allocate for each of these operations, but would make the callback mechanism more generic and useful. Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, Inc. -- 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
