Re: [PATCHv6 3/3] mm/gup_benchemark: add LONGTERM_BENCHMARK test in gup fast path

From: Pingfan Liu <kernelfans@gmail.com>
To: John Hubbard <jhubbard@nvidia.com>
Cc: Linux-MM <linux-mm@kvack.org>, Ira Weiny <ira.weiny@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mike Rapoport <rppt@linux.ibm.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Matthew Wilcox <willy@infradead.org>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	Christoph Hellwig <hch@infradead.org>,
	Shuah Khan <shuah@kernel.org>,
	Alexander Duyck <alexander.duyck@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCHv6 3/3] mm/gup_benchemark: add LONGTERM_BENCHMARK test in gup fast path
Date: Fri, 20 Mar 2020 17:17:19 +0800	[thread overview]
Message-ID: <CAFgQCTv0r1t0J+L4g9X=dfM+ofCy5j84=EU2YM6bXqx7Bykpdg@mail.gmail.com> (raw)
In-Reply-To: <8d748bfe-b2b0-bb56-fb2c-71de86183ba5@nvidia.com>

On Fri, Mar 20, 2020 at 6:27 AM John Hubbard <jhubbard@nvidia.com> wrote:
>
> On 3/15/20 9:34 PM, Pingfan Liu wrote:
> > Introduce a PIN_FAST_LONGTERM_BENCHMARK ioctl to test longterm pin in gup fast
> > path.
>
> 1. The subject line still has "benchemark", which should be "benchmark".
>
> 2. What do you really want to test? More about the use case to be tested would help.
> Just another sentence. I wouldn't normally ask, but in this case the implementation
> is slightly scrambled, and I can't suggest how to fix it because there's no information
> in the commit log as to the use case, nor the motivation.
Oh, the history about [3/3] is to verify the implementation method of
[2/3]. Please refer to
https://lore.kernel.org/linux-mm/20190611122935.GA9919@dhcp-128-55.nay.redhat.com/
Cite "
> > I think the concern is: for the successful gup_fast case with no CMA

> > pages, this patch is adding another complete loop through all the
> > pages. In the fast case.
> >
> > If the check were instead done as part of the gup_pte_range(), then
> > it would be a little more efficient for that case.
> >
> > As for whether it's worth it, *probably* this is too small an effect to measure.
> > But in order to attempt a measurement: running fio (https://github.com/axboe/fio)
> > with O_DIRECT on an NVMe drive, might shed some light. Here's an fio.conf file
> > that Jan Kara and Tom Talpey helped me come up with, for related testing:
"
But I think now, there is no motivation for it, and can be dropped it now.

Thanks,
Pingfan