Re: Report 1 in ext4 and journal based on v5.17-rc1

Byungchul Park
2022-02-23T01:41:02Z

On Tue, Feb 22, 2022 at 09:27:23AM +0100, Jan Kara wrote:
> On Thu 17-02-22 20:10:03, Byungchul Park wrote:
> > [ 7.009608] ===================================================
> > [ 7.009613] DEPT: Circular dependency has been detected.
> > [ 7.009614] 5.17.0-rc1-00014-g8a599299c0cb-dirty #30 Tainted: G W
> > [ 7.009616] ---------------------------------------------------
> > [ 7.009617] summary
> > [ 7.009618] ---------------------------------------------------
> > [ 7.009618] *** DEADLOCK ***
> > [ 7.009618]
> > [ 7.009619] context A
> > [ 7.009619] [S] (unknown)(&(bit_wait_table + i)->dmap:0)
> > [ 7.009621] [W] down_write(&ei->i_data_sem:0)
> > [ 7.009623] [E] event(&(bit_wait_table + i)->dmap:0)
> > [ 7.009624]
> > [ 7.009625] context B
> > [ 7.009625] [S] down_read(&ei->i_data_sem:0)
> > [ 7.009626] [W] wait(&(bit_wait_table + i)->dmap:0)
> > [ 7.009627] [E] up_read(&ei->i_data_sem:0)
> > [ 7.009628]
>
> Looking into this I have noticed that Dept here tracks bitlocks (buffer
> locks in particular) but it apparently treats locks on all buffers as one
> locking class so it conflates lock on superblock buffer with a lock on
> extent tree block buffer. These are wastly different locks with different
> locking constraints. So to avoid false positives in filesystems we will
> need to add annotations to differentiate locks on different buffers (based
> on what the block is used for). Similarly how we e.g. annotate i_rwsem for

Exactly yes. All synchronization objects should be classfied by what it
is used for. Even though it's already classified by the location of the
code initializing the object - roughly and normally saying we can expect
those have the same constraint, we are actually assigning different
constraints according to the subtle design esp. in file systems.

It would also help the code have better documentation ;-) I'm willing to
add annotations for that to fs.

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