[v2,0/4] Remove get_kernel_pages()

  Sumit,

I did not see a follow up on this series per your last email.[1]  I'd like to
move forward with getting rid of kmap_to_page().  So Hopefully this can land
and you can build on this rather than the other way around?

All,

Al Viro found[2] that kmap_to_page() is broken.  But not only is it broken, it
presents confusion over how highmem should be used because kmap() and friends
should not be used for 'long term' mappings.

get_kernel_pages() is a caller of kmap_to_page().  It only has one caller
[shm_get_kernel_pages()] which does not need the functionality.

Alter shm_get_kernel_pages() to no longer call get_kernel_pages() and remove
get_kernel_pages().  Along the way it was noted that shm_get_kernel_pages()
does not have any need to support vmalloc'ed addresses either.  Remove that
functionality to clean up the logic.

This series also fixes is_kmap_addr() and uses it to ensure no kmap addresses
slip in later.

[1] https://lore.kernel.org/all/CAFA6WYMqEVDVW-ifoh-V9ni1zntYdes8adQKf2XXAUpqdaW53w@mail.gmail.com/
[2] https://lore.kernel.org/lkml/YzSSl1ItVlARDvG3@ZenIV

To: Sumit Garg <sumit.garg@linaro.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: "Al Viro" <viro@zeniv.linux.org.uk>
Cc: "Christoph Hellwig" <hch@lst.de>
Cc: linux-kernel@vger.kernel.org
Cc: op-tee@lists.trustedfirmware.org
Cc: linux-mm@kvack.org
Cc: Jens Wiklander <jens.wiklander@linaro.org>
Cc: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
Changes in v2:
- Al Viro: Avoid allocating the kiov.
- Sumit: Update cover letter to clarify the motivation behind removing
  get_kernel_pages()
- Link to v1: https://lore.kernel.org/r/20221002002326.946620-1-ira.weiny@intel.com

---
Ira Weiny (4):
      highmem: Enhance is_kmap_addr() to check kmap_local_page() mappings
      tee: Remove vmalloc page support
      tee: Remove call to get_kernel_pages()
      mm: Remove get_kernel_pages()

 drivers/tee/tee_shm.c            | 37 ++++++++++---------------------------
 include/linux/highmem-internal.h |  5 ++++-
 include/linux/mm.h               |  2 --
 mm/swap.c                        | 30 ------------------------------
 4 files changed, 14 insertions(+), 60 deletions(-)
---
base-commit: 0136d86b78522bbd5755f8194c97a987f0586ba5
change-id: 20230203-get_kernel_pages-199342cfba79

Best regards,
  

On Fri, Feb 3, 2023 at 8:06 PM Ira Weiny <ira.weiny@intel.com> wrote:
>
> This series also fixes is_kmap_addr() and uses it to ensure no kmap addresses
> slip in later.

Ack. Please make it so.

That said...

I wasn't cc'd on all the patches, but checked them on the mailing
list, and that first is_kmap_addr() patch makes me a bit unhappy.

Right now that 'is_kmap_addr()' is only used for user copy addresses,
for debugging purposes, and I'm not exactly thrilled about extending
it this way.

I get the feeling that we should just have a name for that "kmap _or_
kmap_local" range instead of making it two ranges.

But admittedly I can't come up with anything better, and it looks like
different architectures may do different things.  I just don't like
it.

                Linus
  

On Sat, 4 Feb 2023 at 09:36, Ira Weiny <ira.weiny@intel.com> wrote:
>
> Sumit,
>
> I did not see a follow up on this series per your last email.[1]  I'd like to
> move forward with getting rid of kmap_to_page().  So Hopefully this can land
> and you can build on this rather than the other way around?

Apologies Ira for keeping you waiting. Actually I was fully involved
with other high priority work with my upstream review backlog
increasing. So I wasn't able to devote time to this work. Sure I will
rebase my work on top of your changes.

-Sumit

