Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > + if (folio_test_uptodate(folio)) > > + goto out_put_folio; > > Er ... if (!folio_test_uptodate(folio)), perhaps? And is it even > worth testing if read_mapping_folio() returned success? I feel like > we should take ->readpage()'s word for it that success means the > folio is now uptodate. Actually, no, I shouldn't need to do this since it comes out with the page lock still held. > > + len = i_size_read(inode); > > + if (len > folio_size(folio)) > > extra space. Plus, you're hardcoding 4096 below, but using folio_size() > here which is a bit weird to me. As I understand it, 4096 is the maximum length of the inline data, not PAGE_SIZE, so I have to be careful when doing a DIO read because it might start after the data - and there's also truncate to consider:-/ I wonder if the uninlining code should lock the inode while it does it and the truncation code should do uninlining too. David