linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Eric B Munson <emunson@akamai.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Davidlohr Bueso <dave@stgolabs.net>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	linux-api@vger.kernel.org,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	David Rientjes <rientjes@google.com>,
	Hugh Dickins <hughd@google.com>,
	Aneesh Kumar <aneesh.kumar@linux.vnet.ibm.com>,
	Hillf Danton <hillf.zj@alibaba-inc.com>,
	Christoph Hellwig <hch@infradead.org>,
	Michal Hocko <mhocko@suse.cz>,
	emunson@mgebm.net
Subject: Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
Date: Thu, 23 Jul 2015 10:29:14 -0700	[thread overview]
Message-ID: <55B1246A.9010905@oracle.com> (raw)
In-Reply-To: <20150723171728.GC9203@akamai.com>

On 07/23/2015 10:17 AM, Eric B Munson wrote:
> On Thu, 23 Jul 2015, Mike Kravetz wrote:
>
>> On 07/23/2015 08:17 AM, Eric B Munson wrote:
>>> On Wed, 22 Jul 2015, Mike Kravetz wrote:
>>>
>>>> On 07/22/2015 03:30 PM, Andrew Morton wrote:
>>>>> On Wed, 22 Jul 2015 15:19:54 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote:
>>>>>
>>>>>>>
>>>>>>> I didn't know that libhugetlbfs has tests.  I wonder if that makes
>>>>>>> tools/testing/selftests/vm's hugetlbfstest harmful?
>>>>>>
>>>>>> Why harmful? Redundant, maybe(?).
>>>>>
>>>>> The presence of the in-kernel tests will cause people to add stuff to
>>>>> them when it would be better if they were to apply that effort to
>>>>> making libhugetlbfs better.  Or vice versa.
>>>>>
>>>>> Mike's work is an example.  Someone later makes a change to hugetlbfs, runs
>>>>> the kernel selftest and says "yay, everything works", unaware that they
>>>>> just broke fallocate support.
>>>>>
>>>>>> Does anyone even use selftests for
>>>>>> hugetlbfs regression testing? Lets see, we also have these:
>>>>>>
>>>>>> - hugepage-{mmap,shm}.c
>>>>>> - map_hugetlb.c
>>>>>>
>>>>>> There's probably a lot of room for improvement here.
>>>>>
>>>>> selftests is a pretty scrappy place.  It's partly a dumping ground for
>>>>> things so useful test code doesn't just get lost and bitrotted.  Partly
>>>>> a framework so people who add features can easily test them. Partly to
>>>>> provide tools to architecture maintainers when they wire up new
>>>>> syscalls and the like.
>>>>>
>>>>> Unless there's some good reason to retain the hugetlb part of
>>>>> selftests, I'm thinking we should just remove it to avoid
>>>>> distracting/misleading people.  Or possibly move the libhugetlbfs test
>>>>> code into the kernel tree and maintain it there.
>>>>
>>>> Adding Eric as he is the libhugetlbfs maintainer.
>>>>
>>>> I think removing the hugetlb selftests in the kernel and pointing
>>>> people to libhugetlbfs is the way to go.  From a very quick scan
>>>> of the selftests, I would guess libhugetlbfs covers everything
>>>> in those tests.
>>>>
>>>> I'm willing to verify the testing provided by selftests is included
>>>> in libhugetlbfs, and remove selftests if that is the direction we
>>>> want to take.
>>>
>>> I would rather see the test suite stay in the library, there are a
>>> number of tests that rely on infrastructure in the library that is not
>>> available in selftests.
>>>
>>> I am happy to help with any tests that need to be added/modified in the
>>> library to cover.
>>
>> I thought about this some more and think there are two distinct
>> groups of users that should be considered.
>> 1) Application developers who simply want to use hugetlb
>> 2) Kernel developers who are modifying hugetlb related code
>>
>> The application developers will mostly want information in the
>> man pages, hugetlbpage.txt and hugetlb selftest programs to use
>> as sample code to get started.  They can also use libhugetlbfs
>> man pages/library if they desire.  Because of this, I do not
>> really want to remove the hugetlb selftest programs.  There are
>> no equivalent simple stand alone programs in libhugetlbfs.
>>
>> Kernel developers would be more concerned about introducing
>> regressions.  The selftest programs are of limited use for this
>> purpose.  The libhugetlbfs test suite is much more suited for
>> regression testing.
>>
>> With this in mind, I suggest:
>> - Keep the mmap man page reference to Documentation/vm/hugetlbpage.txt
>> - Small modification to hugetlbpage.txt saying the selftest code is
>>    good for application development examples.  And, kernel developers
>>    should use libhugetlbfs test suite for regression testing.  In any
>>    case, the sourceforge URL for libhugetlbfs is no longer valid and
>>    needs to be updated.
>> - Modify the run_vmtests selftest script to print out a message saying
>>    libhugetlbfs should be used for hugetlb regression testing.  This
>>    would help catch people who might think the few selftests are
>>    sufficient.
>>
>> Thoughts?
>
> There are a number of tests in the libhugetlbfs suite that cover kernel
> problems, are you suggesting that we move all these tests out of
> libhugetlbfs and into selftests?  I don't think we should separate the
> responsibility for testing kernel regressions so where ever they end up,
> they should all be together.  The libhugetlbfs suite has some nice
> features for setting up the test environment (consider that a plug to
> move tests in that direction).

