process_madvise() calls do_madvise() for each address range.  Then, each
do_madvise() invocation holds and releases same mmap_lock.  Optimize the
redundant lock operations by splitting do_madvise() internal logics
including the mmap_lock operations, and calling the small logics
directly from process_madvise() in a sequence that removes the redundant
locking.

Changes from RFC v1 (20250111004618.1566-1-sj@kernel.org)
- Split out do_madvise() and use those from vector_madvise(), instead of
  adding a flag to do_madvise() (Liam R. Howlett)

SeongJae Park (4):
  mm/madvise: split out mmap locking operations for madvise()
  mm/madvise: split out madvise input validity check
  mm/madvise: split out madvise() behavior execution
  mm/madvise: remove redundant mmap_lock operations from
    process_madvise()

 mm/madvise.c | 150 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 103 insertions(+), 47 deletions(-)


base-commit: b43ba6938d01ad4487028592109d4116a28b7afa
-- 
2.39.5

* SeongJae Park <sj@kernel.org> [250116 20:31]:
> process_madvise() calls do_madvise() for each address range.  Then, each
> do_madvise() invocation holds and releases same mmap_lock.  Optimize the
> redundant lock operations by splitting do_madvise() internal logics
> including the mmap_lock operations, and calling the small logics
> directly from process_madvise() in a sequence that removes the redundant
> locking.
> 
> Changes from RFC v1 (20250111004618.1566-1-sj@kernel.org)
> - Split out do_madvise() and use those from vector_madvise(), instead of
>   adding a flag to do_madvise() (Liam R. Howlett)

I was waiting for a non-RFC to re-examine the series.  It looks like a
good clean up.

Do you think you'll send out a non-RFC version soon?

> 
> SeongJae Park (4):
>   mm/madvise: split out mmap locking operations for madvise()
>   mm/madvise: split out madvise input validity check
>   mm/madvise: split out madvise() behavior execution
>   mm/madvise: remove redundant mmap_lock operations from
>     process_madvise()
> 
>  mm/madvise.c | 150 +++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 103 insertions(+), 47 deletions(-)
> 
> 
> base-commit: b43ba6938d01ad4487028592109d4116a28b7afa
> -- 
> 2.39.5

On Fri, Jan 31, 2025 at 11:04:51AM -0500, Liam R. Howlett wrote:
> * SeongJae Park <sj@kernel.org> [250116 20:31]:
> > process_madvise() calls do_madvise() for each address range.  Then, each
> > do_madvise() invocation holds and releases same mmap_lock.  Optimize the
> > redundant lock operations by splitting do_madvise() internal logics
> > including the mmap_lock operations, and calling the small logics
> > directly from process_madvise() in a sequence that removes the redundant
> > locking.
> >
> > Changes from RFC v1 (20250111004618.1566-1-sj@kernel.org)
> > - Split out do_madvise() and use those from vector_madvise(), instead of
> >   adding a flag to do_madvise() (Liam R. Howlett)
>
> I was waiting for a non-RFC to re-examine the series.  It looks like a
> good clean up.
>
> Do you think you'll send out a non-RFC version soon?

This is definitely a great cleanup, there's a problem with patch 3/3, but
SJ - feel free to un-RFC with the fix I suggested - and then happy to give
R-b and T-b tags!

Thanks for doing this!

Cheers, Lorenzo

>
> >
> > SeongJae Park (4):
> >   mm/madvise: split out mmap locking operations for madvise()
> >   mm/madvise: split out madvise input validity check
> >   mm/madvise: split out madvise() behavior execution
> >   mm/madvise: remove redundant mmap_lock operations from
> >     process_madvise()
> >
> >  mm/madvise.c | 150 +++++++++++++++++++++++++++++++++++----------------
> >  1 file changed, 103 insertions(+), 47 deletions(-)
> >
> >
> > base-commit: b43ba6938d01ad4487028592109d4116a28b7afa
> > --
> > 2.39.5

On Fri, Jan 31, 2025 at 04:55:05PM +0000, Lorenzo Stoakes wrote:
> On Fri, Jan 31, 2025 at 11:04:51AM -0500, Liam R. Howlett wrote:
> > * SeongJae Park <sj@kernel.org> [250116 20:31]:
> > > process_madvise() calls do_madvise() for each address range.  Then, each
> > > do_madvise() invocation holds and releases same mmap_lock.  Optimize the
> > > redundant lock operations by splitting do_madvise() internal logics
> > > including the mmap_lock operations, and calling the small logics
> > > directly from process_madvise() in a sequence that removes the redundant
> > > locking.
> > >
> > > Changes from RFC v1 (20250111004618.1566-1-sj@kernel.org)
> > > - Split out do_madvise() and use those from vector_madvise(), instead of
> > >   adding a flag to do_madvise() (Liam R. Howlett)
> >
> > I was waiting for a non-RFC to re-examine the series.  It looks like a
> > good clean up.
> >
> > Do you think you'll send out a non-RFC version soon?
>
> This is definitely a great cleanup, there's a problem with patch 3/3, but
> SJ - feel free to un-RFC with the fix I suggested - and then happy to give
> R-b and T-b tags!

