On Thu, Jul 11, 2024 at 09:07:53PM +0900, Hyeonggon Yoo wrote: > Hi folks, > > Byungchul, Gwan-gyeong and I are investigating possible circular > dependency reported by a dependency tracker named DEPT [1], which is > able to report possible circular dependencies involving folio locks > and other forms of dependencies that are not locks (i.e., wait for > completion). > > Below are two similar reports from DEPT where one context takes > i_data_sem and then folio lock in ext4_map_blocks(), while the other > context takes folio lock and then i_data_sem during processing of > pwrite64() system calls. We're reaching out due to a lack of > understanding of ext4 and file system internals. > > The points in question are: > > - Can the two contexts actually create a dependency between each other > in ext4? In other words, do their uses of folio lock make them belong > to the same lock classes? No. > - Are there any locking rules in ext4 that ensure these two contexts > will never be considered as the same lock class? It's inherent is the code path. In one of the stack traces, we are using the page cache for the bitmap allocation block (in other words, a metadata block). In the other stack trace, the page cache belongs to a regular file (in other words, a data block). So this is a false positive with DEPT, which has always been one of the reasons why I've been dubious about the value of DEPT in terms of potential for make-work for mantainer once automated systems like syzbot try to blindly use and it results in huge numbers of false positive reports that we then have to work through as an unfunded mandate. If you want to add lock annotations into the struct page or even struct folio, I cordially invite you to try running that by the mm developers, who will probably tell you why that is a terrible idea since it bloats a critical data structure. Cheers, - Ted
