Re: [PATCH v6 4/5] mm/migrate: skip migrating folios under writeback with AS_WRITEBACK_INDETERMINATE mappings

[Shakeel Butt <shakeel.butt@linux.dev>]

On Thu, Dec 19, 2024 at 04:47:18PM +0100, David Hildenbrand wrote:
> On 19.12.24 16:43, Shakeel Butt wrote:
> > On Thu, Dec 19, 2024 at 02:05:04PM +0100, David Hildenbrand wrote:
> > > On 23.11.24 00:23, Joanne Koong wrote:
> > > > For migrations called in MIGRATE_SYNC mode, skip migrating the folio if
> > > > it is under writeback and has the AS_WRITEBACK_INDETERMINATE flag set on its
> > > > mapping. If the AS_WRITEBACK_INDETERMINATE flag is set on the mapping, the
> > > > writeback may take an indeterminate amount of time to complete, and
> > > > waits may get stuck.
> > > > 
> > > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> > > > Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
> > > > ---
> > > >    mm/migrate.c | 5 ++++-
> > > >    1 file changed, 4 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/mm/migrate.c b/mm/migrate.c
> > > > index df91248755e4..fe73284e5246 100644
> > > > --- a/mm/migrate.c
> > > > +++ b/mm/migrate.c
> > > > @@ -1260,7 +1260,10 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
> > > >    		 */
> > > >    		switch (mode) {
> > > >    		case MIGRATE_SYNC:
> > > > -			break;
> > > > +			if (!src->mapping ||
> > > > +			    !mapping_writeback_indeterminate(src->mapping))
> > > > +				break;
> > > > +			fallthrough;
> > > >    		default:
> > > >    			rc = -EBUSY;
> > > >    			goto out;
> > > 
> > > Ehm, doesn't this mean that any fuse user can essentially completely block
> > > CMA allocations, memory compaction, memory hotunplug, memory poisoning... ?!
> > > 
> > > That sounds very bad.
> > 
> > The page under writeback are already unmovable while they are under
> > writeback. This patch is only making potentially unrelated tasks to
> > synchronously wait on writeback completion for such pages which in worst
> > case can be indefinite. This actually is solving an isolation issue on a
> > multi-tenant machine.
> > 
> Are you sure, because I read in the cover letter:
> 
> "In the current FUSE writeback design (see commit 3be5a52b30aa ("fuse:
> support writable mmap"))), a temp page is allocated for every dirty
> page to be written back, the contents of the dirty page are copied over to
> the temp page, and the temp page gets handed to the server to write back.
> This is done so that writeback may be immediately cleared on the dirty
> page,"
> 
> Which to me means that they are immediately movable again?

Oh sorry, my mistake, yes this will become an isolation issue with the
removal of the temp page in-between which this series is doing. I think
the tradeoff is between extra memory plus slow write performance versus
temporary unmovable memory.

> 
> -- 
> Cheers,
> 
> David / dhildenb
> 
>