On Fri 08-03-19 16:06:05, Alexander Lochmann wrote: > We used LockDoc to derive locking rules for each member > of struct journal_t. Based on those results, we extended > the existing documentation by more members > of struct inode, and updated the existing documentation. > > Signed-off-by: Alexander Lochmann <alexander.lochmann@xxxxxxxxxxxxxx> > Signed-off-by: Horst Schirmeier <horst.schirmeier@xxxxxxxxxxxxxx> > --- > include/linux/jbd2.h | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) Thanks for the patch! Some comments below. > diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h > index 0f919d5fe84f..c1ba44ca515a 100644 > --- a/include/linux/jbd2.h > +++ b/include/linux/jbd2.h > @@ -585,7 +585,8 @@ struct transaction_s > } t_state; > > /* > - * 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; > > @@ -647,17 +648,17 @@ struct transaction_s > unsigned long t_max_wait; > > /* > - * When transaction started > + * When transaction started [j_state_lock] > */ > unsigned long t_start; > > /* > - * When commit was requested > + * When commit was requested [j_state_lock] > */ > unsigned long t_requested; > > /* > - * Checkpointing stats [j_checkpoint_sem] > + * Checkpointing stats [j_list_lock] > */ > struct transaction_chp_stats_s t_chp_stats; > These look correct to me. > @@ -762,6 +764,7 @@ struct journal_s > > /** > * @j_sb_buffer: The first part of the superblock buffer. > + * [j_checkpoint_mutex] > */ > struct buffer_head *j_sb_buffer; This isn't true. j_sb_buffer is just initialized in journal_init_common() and never changed. So there's no lock protecting it. > @@ -1015,6 +1018,7 @@ struct journal_s > * @j_commit_interval: > * > * What is the maximum transaction lifetime before we begin a commit? > + * [super_block->s_umount] > */ > unsigned long j_commit_interval; Ditto here. > @@ -1032,7 +1036,7 @@ struct journal_s > * @j_revoke: > * > * The revoke table - maintains the list of revoked blocks in the > - * current transaction. > + * current transaction. [j_state_lock] > */ > struct jbd2_revoke_table_s *j_revoke; > This is also not true. The contents of the revoke table itself is protected by j_revoke_lock. The pointer to current revoke table (i.e. j_revoke itself) is changed only when the currently running transation is in T_SWITCH state - a special state where no updates can be happening anymore and new transaction cannot be running yet either. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
