On Fri, Jul 12, 2024 at 11:23:36PM +0200, Vlastimil Babka (SUSE) wrote: > On 7/12/24 7:31 AM, Byungchul Park wrote: > > On Fri, Jul 12, 2024 at 01:44:20PM +0900, Byungchul Park wrote: > >> > >> What a funny guy... He did neither 1) insisting it's a bug in your code > >> nor 3) insisting DEPT is a great tool, but just asking if there's any > >> locking rules based on the *different acqusition order* between folio > >> lock and i_data_sem that he observed anyway. > >> > >> I don't think you are a guy who introduces bugs, but the thing is it's > >> hard to find a *document* describing locking rules. Anyone could get > >> fairly curious about the different acquisition order. It's an open > >> source project. You are responsible for appropriate document as well. > >> > >> I don't understand why you act to DEPT like that by the way. You don't > >> have to becasue: > >> > >> 1. I added the *EXPERIMENTAL* tag in Kconfig as you suggested, which > >> will prevent autotesting until it's considered stable. However, > >> the report from DEPT can be a good hint to someone. > >> > >> 2. DEPT can locate code where needs to be documented even if it's not > >> a real bug. It could even help better documentation. > >> > >> DEPT hurts neither code nor performance unless enabling it. > > enabling means building with CONFIG_DEPT right? Yes. > >> > 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. > > I doubt anyone will object making struct page larger for a non-production > debugging config option, which AFAIU DEPT is, i.e. in the same area as > LOCKDEP or KASAN etc... I can see at least KMSAN already adds some fields to > struct page already. I think so. > >> I already said several times. Doesn't consume struct page. > > > > Sorry for that. I've changed the code so the current version consumes > > it by about two words if enabled. I can place it to page_ext as before > > if needed. > > page_ext is useful if you have a debugging feature that can be compiled in > but adds no overhead (memory, nor cpu thanks to static keys) unless enabled > on boot time, i.e. page_owner... so for DEPT it seems it would be an > unnecessary complication. Yeah, I will think it more. However, maybe, as you said, it could introduce a complication. Thanks. Byungchul
