On Mon, May 19, 2025 at 12:31:04PM -0700, Yang Shi wrote: > > > On 5/19/25 7:43 AM, Lorenzo Stoakes wrote: > > Andrew - > > > > OK, I realise there's an issue here with patch 2/2. We're not accounting > > for the fact that madvise() will reject this _anyway_ because > > madvise_behavior_valid() will reject it. > > Good catch. The purpose of this patch is to make MADV_NOHUGEPAGE a no-op, so > we can just simply bail out early? The point of madvise behavior check is to > avoid taking mmap_lock and walking vmas for invalid behavior, but it doesn't > consider no-op (I treat op-op as valid but do nothing), so if we know this > advise is a no-op, we just bail out by returning 0. Yeah I was thinking better to just do it in mm/madvise.c to be honest. > > Maybe MADV_UNMERGEABLE should be no-op too, it returns 0 for !KSM anyway. Ack, yes could maybe bundle up? Anyway I didn't want to delay the s390 fix any longer and this is a mess now so better to defer to next cycle I think :>) > > Thanks, > Yang > > > > > > I've tried to be especially helpful here to aid Ignacio in his early > > contributions, but I think it's best now (if you don't mind Igancio) for me > > to figure out a better solution after the merge window. > > > > We're late in the cycle now so I will just resend the 1st patch (for s390) > > separately if you're happy to take that for 6.16? It's a simple rename of > > an entirely static identifier so should present no risk, and is approved by > > the arch maintainers who have also agreed for it to come through the mm > > tree. > > > > Apologies for the mess! > > > > Cheers, Lorenzo > > > > On Thu, May 15, 2025 at 09:15:44PM +0100, Lorenzo Stoakes wrote: > > > Andrew - > > > > > > I hope the explanation below resolves your query about the header include > > > (in [0]), let me know if doing this as a series like this works (we need to > > > enforce the ordering here). > > > > > > Thanks! > > > > > > [0]: 20250514153648.598bb031a2e498b1ac505b60@xxxxxxxxxxxxxxxxxxxx > > > > > > > > > > > > Currently, when somebody attempts to set MADV_NOHUGEPAGE on a system that > > > does not enable CONFIG_TRANSPARENT_HUGEPAGE the confguration option, this > > > results in an -EINVAL error arising. > > > > > > This doesn't really make sense, as to do so is essentially a no-op. > > > > > > Additionally, the semantics of setting VM_[NO]HUGEPAGE in any case are such > > > that, should the attribute not apply, nothing will be done. > > > > > > It therefore makes sense to simply make this operation a noop. > > > > > > However, a fly in the ointment is that, in order to do so, we must check > > > against the MADV_NOHUGEPAGE constant. In doing so, we encounter two rather > > > annoying issues. > > > > > > The first is that the usual include we would import to get hold of > > > MADV_NOHUGEPAGE, linux/mman.h, results in a circular dependency: > > > > > > * If something includes linux/mman.h, we in turn include linux/mm.h prior > > > to declaring MADV_NOHUGEPAGE. > > > * This then, in turn, includes linux/huge_mm.h. > > > * linux/huge_mm.h declares hugepage_madvise(), which then tries to > > > reference MADV_NOHUGEPAGE, and the build fails. > > > > > > This can be reached in other ways too. > > > > > > So we work around this by including uapi/asm/mman.h instead, which allows > > > us to keep hugepage_madvise() inline. > > > > > > The second issue is that the s390 arch declares PROT_NONE as a value in the > > > enum prot_type enumeration. > > > > > > By updating the include in linux/huge_mm.h, we pull in the PROT_NONE > > > declaration (unavoidably, this is ultimately in > > > uapi/asm-generic/mman-common.h alongside MADV_NOHUGEPAGE), which collides > > > with the enumeration value. > > > > > > To resolve this, we rename PROT_NONE to PROT_TYPE_DUMMY. > > > > > > The ordering of these patches is critical, the s390 patch must be applied > > > prior to the MADV_NOHUGEPAGE patch, and therefore the two patches are sent > > > as a series. > > > > > > v1: > > > * Place patches in series. > > > * Correct typo in comment as per James. > > > > > > previous patches: > > > huge_mm.h patch - https://lore.kernel.org/all/20250508-madvise-nohugepage-noop-without-thp-v1-1-e7ceffb197f3@xxxxxxxx/ > > > s390 patch - https://lore.kernel.org/all/20250514163530.119582-1-lorenzo.stoakes@xxxxxxxxxx/ > > > > > > Ignacio Moreno Gonzalez (1): > > > mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP > > > > > > Lorenzo Stoakes (1): > > > KVM: s390: rename PROT_NONE to PROT_TYPE_DUMMY > > > > > > arch/s390/kvm/gaccess.c | 8 ++++---- > > > include/linux/huge_mm.h | 5 +++++ > > > 2 files changed, 9 insertions(+), 4 deletions(-) > > > > > > -- > > > 2.49.0 > >
