Re: [PATCH] selftest: mm: Test if hugepage does not get leaked during __bio_release_pages()

[David Hildenbrand]

On 24.05.24 11:31, Donet Tom wrote:
> On 5/24/24 12:31, David Hildenbrand wrote:
> > On 24.05.24 08:43, Ritesh Harjani (IBM) wrote:
> > > David Hildenbrand <david@xxxxxxxxxx> writes:
> > >
> > > dropping stable@xxxxxxxxxxxxxxx
> > >
> > > > On 24.05.24 04:57, Andrew Morton wrote:
> > > > > On Thu, 23 May 2024 22:40:25 +0200 David Hildenbrand
> > > > > <david@xxxxxxxxxx> wrote:
> > > > >
> > > > > > > You have stable@xxxxxxxxxxxxxxx in the mail headers, so I assume
> > > > > > > you're
> > > > > > > proposing this for backporting.  When doing this, please include
> > > > > > >
> > > > > > > Cc: <stable@xxxxxxxxxxxxxxx>
> > > > > > >
> > > > > > > in the changelog footers and also include a Fixes: target.  I'm
> > > > > > > assuming the suitable Fixes: target for this patch is 38b43539d64b?
> > > > > >
> > > > > > This adds a new selfest to make sure what was fixed (and
> > > > > > backported to
> > > > > > stable) remains fixed.
> > > > >
> > > > > Sure.  But we should provide -stable maintainers guidance for "how far
> > > > > back to go".  There isn't much point in backporting this into kernels
> > > > > where it's known to fail!
> > > >
> > > > I'm probably missing something important.
> > > >
> > > > 1) It's a test that does not fall into the common stable kernels
> > > > categories (see Documentation/process/stable-kernel-rules.rst).
> > > >
> > > > 2) If it fails in a kernel *it achieved its goal* of highlighting that
> > > > something serious is broken.
> > > >
> > > > > I'm still thinking that we want this in kernels which have
> > > > > 38b43539d64b?
> > > >
> > > > To hide that the other kernels are seriously broken and miss that fix?
> > > >
> > > > Really (1) this shouldn't be backported. I'm not even sure it should be
> > > > a selftest (sounds more like a reproducer that we usually attach to
> > > > commits, but that's too late). And if people care about backporting it,
> > > > (2) you really want this test to succeed everywhere. Especially also to
> > > > find kernels *without* 38b43539d64b
> > >
> > >
> > > Sorry about the noise and cc'd to stable. I believe we don't need to
> > > backport this test. The idea of adding a selftests was "also" to
> > > catch any
> > > future bugs like this.
> >
> > Yes, for that purpose it's fine, but it has quite the "specific
> > reproducer taste". Having it as part of something that is prepared to
> > run against arbitrary kernels (which selftests frequently are not) to
> > detect known problems feels better.
> I have seen some hugetlbfs directio tests in LTP. If you think
> selftest is not the correct place to add this test, we can drop this
> test from selftests and add it to LTP.

I think LTP might be a better fit to spot such issues in the wild. But I 
don't have a strong opinion.

--
Cheers,

David / dhildenb