Re: [PATCH v3 2/3] block: add folio awareness instead of looping through pages

[Matthew Wilcox <willy@infradead.org>]

On Fri, May 24, 2024 at 02:52:31PM +0530, Kundan Kumar wrote:
> On 15/05/24 09:55PM, Matthew Wilcox wrote:
> > On Tue, May 07, 2024 at 08:15:08PM +0530, Kundan Kumar wrote:
> > > Add a bigger size from folio to bio and skip processing for pages.
> > > 
> > > Fetch the offset of page within a folio. Depending on the size of folio
> > > and folio_offset, fetch a larger length. This length may consist of
> > > multiple contiguous pages if folio is multiorder.
> > 
> > The problem is that it may not.  Here's the scenario:
> > 
> > int fd, fd2;
> > fd = open(src, O_RDONLY);
> > char *addr = mmap(NULL, 1024 * 1024, PROT_READ | PROT_WRITE,
> > 	MAP_PRIVATE | MAP_HUGETLB, fd, 0);
> 
> I also added MAP_ANONYMOUS flag here, otherwise mmap fails.

I didn't test this code, but MAP_ANONYMOUS is wrong.  I'm trying to get
a file mapping here, not an anoymous mapping.

The intent is to hit the
        if (vm_flags & VM_HUGEPAGE) {
case in do_sync_mmap_readahead().

Ah, I see.

ksys_mmap_pgoff:
        if (!(flags & MAP_ANONYMOUS)) {
...
                if (is_file_hugepages(file)) {
                        len = ALIGN(len, huge_page_size(hstate_file(file)));
                } else if (unlikely(flags & MAP_HUGETLB)) {
                        retval = -EINVAL;
                        goto out_fput;
                }

Maybe we need something like this:

diff --git a/mm/mmap.c b/mm/mmap.c
index 83b4682ec85c..7c5066a8a3ac 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1307,6 +1307,8 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
 		flags_mask = LEGACY_MAP_MASK;
 		if (file->f_op->fop_flags & FOP_MMAP_SYNC)
 			flags_mask |= MAP_SYNC;
+		if (flags & MAP_HUGETLB)
+			vm_flags |= VM_HUGEPAGE;
 
 		switch (flags & MAP_TYPE) {
 		case MAP_SHARED:
@@ -1414,12 +1416,8 @@ unsigned long ksys_mmap_pgoff(unsigned long addr, unsigned long len,
 		file = fget(fd);
 		if (!file)
 			return -EBADF;
-		if (is_file_hugepages(file)) {
+		if (is_file_hugepages(file))
 			len = ALIGN(len, huge_page_size(hstate_file(file)));
-		} else if (unlikely(flags & MAP_HUGETLB)) {
-			retval = -EINVAL;
-			goto out_fput;
-		}
 	} else if (flags & MAP_HUGETLB) {
 		struct hstate *hs;
 
@@ -1441,7 +1439,6 @@ unsigned long ksys_mmap_pgoff(unsigned long addr, unsigned long len,
 	}
 
 	retval = vm_mmap_pgoff(file, addr, len, prot, flags, pgoff);
-out_fput:
 	if (file)
 		fput(file);
 	return retval;

(compile tested only)