>
> All,
>
> Al Viro found[2] that kmap_to_page() is broken.  But not only is it broken, it
> presents confusion over how highmem should be used because kmap() and friends
> should not be used for 'long term' mappings.
>
> get_kernel_pages() is a caller of kmap_to_page().  It only has one caller
> [shm_get_kernel_pages()] which does not need the functionality.
>
> Alter shm_get_kernel_pages() to no longer call get_kernel_pages() and remove
> get_kernel_pages().  Along the way it was noted that shm_get_kernel_pages()
> does not have any need to support vmalloc'ed addresses either.  Remove that
> functionality to clean up the logic.
>
> This series also fixes is_kmap_addr() and uses it to ensure no kmap addresses
> slip in later.
>
> [1] https://lore.kernel.org/all/CAFA6WYMqEVDVW-ifoh-V9ni1zntYdes8adQKf2XXAUpqdaW53w@mail.gmail.com/
> [2] https://lore.kernel.org/lkml/YzSSl1ItVlARDvG3@ZenIV
>
> To: Sumit Garg <sumit.garg@linaro.org>
> To: Andrew Morton <akpm@linux-foundation.org>
> Cc: "Al Viro" <viro@zeniv.linux.org.uk>
> Cc: "Christoph Hellwig" <hch@lst.de>
> Cc: linux-kernel@vger.kernel.org
> Cc: op-tee@lists.trustedfirmware.org
> Cc: linux-mm@kvack.org
> Cc: Jens Wiklander <jens.wiklander@linaro.org>
> Cc: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>
> ---
> Changes in v2:
> - Al Viro: Avoid allocating the kiov.
> - Sumit: Update cover letter to clarify the motivation behind removing
>   get_kernel_pages()
> - Link to v1: https://lore.kernel.org/r/20221002002326.946620-1-ira.weiny@intel.com
>
> ---
> Ira Weiny (4):
>       highmem: Enhance is_kmap_addr() to check kmap_local_page() mappings
>       tee: Remove vmalloc page support
>       tee: Remove call to get_kernel_pages()
>       mm: Remove get_kernel_pages()
>
>  drivers/tee/tee_shm.c            | 37 ++++++++++---------------------------
>  include/linux/highmem-internal.h |  5 ++++-
>  include/linux/mm.h               |  2 --
>  mm/swap.c                        | 30 ------------------------------
>  4 files changed, 14 insertions(+), 60 deletions(-)
> ---
> base-commit: 0136d86b78522bbd5755f8194c97a987f0586ba5
> change-id: 20230203-get_kernel_pages-199342cfba79
>
> Best regards,
> --
> Ira Weiny <ira.weiny@intel.com>
  

Sumit Garg wrote:
> On Sat, 4 Feb 2023 at 09:36, Ira Weiny <ira.weiny@intel.com> wrote:
> >
> > Sumit,
> >
> > I did not see a follow up on this series per your last email.[1]  I'd like to
> > move forward with getting rid of kmap_to_page().  So Hopefully this can land
> > and you can build on this rather than the other way around?
> 
> Apologies Ira for keeping you waiting. Actually I was fully involved
> with other high priority work with my upstream review backlog
> increasing. So I wasn't able to devote time to this work. Sure I will
> rebase my work on top of your changes.

No problem on my end.  I just wanted to ensure that I did not miss
something.

Thanks for the reviews!
Ira

> 
> -Sumit
> 
> >
> > All,
> >
> > Al Viro found[2] that kmap_to_page() is broken.  But not only is it broken, it
> > presents confusion over how highmem should be used because kmap() and friends
> > should not be used for 'long term' mappings.
> >
> > get_kernel_pages() is a caller of kmap_to_page().  It only has one caller
> > [shm_get_kernel_pages()] which does not need the functionality.
> >
> > Alter shm_get_kernel_pages() to no longer call get_kernel_pages() and remove
> > get_kernel_pages().  Along the way it was noted that shm_get_kernel_pages()
> > does not have any need to support vmalloc'ed addresses either.  Remove that
> > functionality to clean up the logic.
> >
> > This series also fixes is_kmap_addr() and uses it to ensure no kmap addresses
> > slip in later.
> >
> > [1] https://lore.kernel.org/all/CAFA6WYMqEVDVW-ifoh-V9ni1zntYdes8adQKf2XXAUpqdaW53w@mail.gmail.com/
> > [2] https://lore.kernel.org/lkml/YzSSl1ItVlARDvG3@ZenIV
> >
> > To: Sumit Garg <sumit.garg@linaro.org>
> > To: Andrew Morton <akpm@linux-foundation.org>
> > Cc: "Al Viro" <viro@zeniv.linux.org.uk>
> > Cc: "Christoph Hellwig" <hch@lst.de>
> > Cc: linux-kernel@vger.kernel.org
> > Cc: op-tee@lists.trustedfirmware.org
> > Cc: linux-mm@kvack.org
> > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > Cc: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> >
> > ---
> > Changes in v2:
> > - Al Viro: Avoid allocating the kiov.
> > - Sumit: Update cover letter to clarify the motivation behind removing
> >   get_kernel_pages()
> > - Link to v1: https://lore.kernel.org/r/20221002002326.946620-1-ira.weiny@intel.com
> >
> > ---
> > Ira Weiny (4):
> >       highmem: Enhance is_kmap_addr() to check kmap_local_page() mappings
> >       tee: Remove vmalloc page support
> >       tee: Remove call to get_kernel_pages()
> >       mm: Remove get_kernel_pages()
> >
> >  drivers/tee/tee_shm.c            | 37 ++++++++++---------------------------
> >  include/linux/highmem-internal.h |  5 ++++-
> >  include/linux/mm.h               |  2 --
> >  mm/swap.c                        | 30 ------------------------------
> >  4 files changed, 14 insertions(+), 60 deletions(-)
> > ---
> > base-commit: 0136d86b78522bbd5755f8194c97a987f0586ba5
> > change-id: 20230203-get_kernel_pages-199342cfba79
> >
> > Best regards,
> > --
> > Ira Weiny <ira.weiny@intel.com>
  