Liam's pedantry brought me here ;)

Obviously by 3/3 I mean 4/4 here.

Maybe not obviously. But anyway.

>
> Thanks for doing this!
>
> Cheers, Lorenzo
>
> >
> > >
> > > SeongJae Park (4):
> > >   mm/madvise: split out mmap locking operations for madvise()
> > >   mm/madvise: split out madvise input validity check
> > >   mm/madvise: split out madvise() behavior execution
> > >   mm/madvise: remove redundant mmap_lock operations from
> > >     process_madvise()
> > >
> > >  mm/madvise.c | 150 +++++++++++++++++++++++++++++++++++----------------
> > >  1 file changed, 103 insertions(+), 47 deletions(-)
> > >
> > >
> > > base-commit: b43ba6938d01ad4487028592109d4116a28b7afa
> > > --
> > > 2.39.5

On Fri, 31 Jan 2025 11:04:51 -0500 "Liam R. Howlett" <Liam.Howlett@oracle.com> wrote:

> * SeongJae Park <sj@kernel.org> [250116 20:31]:
> > process_madvise() calls do_madvise() for each address range.  Then, each
> > do_madvise() invocation holds and releases same mmap_lock.  Optimize the
> > redundant lock operations by splitting do_madvise() internal logics
> > including the mmap_lock operations, and calling the small logics
> > directly from process_madvise() in a sequence that removes the redundant
> > locking.
> > 
> > Changes from RFC v1 (20250111004618.1566-1-sj@kernel.org)
> > - Split out do_madvise() and use those from vector_madvise(), instead of
> >   adding a flag to do_madvise() (Liam R. Howlett)
> 
> I was waiting for a non-RFC to re-examine the series.  It looks like a
> good clean up.

Thank you Liam :)

> 
> Do you think you'll send out a non-RFC version soon?

I'm planning to send it next week.


Thanks,
SJ

> 
> > 
> > SeongJae Park (4):
> >   mm/madvise: split out mmap locking operations for madvise()
> >   mm/madvise: split out madvise input validity check
> >   mm/madvise: split out madvise() behavior execution
> >   mm/madvise: remove redundant mmap_lock operations from
> >     process_madvise()
> > 
> >  mm/madvise.c | 150 +++++++++++++++++++++++++++++++++++----------------
> >  1 file changed, 103 insertions(+), 47 deletions(-)
> > 
> > 
> > base-commit: b43ba6938d01ad4487028592109d4116a28b7afa
> > -- 
> > 2.39.5

On Thu, Jan 16, 2025 at 05:30:54PM -0800, SeongJae Park wrote:
> process_madvise() calls do_madvise() for each address range.  Then, each
> do_madvise() invocation holds and releases same mmap_lock.  Optimize the
> redundant lock operations by splitting do_madvise() internal logics
> including the mmap_lock operations, and calling the small logics
> directly from process_madvise() in a sequence that removes the redundant
> locking.
> 

You skipped the evaluation section which was present in the v1. If you
decide to post a new version, please include that otherwise we can ask
Andrew to pickup the evaluation section from the v1 [1].

[1] https://lore.kernel.org/all/20250111004618.1566-1-sj@kernel.org/

Hi Shakeel,

On Wed, 29 Jan 2025 11:22:29 -0800 Shakeel Butt <shakeel.butt@linux.dev> wrote:

> On Thu, Jan 16, 2025 at 05:30:54PM -0800, SeongJae Park wrote:
> > process_madvise() calls do_madvise() for each address range.  Then, each
> > do_madvise() invocation holds and releases same mmap_lock.  Optimize the
> > redundant lock operations by splitting do_madvise() internal logics
> > including the mmap_lock operations, and calling the small logics
> > directly from process_madvise() in a sequence that removes the redundant
> > locking.
> > 
> 
> You skipped the evaluation section which was present in the v1. If you
> decide to post a new version, please include that otherwise we can ask
> Andrew to pickup the evaluation section from the v1 [1].
> 
> [1] https://lore.kernel.org/all/20250111004618.1566-1-sj@kernel.org/

Sure.  I will run the evaluation again on the new version and include the
result on the next version.

Also, thank you very much for reviews :)


Thanks,
SJ