Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <251cf8bf-a649-2812-1aa6-39c7b23d5e03@nvidia.com>
Date:   Thu, 18 Aug 2022 11:55:40 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [PATCH] selftests/vm: fix inability to build any vm tests
Content-Language: en-US
To:     Axel Rasmussen <axelrasmussen@google.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Shuah Khan <shuah@kernel.org>,
        Guillaume Tucker <guillaume.tucker@collabora.com>,
        Linux MM <linux-mm@kvack.org>,
        Linuxkselftest <linux-kselftest@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
References: <20220817211356.273019-1-axelrasmussen@google.com>
 <190edda8-1f37-0fa5-1cc1-ada97518698a@nvidia.com>
 <CAJHvVcgktSjo5CncC25+2j1amXvn3TjnsfOCV6CxNDp5joey9Q@mail.gmail.com>
From:   John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <CAJHvVcgktSjo5CncC25+2j1amXvn3TjnsfOCV6CxNDp5joey9Q@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a0xVcmhTY2pwbzZaVytMdDdKWjlLNXRQNjdEMU1TUVQ4YWgzbDRPeGxnbUY3?=
 =?utf-8?B?NThBRFJoRlh6S0dqRE1YTG05d0lBNmVhMmIwYXVnWm9yWUJNakRJdmtCYTFW?=
 =?utf-8?B?OU5oTXkrY2NydHpLN2x5aWNKZHhqUkhzNXd6dk95QUd1WURKSElrS1lOMXFx?=
 =?utf-8?B?T2l6U2JjTjVDOFlKSUs0Y0NYMVBIeUF6eC9tZU9HREZsV0lSR05CNEFSYVE1?=
 =?utf-8?B?dm5QWFAvRiszYjB1clVGbVBCMDFlcW1iVWI1MFZBbVE2c3FqUUtzM0ozL1VK?=
 =?utf-8?B?VzR0NU5QZEIzQ0tRTjB2TUh2a056dU1uMkR3UlRzN2lWT0R1UEpJT2pSZUVi?=
 =?utf-8?B?NzdzVEM2bDh1OE5RSGZsNnN3U0F0RTJ1OGZYTGNyVExicE1MMDd0RUhWVlFX?=
 =?utf-8?B?OXQ0RjRRSTJ6cEZCZjR4dndsWkFiYlhPWTQ5anZMclVCbTA5UXgrcUErTjZj?=
 =?utf-8?B?WUtLT0FkNDFpeXd0a1JIYjVQR3lzZUxJY0FGb3FjU1NPcnRiYy9sc1BoeGhN?=
 =?utf-8?B?Z2xONWkxdmxsTldUNVdYL3MzTmZTc2t4SzgyR3RZTTZlRG1aNUxLdTdWY2pu?=
 =?utf-8?B?cHNSRk5YTVc0cTROdWVIdUdWelk0M2hndVcvaG5hNnNOaThPSi91TU5TSyt0?=
 =?utf-8?B?aGZOU29PTnovSXd1TWZXbVh5aDlrcHlPTkF3aDdaZFdlT3FNMTJhY1dNYXZH?=
 =?utf-8?B?QkIwWHo5TGlPVEZIeCt1TUl0RmVEUXFxMEd2Z3pINWxLZmtoNjUzYVhEQWls?=
 =?utf-8?B?MFprcWRlWFZpeXZPc0FObGQzVXBZQmxuMXVsbjlGVit1c1FvWDRRbG81RXFD?=
 =?utf-8?B?bXJPKzQvMEx3TUhMOFR6WVl0akl6WWZjejFTRjJ1K3hDVjROWkRITUlWUTlx?=
 =?utf-8?B?SDcxb3pxeVVTR3NRM0J4WGV0cFpFQmlMSStkNzV5c05VU3diQmgrVmVFTS8x?=
 =?utf-8?B?blgrcGhEK1Z2RTU0ckFnOTJSZUwwUUY4Sk1wRjUwVVBOcDFnaHVZb1N2KzdR?=
 =?utf-8?B?cEduRmRsUTF0Mjc5UU15emx2VnlNWXBXUDBzdkVIY3d5dUF0T0YzT0FJdGRQ?=
 =?utf-8?B?dStLNnlJTTBIWWdMeExkWkh5MWZXMDZpYWliSHFnazVHL2VwR04rNWxOZTVo?=
 =?utf-8?B?N05PMVlsSXo3azVzNmZLQlNKcjFxMnpueWsvWW1QOVppcG9lU1N5bGRMMVUy?=
 =?utf-8?B?bllCOUt6ZFkvTzlPSlVwWFlKRS9kWjJSTmhtT1NXR2xDazBUeE1zNEsxN3lO?=
 =?utf-8?B?Y1lacWNVOUJjVnFJQTk1aXhtZ2lKOTRZdm9BSVJtVDNoTW5TR3BSVFoxUTZP?=
 =?utf-8?B?OHZHNDQ5MXNqeEx5Y0x6TVhERThBSWMvQXNrYlhFdU5GZmFoSXpHZ1lENGd4?=
 =?utf-8?B?bzNqeDdDbTJhSzk0eFo2cXVaSFVZUWtodGF2MjVURmV4cmdqbDBwYjFnWHNT?=
 =?utf-8?B?U3VIRnJzL0h4eDlXM2g3YXpYb1oxRmg4R2pPTnZXeTYvdkk1dDcwWFVUa1Fq?=
 =?utf-8?B?ZVBiWVJ2UTZ3dHRja29uVFZVbFFIb2k3TC9VdzBockptb2pzbklPcksrdDlk?=
 =?utf-8?B?bFlnYTg3ajU1MEhZNjRwbGZMdFNZZDM1OGlTREZTR3hLN2tNZ0U4Qlh4WkMx?=
 =?utf-8?B?UWJMMWg5eFdldmxqaURTMUpnU25LK0pWazljT25iL0xKazBuNUZFT25MN1VB?=
 =?utf-8?B?eks0UWdIYnNycDJ1Vm5VTjdVOVIrem9uRUYybFl4MlFlWTlsOVNPcE1vZ2xZ?=
 =?utf-8?B?OEI2WXhBaDYybm5nMXZPWUZYemxqTDJvMUkyTmNKUDUzLzdaUkdsc3RwYTE0?=
 =?utf-8?B?MFlESUhZV1N0dEdBaU9NL0xuekJKdzF6QjlyeUtOQjJObHFxZHpxQ0ZPS0Va?=
 =?utf-8?B?U0d1WSt4Y3FORGdSbGFFZUFseGVPbHNlWGFZQjB2aVFzbUhnVFZnQWxKY1Yz?=
 =?utf-8?B?YWxnc3ZyWTRBUVBIdDBhMDlnQ3NxbnR5N1FpY21pNHJZalJ0ZjNnc1VvVkx2?=
 =?utf-8?B?ZHNxZ2t6cXZMNjBxaHNqaUZkd2NrZWRvRW01TG9hWjREVEFZNjZNNTZ4SGtN?=
 =?utf-8?B?N3E1N0tJSkJubmphNVRGQnp2eDJ4NmpoUDQraFk5eThFcklOYUxVVUhYblRK?=
 =?utf-8?Q?vfoBtMxzxxq6pEz9mDsN3Ap3C?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5b2ec4da-c607-491d-b888-08da814b413f