No, not suggesting we move anything out of libhugetlbfs.  I believe
that should be the primary test suite for hugetlb.

However, the few programs in selftest do provide some value IMO.
They are examples of hugetlb usage without any of the libhugetlbfs
infrastructure present.  Ideally, there would be some place to put
this sample code.  I can not think of an ideal location.

-- 
Mike Kravetz

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

      reply	other threads:[~2015-07-23 17:29 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-21 18:09 [PATCH v4 00/10] hugetlbfs: add fallocate support Mike Kravetz
2015-07-21 18:09 ` [PATCH v4 01/10] mm/hugetlb: add cache of descriptors to resv_map for region_add Mike Kravetz
2015-07-22 22:03   ` Andrew Morton
2015-07-21 18:09 ` [PATCH v4 02/10] mm/hugetlb: add region_del() to delete a specific range of entries Mike Kravetz
2015-07-21 18:09 ` [PATCH v4 03/10] mm/hugetlb: expose hugetlb fault mutex for use by fallocate Mike Kravetz
2015-07-22 22:03   ` Andrew Morton
2015-07-21 18:09 ` [PATCH v4 04/10] hugetlbfs: hugetlb_vmtruncate_list() needs to take a range to delete Mike Kravetz
2015-07-21 18:09 ` [PATCH v4 05/10] hugetlbfs: truncate_hugepages() takes a range of pages Mike Kravetz
2015-07-21 18:09 ` [PATCH v4 06/10] mm/hugetlb: vma_has_reserves() needs to handle fallocate hole punch Mike Kravetz
2015-07-21 18:09 ` [PATCH v4 07/10] mm/hugetlb: alloc_huge_page handle areas hole punched by fallocate Mike Kravetz
2015-07-21 18:09 ` [PATCH v4 08/10] hugetlbfs: New huge_add_to_page_cache helper routine Mike Kravetz
2015-07-21 18:09 ` [PATCH v4 09/10] hugetlbfs: add hugetlbfs_fallocate() Mike Kravetz
     [not found]   ` <1437502184-14269-10-git-send-email-mike.kravetz-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2015-07-22 22:03     ` Andrew Morton
2015-07-22 22:23       ` Mike Kravetz
     [not found]         ` <55B017EE.5020203-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2015-07-22 22:30           ` Andrew Morton
2015-07-21 18:09 ` [PATCH v4 10/10] mm: madvise allow remove operation for hugetlbfs Mike Kravetz
2015-07-22 22:03 ` [PATCH v4 00/10] hugetlbfs: add fallocate support Andrew Morton
     [not found] ` <1437502184-14269-1-git-send-email-mike.kravetz-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2015-07-22 22:06   ` Andrew Morton
     [not found]     ` <20150722150647.2597c7e5be9ee1eecc438b6f-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2015-07-22 22:19       ` Davidlohr Bueso
     [not found]         ` <1437603594.3298.5.camel-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org>
2015-07-22 22:30           ` Andrew Morton
     [not found]             ` <20150722153023.e8f15eb4e490f79cc029c8cd-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2015-07-22 22:34               ` Davidlohr Bueso
     [not found]                 ` <1437604474.3298.7.camel-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org>
2015-07-22 22:37                   ` Andrew Morton
2015-07-22 22:50                     ` Davidlohr Bueso
2015-07-22 22:54                       ` Davidlohr Bueso
2015-07-22 23:18             ` Mike Kravetz
2015-07-22 23:54               ` Davidlohr Bueso
2015-07-23  0:05                 ` David Rientjes
2015-07-23 15:17               ` Eric B Munson
2015-07-23 17:05                 ` Mike Kravetz
     [not found]                   ` <55B11EEF.1070605-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2015-07-23 17:17                     ` Eric B Munson
2015-07-23 17:29                       ` Mike Kravetz [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55B1246A.9010905@oracle.com \
    --to=mike.kravetz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=dave@stgolabs.net \
    --cc=emunson@akamai.com \
    --cc=emunson@mgebm.net \
    --cc=hch@infradead.org \
    --cc=hillf.zj@alibaba-inc.com \
    --cc=hughd@google.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=rientjes@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).