Re: [PATCH mm-unstable v2 00/16] mm: Introduce arch_mmap_hint()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



NACK.

Resend this _as an RFC_, _with the correct maintainers and reviewers cc'd_.

You've fundamentally violated kernel process and etiquette. I'd be more
forgiving, but this is at v2 and you've not cc'd KEY people. Twice. This is
totally unacceptable. See [0] if you are unsure of how to do so.

You've also sent a 16 patch series (!) immediately prior to the holidays
(!!) introducing an arch hook (!!!), which is strictly something we want to
try to avoid or lessen in future, which you'd know, had you followed basic
kernel etiquette and process and cc'd us on v1.

Any discussion that will be had here with others we won't be cc'd on, it's
16 patches, this isn't workable.

I'm on holiday from next week, it's not really fair to put this all on Liam
immediately prior to Christmas, so let's just examine this as an RFC first.

I hate to think that I need to set lei rules to catch stuff like this, but
clearly, I do. I will be updating these to check on all relevant
files. Sigh.

[0]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#select-the-recipients-for-your-patch

On Thu, Dec 12, 2024 at 04:02:31PM -0500, Liam R. Howlett wrote:
>
> + Lorenzo
>
> Can you please Cc the people listed in the maintainers on the files you
> are submitting against?  You seemed to Cc everyone but the mmap.c file
> maintainers?

Thanks Liam.

Also +Jann, another reviewer Kalesh missed (he only got Vlastimil and
Andrew so 2/5... of those required...)

>
>
> * Kalesh Singh <kaleshsingh@xxxxxxxxxx> [241211 18:28]:
> > Hi all,
> >
> > This is v2 othe the arch_mmap_hint() series.
> >
> > Changes in v2:
> >   - MAP_FIXED case is also handled in arch_mmap_hint() since this is just a
> >     special case of the hint addr being "enforced", per Yang Shi.
> >   - Consolidate most of the error handling in arch_mmap_hint().
> >   - Patch 16 ("mm: Fallback to generic_mmap_hint()") was folded into
> >     Patch 2 ("mm: x86: Introduce arch_mmap_hint()")
> >
> > v1: https://lore.kernel.org/r/20241210024119.2488608-1-kaleshsingh@xxxxxxxxxx/
> >
> > =======
> >
> > This series introduces arch_mmap_hint() to handle allocating VA space
> > for the hint address.
>
> Why?  Could we get more details in your cover letter please?  This
> entire email has as much detail as the subject line.

Yes the cover letter is ridiculously small for what that is doing.

>
> I don't want more arch_ anything.  If we can do this in a more generic
> way, then we should.

ENTIRELY agreed.

>
> >
> > Patches 1-16 introduce this new helper and Patch 17 uses it to fix the
> > issue of mmap hint being ignored in some cases due to THP alignment [1]
> >
> > [1] https://lore.kernel.org/r/20241118214650.3667577-1-kaleshsingh@xxxxxxxxxx/
> >
> > Thanks,
> > Kalesh
> >
> >
> > Kalesh Singh (16):
> >   mm: Introduce generic_mmap_hint()
> >   mm: x86: Introduce arch_mmap_hint()
> >   mm: arm: Introduce arch_mmap_hint()
> >   mm: alpha: Introduce arch_mmap_hint()
> >   mm: arc: Use generic_mmap_hint()
> >   mm: csky: Introduce arch_mmap_hint()
> >   mm: loongarch: Introduce arch_mmap_hint()
> >   mm: mips: Introduce arch_align_mmap_hint()
> >   mm: parisc: Introduce arch_align_mmap_hint()
> >   mm: s390: Use generic_mmap_hint()
> >   mm: sh: Introduce arch_mmap_hint()
> >   mm: sparc32: Introduce arch_mmap_hint()
> >   mm: sparc64: Introduce arch_mmap_hint()
> >   mm: xtensa: Introduce arch_mmap_hint()
> >   mm: powerpc: Introduce arch_mmap_hint()
> >   mm: Respect mmap hint before THP alignment if allocation is possible
> >
> >  arch/alpha/include/asm/pgtable.h           |   1 +
> >  arch/alpha/kernel/osf_sys.c                |  31 +++---
> >  arch/arc/include/asm/pgtable.h             |   1 +
> >  arch/arc/mm/mmap.c                         |  43 +++++----
> >  arch/arm/include/asm/pgtable.h             |   1 +
> >  arch/arm/mm/mmap.c                         | 107 +++++++++------------
> >  arch/csky/abiv1/inc/abi/pgtable-bits.h     |   1 +
> >  arch/csky/abiv1/mmap.c                     |  68 +++++++------
> >  arch/loongarch/include/asm/pgtable.h       |   1 +
> >  arch/loongarch/mm/mmap.c                   |  49 +++++-----
> >  arch/mips/include/asm/pgtable.h            |   1 +
> >  arch/mips/mm/mmap.c                        |  50 +++++-----
> >  arch/parisc/include/asm/pgtable.h          |   1 +
> >  arch/parisc/kernel/sys_parisc.c            |  53 +++++-----
> >  arch/powerpc/include/asm/book3s/64/slice.h |   1 +
> >  arch/powerpc/mm/book3s64/slice.c           |  31 ++++++
> >  arch/s390/include/asm/pgtable.h            |   1 +
> >  arch/s390/mm/mmap.c                        |  51 +++++-----
> >  arch/sh/include/asm/pgtable.h              |   1 +
> >  arch/sh/mm/mmap.c                          |  83 ++++++----------
> >  arch/sparc/include/asm/pgtable_32.h        |   1 +
> >  arch/sparc/include/asm/pgtable_64.h        |   1 +
> >  arch/sparc/kernel/sys_sparc_32.c           |  33 ++++---
> >  arch/sparc/kernel/sys_sparc_64.c           |  96 +++++++-----------
> >  arch/x86/include/asm/pgtable_64.h          |   1 +
> >  arch/x86/kernel/sys_x86_64.c               |  64 ++++++------
> >  arch/xtensa/kernel/syscall.c               |  31 ++++--
> >  include/linux/sched/mm.h                   |   9 ++
> >  mm/huge_memory.c                           |  17 ++--
> >  mm/mmap.c                                  |  86 +++++++++++------
> >  30 files changed, 491 insertions(+), 424 deletions(-)

Yuck. Is this copy/paste really necessary...

> >
> > --
> > 2.47.0.338.g60cca15819-goog
> >
> >




[Index of Archives]     [Linux SoC]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux