Re: [PATCH 2/3] mm/gup_benchmark: support pin_user_pages() and related calls

[John Hubbard]

On 1/27/20 12:52 PM, Nathan Chancellor wrote:
...
>> --- a/mm/gup_benchmark.c
>> +++ b/mm/gup_benchmark.c
>> @@ -8,6 +8,8 @@
>>  #define GUP_FAST_BENCHMARK	_IOWR('g', 1, struct gup_benchmark)
>>  #define GUP_LONGTERM_BENCHMARK	_IOWR('g', 2, struct gup_benchmark)
>>  #define GUP_BENCHMARK		_IOWR('g', 3, struct gup_benchmark)
>> +#define PIN_FAST_BENCHMARK	_IOWR('g', 4, struct gup_benchmark)
>> +#define PIN_BENCHMARK		_IOWR('g', 5, struct gup_benchmark)
>>  
>>  struct gup_benchmark {
>>  	__u64 get_delta_usec;
>> @@ -19,6 +21,47 @@ struct gup_benchmark {
>>  	__u64 expansion[10];	/* For future use */
>>  };
>>  
>> +static void put_back_pages(int cmd, struct page **pages, unsigned long nr_pages)
> 
> We received a Clang build report on this patch because the use of
> PIN_FAST_BENCHMARK and PIN_BENCHMARK in the switch statement below will
> overflow int; this should be unsigned int to match the cmd parameter in
> the ioctls.


Thanks for the report! Yes, that should have been "unsigned int cmd", to match the
one in the ioctls, just as you said.

I'll apply this diff, for the next version of the series:

diff --git a/mm/gup_benchmark.c b/mm/gup_benchmark.c
index 3d5fb765e4e6..7d5573083ab3 100644
--- a/mm/gup_benchmark.c
+++ b/mm/gup_benchmark.c
@@ -21,7 +21,8 @@ struct gup_benchmark {
        __u64 expansion[10];    /* For future use */
 };
 
-static void put_back_pages(int cmd, struct page **pages, unsigned long nr_pages)
+static void put_back_pages(unsigned int cmd, struct page **pages,
+                          unsigned long nr_pages)
 {
        int i;
 
@@ -40,7 +41,7 @@ static void put_back_pages(int cmd, struct page **pages, unsigned long nr_pages)
        }
 }
 
-static void verify_dma_pinned(int cmd, struct page **pages,
+static void verify_dma_pinned(unsigned int cmd, struct page **pages,
                              unsigned long nr_pages)
 {
        int i;



thanks,
-- 
John Hubbard
NVIDIA