On Wed, Nov 06, 2024 at 03:37:11PM -0800, Joanne Koong wrote: > On Thu, Oct 31, 2024 at 3:38 PM Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote: > > > > On Thu, Oct 31, 2024 at 02:52:57PM GMT, Joanne Koong wrote: > > > On Thu, Oct 31, 2024 at 1:06 PM Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote: > > > > > > > > On Thu, Oct 31, 2024 at 12:06:49PM GMT, Joanne Koong wrote: > > > > > On Wed, Oct 30, 2024 at 5:30 PM Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote: > > > > [...] > > > > > > > > > > > > Memory pool is a bit confusing term here. Most probably you are asking > > > > > > about the migrate type of the page block from which tmp page is > > > > > > allocated from. In a normal system, tmp page would be allocated from page > > > > > > block with MIGRATE_UNMOVABLE migrate type while the page cache page, it > > > > > > depends on what gfp flag was used for its allocation. What does fuse fs > > > > > > use? GFP_HIGHUSER_MOVABLE or something else? Under low memory situation > > > > > > allocations can get mixed up with different migrate types. > > > > > > > > > > > > > > > > I believe it's GFP_HIGHUSER_MOVABLE for the page cache pages since > > > > > fuse doesn't set any additional gfp masks on the inode mapping. > > > > > > > > > > Could we just allocate the fuse writeback pages with GFP_HIGHUSER > > > > > instead of GFP_HIGHUSER_MOVABLE? That would be in fuse_write_begin() > > > > > where we pass in the gfp mask to __filemap_get_folio(). I think this > > > > > would give us the same behavior memory-wise as what the tmp pages > > > > > currently do, > > > > > > > > I don't think it would be the same behavior. From what I understand the > > > > liftime of the tmp page is from the start of the writeback till the ack > > > > from the fuse server that writeback is done. While the lifetime of the > > > > page of the page cache can be arbitrarily large. We should just make it > > > > unmovable for its lifetime. I think it is fine to make the page > > > > unmovable during the writeback. We should not try to optimize for the > > > > bad or buggy behavior of fuse server. > > > > > > > > Regarding the avoidance of wait on writeback for fuse folios, I think we > > > > can handle the migration similar to how you are handling reclaim and in > > > > addition we can add a WARN() in folio_wait_writeback() if the kernel ever > > > > sees a fuse folio in that function. > > > > > > Awesome, this is what I'm planning to do in v3 to address migration then: > > > > > > 1) in migrate_folio_unmap(), only call "folio_wait_writeback(src);" if > > > src->mapping does not have the AS_NO_WRITEBACK_WAIT bit set on it (eg > > > fuse folios will have that AS_NO_WRITEBACK_WAIT bit set) > > > > > > 2) in the fuse filesystem's implementation of the > > > mapping->a_ops->migrate_folio callback, return -EAGAIN if the folio is > > > under writeback. > > > > 3) Add WARN_ONCE() in folio_wait_writeback() if folio->mapping has > > AS_NO_WRITEBACK_WAIT set and return without waiting. > > For v3, I'm going to change AS_NO_WRITEBACK_RECLAIM to > AS_WRITEBACK_MAY_BLOCK and skip 3) because 3) may be too restrictive. > For example, for the sync_file_range() syscall, we do want to wait on > writeback - it's ok in this case to call folio_wait_writeback() on a > fuse folio since the caller would have intentionally passed in a fuse > fd to sync_file_range(). > Sounds good.
