| Message ID | 20230713172636.1705415-1-shikemeng@huaweicloud.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Remove unused parameters in page_table_check | expand |
On Thu, Jul 13, 2023 at 5:25 AM Kemeng Shi <shikemeng@huaweicloud.com> wrote: > > Hi all, this series remove unused parameters in functions from > page_table_check. The first 2 patches remove unused mm and addr > parameters in static common functions page_table_check_clear and > page_table_check_set. The last 6 patches remove unused addr parameter > in some externed functions which only need addr for cleaned > page_table_check_clear or page_table_check_set. There is no intended > functional change. Thanks! NAK Both, mm and addr are common arguments that are used for PTE handling in many parts of memory management even when they are not used in every function. Currently, they are not used in page table check, but it is possible we may need to use them in the future when support for other arches or different types of page tables (i.e. extended page table) is added. It is going to be hard to again modify all arch dependent code to add these arguments back. Also, internally at Google we are using these arguments, as we have a module that maps user memory in a way that is incompatible with upstream, and these arguments are used to support this module. Thank you, Pasha
On Sat, 22 Jul 2023 17:48:31 -0400 Pasha Tatashin <pasha.tatashin@soleen.com> wrote: > On Thu, Jul 13, 2023 at 5:25 AM Kemeng Shi <shikemeng@huaweicloud.com> wrote: > > > > Hi all, this series remove unused parameters in functions from > > page_table_check. The first 2 patches remove unused mm and addr > > parameters in static common functions page_table_check_clear and > > page_table_check_set. The last 6 patches remove unused addr parameter > > in some externed functions which only need addr for cleaned > > page_table_check_clear or page_table_check_set. There is no intended > > functional change. Thanks! > > NAK > > Both, mm and addr are common arguments that are used for PTE handling > in many parts of memory management even when they are not used in > every function. > > Currently, they are not used in page table check, but it is possible > we may need to use them in the future when support for other arches or > different types of page tables (i.e. extended page table) is added. It > is going to be hard to again modify all arch dependent code to add > these arguments back. > > Also, internally at Google we are using these arguments, as we have a > module that maps user memory in a way that is incompatible with > upstream, and these arguments are used to support this module. > I don't think these are very good arguments for carrying cruft in the mainline kernel. If such an architecture is introduced in the future or if google upstreams that module then we can restore one or both of these arguments at that time. This is hardly insurmountable: arch/arm64/include/asm/pgtable.h | 12 +++--- arch/riscv/include/asm/pgtable.h | 12 +++--- arch/x86/include/asm/pgtable.h | 16 ++++---- include/linux/page_table_check.h | 66 ++++++++++++-------------------- include/linux/pgtable.h | 6 +-- mm/page_table_check.c | 50 +++++++++--------------- 6 files changed, 65 insertions(+), 97 deletions(-)
On Mon, Jul 24, 2023 at 1:02 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > On Sat, 22 Jul 2023 17:48:31 -0400 Pasha Tatashin <pasha.tatashin@soleen.com> wrote: > > > On Thu, Jul 13, 2023 at 5:25 AM Kemeng Shi <shikemeng@huaweicloud.com> wrote: > > > > > > Hi all, this series remove unused parameters in functions from > > > page_table_check. The first 2 patches remove unused mm and addr > > > parameters in static common functions page_table_check_clear and > > > page_table_check_set. The last 6 patches remove unused addr parameter > > > in some externed functions which only need addr for cleaned > > > page_table_check_clear or page_table_check_set. There is no intended > > > functional change. Thanks! > > > > NAK > > > > Both, mm and addr are common arguments that are used for PTE handling > > in many parts of memory management even when they are not used in > > every function. > > > > Currently, they are not used in page table check, but it is possible > > we may need to use them in the future when support for other arches or > > different types of page tables (i.e. extended page table) is added. It > > is going to be hard to again modify all arch dependent code to add > > these arguments back. > > > > Also, internally at Google we are using these arguments, as we have a > > module that maps user memory in a way that is incompatible with > > upstream, and these arguments are used to support this module. > > > > I don't think these are very good arguments for carrying cruft in the > mainline kernel. > > If such an architecture is introduced in the future or if google > upstreams that module then we can restore one or both of these > arguments at that time. This is hardly insurmountable: There is another argument: as a follow-up to https://lore.kernel.org/linux-mm/20230722231508.1030269-1-pasha.tatashin@soleen.com/, I plan to improve the debugging message instead of simple WARN_ON(), I want to print more information about what went wrong, what it means, and what was expected behaviour. Having more data about the mm, and the addr, is going to be useful for this extended warning message. set_p**_at functions have prototype like this: set_p**_at(mm, addr, ptep, pte), in these functions mm, and addr are also not used, as they call something like: set_pte(ptep, pte); directly, yet having access to mm, and addr is useful during debugging instead of rewriting them all over the kernel. Thank you, Pasha > > arch/arm64/include/asm/pgtable.h | 12 +++--- > arch/riscv/include/asm/pgtable.h | 12 +++--- > arch/x86/include/asm/pgtable.h | 16 ++++---- > include/linux/page_table_check.h | 66 ++++++++++++-------------------- > include/linux/pgtable.h | 6 +-- > mm/page_table_check.c | 50 +++++++++--------------- > 6 files changed, 65 insertions(+), 97 deletions(-) > >
Hi all, this series remove unused parameters in functions from page_table_check. The first 2 patches remove unused mm and addr parameters in static common functions page_table_check_clear and page_table_check_set. The last 6 patches remove unused addr parameter in some externed functions which only need addr for cleaned page_table_check_clear or page_table_check_set. There is no intended functional change. Thanks! Kemeng Shi (8): mm/page_table_check: remove unused parameters in page_table_check_clear() mm/page_table_check: remove unused parameters in page_table_check_set() mm/page_table_check: remove unused parameter in [__]page_table_check_pte_clear mm/page_table_check: remove unused parameter in [__]page_table_check_pmd_clear mm/page_table_check: remove unused parameter in [__]page_table_check_pud_clear mm/page_table_check: remove unused parameter in [__]page_table_check_pte_set mm/page_table_check: remove unused parameter in [__]page_table_check_pmd_set mm/page_table_check: remove unused parameter in [__]page_table_check_pud_set arch/arm64/include/asm/pgtable.h | 12 +++--- arch/riscv/include/asm/pgtable.h | 12 +++--- arch/x86/include/asm/pgtable.h | 16 ++++---- include/linux/page_table_check.h | 66 ++++++++++++-------------------- include/linux/pgtable.h | 6 +-- mm/page_table_check.c | 50 +++++++++--------------- 6 files changed, 65 insertions(+), 97 deletions(-)