From: "Pankaj Raghav (Samsung)" <kernel@pankajraghav.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
gost.dev@samsung.com, chandan.babu@oracle.com, hare@suse.de,
mcgrof@kernel.org, djwong@kernel.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org, david@fromorbit.com,
akpm@linux-foundation.org, Pankaj Raghav <p.raghav@samsung.com>
Subject: Re: [PATCH v3 05/11] readahead: allocate folios with mapping_min_order in readahead
Date: Mon, 22 Apr 2024 13:03:45 +0200 [thread overview]
Message-ID: <i4c6xe6jdei2to6kah4kgjehpjlanaqfulju2jzsu5ny2gmegv@2b2oh44oilnj> (raw)
In-Reply-To: <ZgHJxiYHvN9DfD15@casper.infradead.org>
> > @@ -515,7 +562,7 @@ void page_cache_ra_order(struct readahead_control *ractl,
> > if (index & ((1UL << order) - 1))
> > order = __ffs(index);
> > /* Don't allocate pages past EOF */
> > - while (index + (1UL << order) - 1 > limit)
> > + while (order > min_order && index + (1UL << order) - 1 > limit)
> > order--;
>
> This raises an interesting question that I don't know if we have a test
> for. POSIX says that if we mmap, let's say, the first 16kB of a 10kB
> file, then we can store into offset 0-12287, but stores to offsets
> 12288-16383 get a signal (I forget if it's SEGV or BUS). Thus far,
> we've declined to even create folios in the page cache that would let us
> create PTEs for offset 12288-16383, so I haven't paid too much attention
> to this. Now we're going to have folios that extend into that range, so
> we need to be sure that when we mmap(), we only create PTEs that go as
> far as 12287.
>
> Can you check that we have such an fstest, and that we still pass it
> with your patches applied and a suitably large block size?
>
So the mmap is giving the correct SIGBUS error when we try to do this:
dd if=/dev/zero of=./test bs=10k count=1;
xfs_io -c "mmap -w 0 16384" -c "mwrite 13000 10" test
Logs on bs=64k ps=4k system:
root@debian:/media/test# dd if=/dev/zero of=./test bs=10k count=1;
root@debian:/media/test# du -sh test
64K test
root@debian:/media/test# ls -l --block-size=k test
-rw-r--r-- 1 root root 10K Apr 22 10:42 test
root@debian:/media/test# xfs_io -c "mmap 0 16384" -c "mwrite 13000 10" test
Bus error
The check in filemap_fault takes care of this:
max_idx = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE);
if (unlikely(index >= max_idx))
return VM_FAULT_SIGBUS;
The same operation for read should also give a bus error, but it didn't.
Further investigation pointed out that the fault_around() does not take
this condition into account for LBS configuration. When I set fault_around_bytes
to 4096, things worked as expected as we skip fault_around for reads.
I have a patch that return SIGBUS also for the following read operation:
dd if=/dev/zero of=./test bs=10k count=1;
xfs_io -c "mmap -r 0 16384" -c "mread 13000 10" test
This is the patch I have for now that fixes fault_around() logic for LBS
configuration:
diff --git a/mm/filemap.c b/mm/filemap.c
index f0c0cfbbd134..259531dd297b 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3600,12 +3600,15 @@ vm_fault_t filemap_map_pages(struct vm_fault *vmf,
}
do {
unsigned long end;
+ unsigned long i_size;
addr += (xas.xa_index - last_pgoff) << PAGE_SHIFT;
vmf->pte += xas.xa_index - last_pgoff;
last_pgoff = xas.xa_index;
end = folio_next_index(folio) - 1;
- nr_pages = min(end, end_pgoff) - xas.xa_index + 1;
+ i_size = DIV_ROUND_UP(i_size_read(mapping->host),
+ PAGE_SIZE) - 1;
+ nr_pages = min3(end, end_pgoff, i_size) - xas.xa_index + 1;
if (!folio_test_large(folio))
ret |= filemap_map_order0_folio(vmf,
I will send a new version of the series this week after doing some more
testing.
next prev parent reply other threads:[~2024-04-22 11:12 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-13 17:02 [PATCH v3 00/11] enable bs > ps in XFS Pankaj Raghav (Samsung)
2024-03-13 17:02 ` [PATCH v3 01/11] mm: Support order-1 folios in the page cache Pankaj Raghav (Samsung)
2024-03-13 17:02 ` [PATCH v3 02/11] fs: Allow fine-grained control of folio sizes Pankaj Raghav (Samsung)
2024-03-25 18:29 ` Matthew Wilcox
2024-03-26 8:44 ` Pankaj Raghav (Samsung)
2024-03-13 17:02 ` [PATCH v3 03/11] filemap: allocate mapping_min_order folios in the page cache Pankaj Raghav (Samsung)
2024-03-15 13:21 ` Pankaj Raghav (Samsung)
2024-03-13 17:02 ` [PATCH v3 04/11] readahead: rework loop in page_cache_ra_unbounded() Pankaj Raghav (Samsung)
2024-03-25 18:41 ` Matthew Wilcox
2024-03-26 8:56 ` Pankaj Raghav (Samsung)
2024-03-26 9:39 ` Hannes Reinecke
2024-03-26 9:44 ` Pankaj Raghav
2024-03-26 10:00 ` Hannes Reinecke
2024-03-26 10:06 ` Pankaj Raghav
2024-03-26 10:55 ` Hannes Reinecke
2024-03-26 13:41 ` Pankaj Raghav (Samsung)
2024-03-26 15:11 ` Pankaj Raghav (Samsung)
2024-03-13 17:02 ` [PATCH v3 05/11] readahead: allocate folios with mapping_min_order in readahead Pankaj Raghav (Samsung)
2024-03-25 19:00 ` Matthew Wilcox
2024-03-26 13:08 ` Pankaj Raghav (Samsung)
2024-04-22 11:03 ` Pankaj Raghav (Samsung) [this message]
2024-03-13 17:02 ` [PATCH v3 06/11] readahead: round up file_ra_state->ra_pages to mapping_min_nrpages Pankaj Raghav (Samsung)
2024-03-13 17:02 ` [PATCH v3 07/11] mm: do not split a folio if it has minimum folio order requirement Pankaj Raghav (Samsung)
2024-03-25 19:06 ` Matthew Wilcox
2024-03-26 16:10 ` Pankaj Raghav (Samsung)
2024-03-26 16:23 ` Zi Yan
2024-03-26 16:33 ` Pankaj Raghav
2024-03-26 16:38 ` Zi Yan
2024-03-13 17:02 ` [PATCH v3 08/11] iomap: fix iomap_dio_zero() for fs bs > system page size Pankaj Raghav (Samsung)
2024-03-13 17:02 ` [PATCH v3 09/11] xfs: expose block size in stat Pankaj Raghav (Samsung)
2024-03-13 17:02 ` [PATCH v3 10/11] xfs: make the calculation generic in xfs_sb_validate_fsb_count() Pankaj Raghav (Samsung)
2024-03-25 19:15 ` Matthew Wilcox
2024-03-26 9:53 ` Pankaj Raghav (Samsung)
2024-03-13 17:02 ` [PATCH v3 11/11] xfs: enable block size larger than page size support Pankaj Raghav (Samsung)
2024-03-25 19:19 ` [PATCH v3 00/11] enable bs > ps in XFS Matthew Wilcox
2024-03-26 9:53 ` Hannes Reinecke
2024-03-26 15:06 ` Pankaj Raghav
2024-03-26 14:54 ` Pankaj Raghav (Samsung)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=i4c6xe6jdei2to6kah4kgjehpjlanaqfulju2jzsu5ny2gmegv@2b2oh44oilnj \
--to=kernel@pankajraghav.com \
--cc=akpm@linux-foundation.org \
--cc=chandan.babu@oracle.com \
--cc=david@fromorbit.com \
--cc=djwong@kernel.org \
--cc=gost.dev@samsung.com \
--cc=hare@suse.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-xfs@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=p.raghav@samsung.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).