lkml.org 
[lkml]   [2021]   [Feb]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/2] Updated locking documentation for transaction_t
    On Wed 10-02-21 10:57:39, Alexander Lochmann wrote:
    > Some members of transaction_t are allowed to be read without
    > any lock being held if consistency doesn't matter.
    > Based on LockDoc's findings, we extended the locking
    > documentation of those members.
    > Each one of them is marked with a short comment:
    > "no lock for quick racy checks".
    >
    > Signed-off-by: Alexander Lochmann <alexander.lochmann@tu-dortmund.de>
    > Signed-off-by: Horst Schirmeier <horst.schirmeier@tu-dortmund.de>

    Thanks for the patch! Some comments below...

    > ---
    > include/linux/jbd2.h | 8 ++++----
    > 1 file changed, 4 insertions(+), 4 deletions(-)
    >
    > diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
    > index 99d3cd051ac3..18f77d9b1745 100644
    > --- a/include/linux/jbd2.h
    > +++ b/include/linux/jbd2.h
    > @@ -594,18 +594,18 @@ struct transaction_s
    > */
    > unsigned long t_log_start;
    >
    > - /* Number of buffers on the t_buffers list [j_list_lock] */
    > + /* Number of buffers on the t_buffers list [j_list_lock, no lock for quick racy checks] */
    > int t_nr_buffers;

    So this case is actually somewhat different now that I audited the uses.
    There are two types of users - commit code (fs/jbd2/commit.c) and others.
    Other users properly use j_list_lock to access t_nr_buffers. Commit code
    does not use any locks because committing transaction is fully in
    ownership of the jbd2 thread and all other users need to check & wait for
    commit to be finished before doing anything with the transaction's buffers.

    > /*
    > * Doubly-linked circular list of all buffers reserved but not yet
    > - * modified by this transaction [j_list_lock]
    > + * modified by this transaction [j_list_lock, no lock for quick racy checks]
    > */
    > struct journal_head *t_reserved_list;
    >
    > /*
    > * Doubly-linked circular list of all metadata buffers owned by this
    > - * transaction [j_list_lock]
    > + * transaction [j_list_lock, no lock for quick racy checks]
    > */
    > struct journal_head *t_buffers;
    >
    > @@ -631,7 +631,7 @@ struct transaction_s
    > /*
    > * Doubly-linked circular list of metadata buffers being shadowed by log
    > * IO. The IO buffers on the iobuf list and the shadow buffers on this
    > - * list match each other one for one at all times. [j_list_lock]
    > + * list match each other one for one at all times. [j_list_lock, no lock for quick racy checks]
    > */
    > struct journal_head *t_shadow_list;

    The above three cases are the same as t_reserved_list.

    Honza
    --
    Jan Kara <jack@suse.com>
    SUSE Labs, CR

    \
     
     \ /
      Last update: 2021-02-11 10:39    [W:6.301 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site