X-MS-Exchange-CrossTenant-AuthSource: BY5PR12MB4130.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Aug 2022 18:55:44.5134
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: OG4erW98s4+gj9WNy8B8b+0hDLXKkWEXhLUPPIzC0OzyysUf6kMLOLjItYXl89tvEz21uOSGyP39IcoOg6hn0A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB2582
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 8/18/22 10:58, Axel Rasmussen wrote:
>> 1) I recalled that hmm-tests.c in the same directory also needs
>> gup_test.h, and wondered if that was covered. And it turns out the the
>> relative "up and over" include path is done in hmm-tests.c itself,
>> instead of in the Makefile, like this:
>>
>> /*
>>    * This is a private UAPI to the kernel test module so it isn't exported
>>    * in the usual include/uapi/... directory.
>>    */
>> #include "../../../../lib/test_hmm_uapi.h"
>> #include "../../../../mm/gup_test.h"
>>
>> It would be nice (maybe follow-up polishing for someone) if this were
>> done once, instead of twice (or more?) via different (source code vs.
>> Makefile) mechanisms.
> 
> Hmm, I suppose the way to clean this up would be to have the Makefile
> compute this once, and pass in "-I $(selfdir)/../../.." to the
> compiler so we could just "#include <mm/gup_test.h>" directly?

Yes, it's better to have the Makefile know where the include paths are,
rather than each source file, so that looks better.

But hold on, now I see that selftests/vm/Makefile already uses what is
effectively a src_topdir, anyway! Here:

CFLAGS = -Wall -I ../../../../usr/include $(EXTRA_CFLAGS) $(KHDR_INCLUDES)

...which makes me think that real fix for the original problem should be
simply this:

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 947fc72413e9..d44c72b3abe3 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -40,6 +40,7 @@ ifeq (0,$(MAKELEVEL))
      endif
  endif
  selfdir = $(realpath $(dir $(filter %/lib.mk,$(MAKEFILE_LIST))))
+top_srcdir = $(selfdir)/../../..

  # The following are built by lib.mk common compile rules.
  # TEST_CUSTOM_PROGS should be used by tests that require


...and then the follow up cleanup can use top_srcdir to cleanup CFLAGS and
also pull the relative paths out of the source files and into the Makefile.


thanks,
-- 
John Hubbard
NVIDIA