2018-05-18 18:04 GMT+02:00 Christoph Hellwig <hch@xxxxxx>: > On Tue, May 15, 2018 at 10:16:52AM +0200, Andreas Gruenbacher wrote: >> > Yes. And from a quick look we can easily handle "stuffed" aka inline >> > data with the existing architecture. We just need a new IOMAP_INLINE >> > flag for the extent type. Details will need to be worked out how to >> > pass that data, either a struct page or virtual address should probably >> > do it. >> >> This is about data journaling, it has nothing to do with "stuffed" >> files per se. With journaled data writes, we need to do some special >> processing for each page so that the page ends up in the journal >> before it gets written back to its proper on-disk location. It just >> happens that since we need these per-page operations anyway, the >> "unstuffing" and "re-stuffing" nicely fits in those operations as >> well. > > Your series makes two uses of the callbacks - one use case is to > deal with inline data. Yes, it's not perfect. > The right way to deal with that is to make > inline data an explicit iomap type (done in my next posting of the > buffer head removal series), and then have iomap_begin find the data, > kmap it and return it in the iomap, with iomap_end doing any fixups. I see what you mean. How would iomap_write_actor deal with IOMAP_INLINE -- just skip iomap_write_begin and iomap_write_end and grab the page from the iomap? I'm a bit worried about reintroducing the deadlock that iomap_write_actor so carefully tries to avoid. > For the data journaing I agree that we need a post-write per page > callout, but I think this should be just a simple optional callout > just for data journaling, which doesn't override the remaining > write_end handling. > >> Been there. An earlier version of the patches did have a separate >> stuffed write path and the unstuffing and re-stuffing didn't happen in >> those per-page operations. The result was much messier overall. > > Pointer, please. It's in this patch: https://patchwork.kernel.org/patch/10191007/ from this posting: https://www.spinics.net/lists/linux-fsdevel/msg121318.html Thanks, Andreas