| Message ID | 20240319092733.4501-2-ryncsn@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/filemap: optimize folio adding and splitting | expand |
On Tue, Mar 19, 2024 at 05:27:30PM +0800, Kairui Song wrote: > From: Kairui Song <kasong@tencent.com> > > xas_split_alloc could fail with NOMEM, and in such case, it should abort > early instead of keep going and fail the xas_split below. > > Signed-off-by: Kairui Song <kasong@tencent.com> The usual way of programming with an xa_state is to not do any error handling; the xas_ functions check for error and become no-ops, so we only have to check at the end. I think this case is worth making an exception for because we avoid acquiring/releasing the lock. Not that we should be getting ENOMEM very often; we only allocate about 8 nodes usually and we get 7 nodes/page. Acked-by: Matthew Wilcox (Oracle) <willy@infradead.org> > mm/filemap.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/mm/filemap.c b/mm/filemap.c > index 7437b2bd75c1..f07ea0b97698 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -880,9 +880,12 @@ noinline int __filemap_add_folio(struct address_space *mapping, > unsigned int order = xa_get_order(xas.xa, xas.xa_index); > void *entry, *old = NULL; > > - if (order > folio_order(folio)) > + if (order > folio_order(folio)) { > xas_split_alloc(&xas, xa_load(xas.xa, xas.xa_index), > order, gfp); > + if (xas_error(&xas)) > + goto error; > + } > xas_lock_irq(&xas); > xas_for_each_conflict(&xas, entry) { > old = entry; > -- > 2.43.0 >
diff --git a/mm/filemap.c b/mm/filemap.c index 7437b2bd75c1..f07ea0b97698 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -880,9 +880,12 @@ noinline int __filemap_add_folio(struct address_space *mapping, unsigned int order = xa_get_order(xas.xa, xas.xa_index); void *entry, *old = NULL; - if (order > folio_order(folio)) + if (order > folio_order(folio)) { xas_split_alloc(&xas, xa_load(xas.xa, xas.xa_index), order, gfp); + if (xas_error(&xas)) + goto error; + } xas_lock_irq(&xas); xas_for_each_conflict(&xas, entry) { old = entry;