| Message ID | 20190308150605.98405-1-alexander.lochmann@tu-dortmund.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] Updated locking documentation for struct journal_t | expand |
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@tu-dortmund.de> > Signed-off-by: Horst Schirmeier <horst.schirmeier@tu-dortmund.de> > --- > 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
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; @@ -744,6 +745,7 @@ jbd2_time_diff(unsigned long start, unsigned long end) /** * struct journal_s - The journal_s type is the concrete type associated with * journal_t. + * (Locking Documentation improved by LockDoc) */ struct journal_s { @@ -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; @@ -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; @@ -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;