Ira Weiny wrote:
> Sumit Garg wrote:
> > On Sat, 4 Feb 2023 at 09:36, Ira Weiny <ira.weiny@intel.com> wrote:
> > >
> > > Sumit,
> > >
> > > I did not see a follow up on this series per your last email.[1]  I'd like to
> > > move forward with getting rid of kmap_to_page().  So Hopefully this can land
> > > and you can build on this rather than the other way around?
> > 
> > Apologies Ira for keeping you waiting. Actually I was fully involved
> > with other high priority work with my upstream review backlog
> > increasing. So I wasn't able to devote time to this work. Sure I will
> > rebase my work on top of your changes.
> 
> No problem on my end.  I just wanted to ensure that I did not miss
> something.

Andrew, can I get an ack on patches 1 and 4 for this series?  I realized
that perhaps I was not clear on my expectations of this series.  I was
thinking this would be easiest to go through the tee subsystem tree.

Sumit or Jens, is that ok with you all?

Thanks,
Ira
  

Hi Ira,

On Fri, Feb 10, 2023 at 9:28 PM Ira Weiny <ira.weiny@intel.com> wrote:
>
> Ira Weiny wrote:
> > Sumit Garg wrote:
> > > On Sat, 4 Feb 2023 at 09:36, Ira Weiny <ira.weiny@intel.com> wrote:
> > > >
> > > > Sumit,
> > > >
> > > > I did not see a follow up on this series per your last email.[1]  I'd like to
> > > > move forward with getting rid of kmap_to_page().  So Hopefully this can land
> > > > and you can build on this rather than the other way around?
> > >
> > > Apologies Ira for keeping you waiting. Actually I was fully involved
> > > with other high priority work with my upstream review backlog
> > > increasing. So I wasn't able to devote time to this work. Sure I will
> > > rebase my work on top of your changes.
> >
> > No problem on my end.  I just wanted to ensure that I did not miss
> > something.
>
> Andrew, can I get an ack on patches 1 and 4 for this series?  I realized
> that perhaps I was not clear on my expectations of this series.  I was
> thinking this would be easiest to go through the tee subsystem tree.
>
> Sumit or Jens, is that ok with you all?

Sure, I'll take it. The timing is a bit unfortunate, it's likely too
close to the merge window to be included there. However, I'll pick it
up and add it to linux-next so it's ready for the 6.4 merge window.

Thanks,
Jens
  

Jens Wiklander wrote:
> Hi Ira,
> 
> On Fri, Feb 10, 2023 at 9:28 PM Ira Weiny <ira.weiny@intel.com> wrote:
> >
> > Ira Weiny wrote:
> > > Sumit Garg wrote:
> > > > On Sat, 4 Feb 2023 at 09:36, Ira Weiny <ira.weiny@intel.com> wrote:
> > > > >
> > > > > Sumit,
> > > > >
> > > > > I did not see a follow up on this series per your last email.[1]  I'd like to
> > > > > move forward with getting rid of kmap_to_page().  So Hopefully this can land
> > > > > and you can build on this rather than the other way around?
> > > >
> > > > Apologies Ira for keeping you waiting. Actually I was fully involved
> > > > with other high priority work with my upstream review backlog
> > > > increasing. So I wasn't able to devote time to this work. Sure I will
> > > > rebase my work on top of your changes.
> > >
> > > No problem on my end.  I just wanted to ensure that I did not miss
> > > something.
> >
> > Andrew, can I get an ack on patches 1 and 4 for this series?  I realized
> > that perhaps I was not clear on my expectations of this series.  I was
> > thinking this would be easiest to go through the tee subsystem tree.
> >
> > Sumit or Jens, is that ok with you all?
> 
> Sure, I'll take it. The timing is a bit unfortunate, it's likely too
> close to the merge window to be included there. However, I'll pick it
> up and add it to linux-next so it's ready for the 6.4 merge window.

6.4 is fine with me.

Thanks everyone!
Ira
  

On Mon, Feb 13, 2023 at 7:02 AM Jens Wiklander
<jens.wiklander@linaro.org> wrote:
>
> Sure, I'll take it. The timing is a bit unfortunate, it's likely too
> close to the merge window to be included there. However, I'll pick it
> up and add it to linux-next so it's ready for the 6.4 merge window.

With this boeing almost all code removal, I'm perfectly fine taking it
in the upcoming merge window.

           Linus
  

On Mon, Feb 13, 2023 at 11:02:52AM -0800, Linus Torvalds wrote:
> On Mon, Feb 13, 2023 at 7:02 AM Jens Wiklander
> <jens.wiklander@linaro.org> wrote:
> >
> > Sure, I'll take it. The timing is a bit unfortunate, it's likely too
> > close to the merge window to be included there. However, I'll pick it
> > up and add it to linux-next so it's ready for the 6.4 merge window.
> 
> With this boeing almost all code removal, I'm perfectly fine taking it
> in the upcoming merge window.

OK, thank you.

Cheers,
Jens