Re: [PATCH v17 30/47] fs/jbd2: use a weaker annotation in journal handling

Byungchul Park
2025-10-03T01:13:08Z

On Thu, Oct 02, 2025 at 10:40:56AM +0200, Jan Kara wrote:
> On Thu 02-10-25 17:12:30, Byungchul Park wrote:
> > jbd2 journal handling code doesn't want jbd2_might_wait_for_commit()
> > to be placed between start_this_handle() and stop_this_handle(). So it
> > marks the region with rwsem_acquire_read() and rwsem_release().
> >
> > However, the annotation is too strong for that purpose. We don't have
> > to use more than try lock annotation for that.
> >
> > rwsem_acquire_read() implies:
> >
> > 1. might be a waiter on contention of the lock.
> > 2. enter to the critical section of the lock.
> >
> > All we need in here is to act 2, not 1. So trylock version of
> > annotation is sufficient for that purpose. Now that dept partially
> > relies on lockdep annotaions, dept interpets rwsem_acquire_read() as a
> > potential wait and might report a deadlock by the wait.
> >
> > Replace it with trylock version of annotation.
> >
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
>
> Indeed. Feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>

Thank you, Jan.

Byungchul

> Honza
>
> > ---
> > fs/jbd2/transaction.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> > index c7867139af69..b4e65f51bf5e 100644
> > --- a/fs/jbd2/transaction.c
> > +++ b/fs/jbd2/transaction.c
> > @@ -441,7 +441,7 @@ static int start_this_handle(journal_t *journal, handle_t *handle,
> > read_unlock(&journal->j_state_lock);
> > current->journal_info = handle;
> >
> > - rwsem_acquire_read(&journal->j_trans_commit_map, 0, 0, _THIS_IP_);
> > + rwsem_acquire_read(&journal->j_trans_commit_map, 0, 1, _THIS_IP_);
> > jbd2_journal_free_transaction(new_transaction);
> > /*
> > * Ensure that no allocations done while the transaction is open are
> > --
> > 2.17.1
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR