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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
