Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 1/4] mm/gup: add compound page list iterator
To:     Jason Gunthorpe <jgg@nvidia.com>
CC:     Joao Martins <joao.m.martins@oracle.com>, <linux-mm@kvack.org>,
        <linux-kernel@vger.kernel.org>, <linux-rdma@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Doug Ledford <dledford@redhat.com>,
        "Matthew Wilcox" <willy@infradead.org>
References: <20210203220025.8568-1-joao.m.martins@oracle.com>
 <20210203220025.8568-2-joao.m.martins@oracle.com>
 <955dbe68-7302-a8bc-f0b5-e9032d7f190e@nvidia.com>
 <20210204195355.GO4247@nvidia.com>
From:   John Hubbard <jhubbard@nvidia.com>
Message-ID: <2919605d-f00f-4a07-8420-6b6d0a42081a@nvidia.com>
Date:   Thu, 4 Feb 2021 15:37:43 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.16; rv:78.0)
 Gecko/20100101 Thunderbird/78.7.0
MIME-Version: 1.0
In-Reply-To: <20210204195355.GO4247@nvidia.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2/4/21 11:53 AM, Jason Gunthorpe wrote:
> On Wed, Feb 03, 2021 at 03:00:01PM -0800, John Hubbard wrote:
>>> +static inline void compound_next(unsigned long i, unsigned long npages,
>>> +				 struct page **list, struct page **head,
>>> +				 unsigned int *ntails)
>>> +{
>>> +	if (i >= npages)
>>> +		return;
>>> +
>>> +	*ntails = count_ntails(list + i, npages - i);
>>> +	*head = compound_head(list[i]);
>>> +}
>>> +
>>> +#define for_each_compound_head(i, list, npages, head, ntails) \
>>
>> When using macros, which are dangerous in general, you have to worry about
>> things like name collisions. I really dislike that C has forced this unsafe
>> pattern upon us, but of course we are stuck with it, for iterator helpers.
>>
>> Given that we're stuck, you should probably use names such as __i, __list, etc,
>> in the the above #define. Otherwise you could stomp on existing variables.
> 
> Not this macro, it after cpp gets through with it all the macro names
> vanish, it can't collide with variables.
> 

Yes, I guess it does just vaporize, because it turns all the args into
their substituted values. I was just having flashbacks from similar cases
I guess.

> The usual worry is you might collide with other #defines, but we don't
> seem to worry about that in the kernel
> 

Well, I worry about it a little anyway. haha :)


thanks,
-- 
John Hubbard
NVIDIA