On Mon, Mar 18, 2019 at 07:42:37PM +0100, Alexander Lochmann wrote: > /*t > - * Where in the log does this transaction's commit start? [no locking] > + * Where in the log does this transaction's commit start? > + * [journal_t.j_state_lock] > */ > unsigned long t_log_start; Well, technically, that's not quite right. It's only assigned in one location, and we hold j_state_lock, yes. But that's because we need to access journal->j_head. At the point where we set t_log_start, the transaction has already been locked down (transaction->t_state > T_LOCKED). Similarly, we happen to be holding j_state where it is currently being accessed, but it's not because we needed the lock in order to access t_log_start safely. > /* > - * When transaction started > + * When transaction started [journal_t.j_state_lock] > */ > unsigned long t_start; And again, not really. The primary place where t_start is set is when the transaction is firstt created, before it's visible anywhere else. after that, it is used exclusively by the commit thread, and so no locking is necessary. It's true that in the places where it is used, j_state_lock happens to be taken, but it's strictly not necessary. > > /* > - * When commit was requested > + * When commit was requested [journal_t.j_state_lock] > */ > unsigned long t_requested; Yes, that appears to be correct. > > /* > - * Checkpointing stats [j_checkpoint_sem] > + * Checkpointing stats [journal_t.j_list_lock] > */ > struct transaction_chp_stats_s t_chp_stats; > This appears to be correct. - Ted
