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

[David Hildenbrand <david@redhat.com>]

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@redhat.com> writes:
>>>
>>> dropping stable@vger.kernel.org
>>>
>>>> On 24.05.24 04:57, Andrew Morton wrote:
>>>>> On Thu, 23 May 2024 22:40:25 +0200 David Hildenbrand
>>>>> <david@redhat.com> wrote:
>>>>>
>>>>>>> You have stable@vger.kernel.org in the mail headers, so I assume
>>>>>>> you're
>>>>>>> proposing this for backporting.  When doing this, please include
>>>>>>>
>>>>>>> Cc: <stable@vger.kernel.org>
>>>>>>>
>>>>>>> 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