* [PATCH] mm/truncate: reset xa_has_values flag on each iteration
@ 2024-10-02 22:51 Shakeel Butt
2024-10-02 22:55 ` Andrew Morton
2024-10-06 22:03 ` Yu Zhao
0 siblings, 2 replies; 6+ messages in thread
From: Shakeel Butt @ 2024-10-02 22:51 UTC (permalink / raw)
To: Andrew Morton
Cc: Johannes Weiner, Matthew Wilcox, Yu Zhao, linux-fsdevel, linux-mm,
linux-kernel, Meta kernel team
Currently mapping_try_invalidate() and invalidate_inode_pages2_range()
traverses the xarray in batches and then for each batch, maintains and
set the flag named xa_has_values if the batch has a shadow entry to
clear the entries at the end of the iteration. However they forgot to
reset the flag at the end of the iteration which cause them to always
try to clear the shadow entries in the subsequent iterations where
there might not be any shadow entries. Fixing it.
Fixes: 61c663e020d2 ("mm/truncate: batch-clear shadow entries")
Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
mm/truncate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/truncate.c b/mm/truncate.c
index 520c8cf8f58f..e5151703ba04 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -463,10 +463,10 @@ unsigned long mapping_try_invalidate(struct address_space *mapping,
unsigned long ret;
unsigned long count = 0;
int i;
- bool xa_has_values = false;
folio_batch_init(&fbatch);
while (find_lock_entries(mapping, &index, end, &fbatch, indices)) {
+ bool xa_has_values = false;
int nr = folio_batch_count(&fbatch);
for (i = 0; i < nr; i++) {
@@ -592,7 +592,6 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
int ret = 0;
int ret2 = 0;
int did_range_unmap = 0;
- bool xa_has_values = false;
if (mapping_empty(mapping))
return 0;
@@ -600,6 +599,7 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
folio_batch_init(&fbatch);
index = start;
while (find_get_entries(mapping, &index, end, &fbatch, indices)) {
+ bool xa_has_values = false;
int nr = folio_batch_count(&fbatch);
for (i = 0; i < nr; i++) {
--
2.43.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] mm/truncate: reset xa_has_values flag on each iteration
2024-10-02 22:51 [PATCH] mm/truncate: reset xa_has_values flag on each iteration Shakeel Butt
@ 2024-10-02 22:55 ` Andrew Morton
2024-10-02 23:09 ` Shakeel Butt
2024-10-06 22:03 ` Yu Zhao
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2024-10-02 22:55 UTC (permalink / raw)
To: Shakeel Butt
Cc: Johannes Weiner, Matthew Wilcox, Yu Zhao, linux-fsdevel, linux-mm,
linux-kernel, Meta kernel team
On Wed, 2 Oct 2024 15:51:50 -0700 Shakeel Butt <shakeel.butt@linux.dev> wrote:
> Currently mapping_try_invalidate() and invalidate_inode_pages2_range()
> traverses the xarray in batches and then for each batch, maintains and
> set the flag named xa_has_values if the batch has a shadow entry to
> clear the entries at the end of the iteration. However they forgot to
> reset the flag at the end of the iteration which cause them to always
> try to clear the shadow entries in the subsequent iterations where
> there might not be any shadow entries. Fixing it.
>
So this is an efficiency thing, no other effects expected?
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -463,10 +463,10 @@ unsigned long mapping_try_invalidate(struct address_space *mapping,
> unsigned long ret;
> unsigned long count = 0;
> int i;
> - bool xa_has_values = false;
>
> folio_batch_init(&fbatch);
> while (find_lock_entries(mapping, &index, end, &fbatch, indices)) {
> + bool xa_has_values = false;
> int nr = folio_batch_count(&fbatch);
>
> for (i = 0; i < nr; i++) {
> @@ -592,7 +592,6 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
> int ret = 0;
> int ret2 = 0;
> int did_range_unmap = 0;
> - bool xa_has_values = false;
>
> if (mapping_empty(mapping))
> return 0;
> @@ -600,6 +599,7 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
> folio_batch_init(&fbatch);
> index = start;
> while (find_get_entries(mapping, &index, end, &fbatch, indices)) {
> + bool xa_has_values = false;
> int nr = folio_batch_count(&fbatch);
>
> for (i = 0; i < nr; i++) {
> --
> 2.43.5
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm/truncate: reset xa_has_values flag on each iteration
2024-10-02 22:55 ` Andrew Morton
@ 2024-10-02 23:09 ` Shakeel Butt
2024-10-03 20:01 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Shakeel Butt @ 2024-10-02 23:09 UTC (permalink / raw)
To: Andrew Morton
Cc: Johannes Weiner, Matthew Wilcox, Yu Zhao, linux-fsdevel, linux-mm,
linux-kernel, Meta kernel team
On Wed, Oct 02, 2024 at 03:55:55PM GMT, Andrew Morton wrote:
> On Wed, 2 Oct 2024 15:51:50 -0700 Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> > Currently mapping_try_invalidate() and invalidate_inode_pages2_range()
> > traverses the xarray in batches and then for each batch, maintains and
> > set the flag named xa_has_values if the batch has a shadow entry to
> > clear the entries at the end of the iteration. However they forgot to
> > reset the flag at the end of the iteration which cause them to always
> > try to clear the shadow entries in the subsequent iterations where
> > there might not be any shadow entries. Fixing it.
> >
>
> So this is an efficiency thing, no other effects expected?
>
Correct, just an efficiency thing.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/truncate: reset xa_has_values flag on each iteration
2024-10-02 23:09 ` Shakeel Butt
@ 2024-10-03 20:01 ` Andrew Morton
2024-10-03 20:11 ` Shakeel Butt
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2024-10-03 20:01 UTC (permalink / raw)
To: Shakeel Butt
Cc: Johannes Weiner, Matthew Wilcox, Yu Zhao, linux-fsdevel, linux-mm,
linux-kernel, Meta kernel team
On Wed, 2 Oct 2024 16:09:11 -0700 Shakeel Butt <shakeel.butt@linux.dev> wrote:
> On Wed, Oct 02, 2024 at 03:55:55PM GMT, Andrew Morton wrote:
> > On Wed, 2 Oct 2024 15:51:50 -0700 Shakeel Butt <shakeel.butt@linux.dev> wrote:
> >
> > > Currently mapping_try_invalidate() and invalidate_inode_pages2_range()
> > > traverses the xarray in batches and then for each batch, maintains and
> > > set the flag named xa_has_values if the batch has a shadow entry to
> > > clear the entries at the end of the iteration. However they forgot to
> > > reset the flag at the end of the iteration which cause them to always
> > > try to clear the shadow entries in the subsequent iterations where
> > > there might not be any shadow entries. Fixing it.
> > >
> >
> > So this is an efficiency thing, no other effects expected?
> >
>
> Correct, just an efficiency thing.
Thanks. I'm assuming the benfits are sufficiently small that a
backport is inappropriate.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/truncate: reset xa_has_values flag on each iteration
2024-10-03 20:01 ` Andrew Morton
@ 2024-10-03 20:11 ` Shakeel Butt
0 siblings, 0 replies; 6+ messages in thread
From: Shakeel Butt @ 2024-10-03 20:11 UTC (permalink / raw)
To: Andrew Morton
Cc: Johannes Weiner, Matthew Wilcox, Yu Zhao, linux-fsdevel, linux-mm,
linux-kernel, Meta kernel team
On Thu, Oct 03, 2024 at 01:01:33PM GMT, Andrew Morton wrote:
> On Wed, 2 Oct 2024 16:09:11 -0700 Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> > On Wed, Oct 02, 2024 at 03:55:55PM GMT, Andrew Morton wrote:
> > > On Wed, 2 Oct 2024 15:51:50 -0700 Shakeel Butt <shakeel.butt@linux.dev> wrote:
> > >
> > > > Currently mapping_try_invalidate() and invalidate_inode_pages2_range()
> > > > traverses the xarray in batches and then for each batch, maintains and
> > > > set the flag named xa_has_values if the batch has a shadow entry to
> > > > clear the entries at the end of the iteration. However they forgot to
> > > > reset the flag at the end of the iteration which cause them to always
> > > > try to clear the shadow entries in the subsequent iterations where
> > > > there might not be any shadow entries. Fixing it.
> > > >
> > >
> > > So this is an efficiency thing, no other effects expected?
> > >
> >
> > Correct, just an efficiency thing.
>
> Thanks. I'm assuming the benfits are sufficiently small that a
> backport is inappropriate.
I agree.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/truncate: reset xa_has_values flag on each iteration
2024-10-02 22:51 [PATCH] mm/truncate: reset xa_has_values flag on each iteration Shakeel Butt
2024-10-02 22:55 ` Andrew Morton
@ 2024-10-06 22:03 ` Yu Zhao
1 sibling, 0 replies; 6+ messages in thread
From: Yu Zhao @ 2024-10-06 22:03 UTC (permalink / raw)
To: Shakeel Butt
Cc: Andrew Morton, Johannes Weiner, Matthew Wilcox, linux-fsdevel,
linux-mm, linux-kernel, Meta kernel team
On Wed, Oct 2, 2024 at 4:52 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> Currently mapping_try_invalidate() and invalidate_inode_pages2_range()
> traverses the xarray in batches and then for each batch, maintains and
> set the flag named xa_has_values if the batch has a shadow entry to
> clear the entries at the end of the iteration. However they forgot to
> reset the flag at the end of the iteration which cause them to always
> try to clear the shadow entries in the subsequent iterations where
> there might not be any shadow entries. Fixing it.
>
> Fixes: 61c663e020d2 ("mm/truncate: batch-clear shadow entries")
> Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
Acked-by: Yu Zhao <yuzhao@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-10-06 22:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-02 22:51 [PATCH] mm/truncate: reset xa_has_values flag on each iteration Shakeel Butt
2024-10-02 22:55 ` Andrew Morton
2024-10-02 23:09 ` Shakeel Butt
2024-10-03 20:01 ` Andrew Morton
2024-10-03 20:11 ` Shakeel Butt
2024-10-06 22:03 ` Yu Zhao
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 NNTP newsgroup(s).