| Message ID | 20250220204033.284730-1-kent.overstreet@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | filemap: check for folio order < mapping min order | expand |
On Thu, Feb 20, 2025 at 03:40:33PM -0500, Kent Overstreet wrote: > accidentally inserting a folio < mapping min order causes the readahead > code to go into an infinite loop, which is unpleasant. We already have: VM_BUG_ON_FOLIO(folio_order(folio) < mapping_min_folio_order(mapping), folio); If you're not enabling CONFIG_DEBUG_VM in your testing, I think that's an oversight on your part.
On Thu, Feb 20, 2025 at 11:24:26PM +0000, Matthew Wilcox wrote: > On Thu, Feb 20, 2025 at 03:40:33PM -0500, Kent Overstreet wrote: > > accidentally inserting a folio < mapping min order causes the readahead > > code to go into an infinite loop, which is unpleasant. > > We already have: > > VM_BUG_ON_FOLIO(folio_order(folio) < mapping_min_folio_order(mapping), > folio); WARN_ON() and returning an error is a bit more appropriate here, don't you think? > If you're not enabling CONFIG_DEBUG_VM in your testing, I think that's > an oversight on your part. Fastpath and expensive asserts should certainly be behind a config option, but I don't think this one should be. There's too many subsystem specific debug modes to test the full matrix of all of them, which means there are things that make it past our testing and we need to be able to catch them in the wild or on realistic test configurations. In practice, subsystems need to have a subset of judiciously chosen asserts that are always on.
diff --git a/mm/filemap.c b/mm/filemap.c index 804d7365680c..b88bed922fff 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -973,6 +973,11 @@ int filemap_add_folio(struct address_space *mapping, struct folio *folio, void *shadow = NULL; int ret; + if (WARN(folio_order(folio) < mapping_min_folio_order(mapping), + "folio order %u < mapping min order %u\n", + folio_order(folio), mapping_min_folio_order(mapping))) + return -EINVAL; + ret = mem_cgroup_charge(folio, NULL, gfp); if (ret) return ret;
accidentally inserting a folio < mapping min order causes the readahead code to go into an infinite loop, which is unpleasant. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> --- mm/filemap.c | 5 +++++ 1 file changed, 5 insertions(+)