Issue with 8K folio size in __filemap_get_folio()

[Viacheslav Dubeyko <slava@xxxxxxxxxxx>]

Hi Matthew,

I believe we have issue in __filemap_get_folio() logic for the case of 8K folio size (order is equal to 1).

Let’s imagine we have such code and folio is not created yet for index == 0:

fgf_t fgp_flags = FGP_WRITEBEGIN;

mapping_set_large_folios(mapping);
fgp_flags |= fgf_set_order(8192);

folio = __filemap_get_folio(mapping, 0, fgf_flags, mapping_gfp_mask(mapping));

As a result, we received folio with size 4K but not 8K as it was expected:

struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
		fgf_t fgp_flags, gfp_t gfp)
{

	folio = filemap_get_entry(mapping, index);
	if (xa_is_value(folio))
		folio = NULL;
	if (!folio)
		goto no_page;   <—— we have no folio, so we jump to no_page

/* skipped */

no_page:
	if (!folio && (fgp_flags & FGP_CREAT)) {
		unsigned order = FGF_GET_ORDER(fgp_flags);  <—— we have order == 1
		int err;

/* skipped */

		if (!mapping_large_folio_support(mapping)) <— we set up the support of large folios
			order = 0;
		if (order > MAX_PAGECACHE_ORDER)
			order = MAX_PAGECACHE_ORDER;
		/* If we're not aligned, allocate a smaller folio */
		if (index & ((1UL << order) - 1))
			order = __ffs(index);

/* we still have order is equal to 1 here */

		do {
			gfp_t alloc_gfp = gfp;

			err = -ENOMEM;
			if (order == 1)
				order = 0;  <—— we correct the order to zero because order was 1

			if (order > 0)
				alloc_gfp |= __GFP_NORETRY | __GFP_NOWARN;
			folio = filemap_alloc_folio(alloc_gfp, order);

/* Finally, we allocated folio with 4K instead of 8K */

			if (!folio)
				continue;

/* skipped */
		} while (order-- > 0);

/* skipped */
	}

	if (!folio)
		return ERR_PTR(-ENOENT);
	return folio;
}

So, why do we correct the order to zero always if order is equal to one?
It sounds for me like incorrect logic. Even if we consider the troubles
with memory allocation, then we will try allocate, for example, 16K, exclude 8K,
and, finally, will try to allocate 4K. This logic puzzles me anyway.
Do I miss something here?

Thanks,
Slava.