On Fri, Apr 22, 2016 at 02:23:59PM -0400, Josef Bacik wrote: > On 04/22/2016 02:21 PM, Mark Fasheh wrote: > >On Fri, Apr 22, 2016 at 02:12:11PM -0400, Josef Bacik wrote: > >>On 04/15/2016 05:08 AM, Qu Wenruo wrote: > >>>+ /* > >>>+ * Force parent root to be updated, as we recorded it before so its > >>>+ * last_trans == cur_transid. > >>>+ * Or it won't be committed again onto disk after later > >>>+ * insert_dir_item() > >>>+ */ > >>>+ if (!ret) > >>>+ record_root_in_trans(trans, parent, 1); > >>>+ return ret; > >>>+} > >> > >>NACK, holy shit we aren't adding a special transaction commit only > >>for qgroup snapshots. Figure out a different way. Thanks, > > > >Yeah I saw that. To be fair, we run a whole lot of the transaction stuff > >multiple times (at least from my reading) so I'm really unclear on what the > >performance impact is. > > > >Do you have any suggestion though? We've been banging our heads against this > >for a while now and as slow as this patch might be, it actually works where > >nothing else has so far. > > I'm less concerned about committing another transaction and more > concerned about the fact that it is an special variant of the > transaction commit. If this goes wrong, or at some point in the > future we fail to update it along with btrfs_transaction_commit we > suddenly are corrupting metadata. If we have to commit a > transaction then call btrfs_commit_transaction(), don't open code a > stripped down version, here be dragons. Thanks, Ok yeah that makes perfect sense - I thought you were telling me that this would be a big performance problem. --Mark -- Mark Fasheh -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
