On 22/11/04 12:27AM, Christoph Hellwig wrote: > On Wed, Nov 02, 2022 at 10:35:38AM -0700, Darrick J. Wong wrote: > > Just so long as nobody imports that nonfeature of the bufferhead code > > where dirtying an already dirty bufferhead skips marking the folio dirty > > and writepage failures also fail to redirty the page. That bit us hard > > recently and I strongly prefer not to repeat that. > > Yes, that absolutely needs to be avoided. Sure, I am trying to discuss & investigate more in the same lines to avoid the coherency problems (if any) in the other thread. i.e. to understand the rules between keeping the iomap->state[] dirty bitmap in sync with folio dirtiness (in folio_mark_dirty() / folio_cancel_dirty()) When are those required / what "external operations" could require folio_mark_dirty() to have a call to iomap_set_range_dirty() to dirty the iop->state[] bitmaps. Similary for marking it clean e.g. in folio_cancel_dirty(). In parallel I am looking into, to have a quick test case which could help me replicate such scenario. > > > > We can always optimize by having a bit for the fairly common all dirty > > > case and only track and look at the array if that is no the case. > > > > Yes, it would help to make the ranges in the bit array better defined > > than the semi-opencoded logic there is now. (I'm whining specifically > > about the test_bit calls sprinkled around). > > That is an absolutely requirement. It was so obviosu to me that I > didn't bother to restate it after two others already pointed it out :) Yes. I will make those changes once rest of the things are sorted. > > > Once that's done it > > shouldn't be hard to add one more bit for the all-dirty state. Though > > I'd want to see the numbers to prove that it saves us time anywhere. > > We might be able to just use PG_private_2 in the page for now, but > maybe just adding a flags field to the iomap_page might be a better > idea, as the pageflags tend to have strange entanglements. Sure, thanks for the suggestions. -ritesh