[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210204195355.GO4247@nvidia.com>
Date: Thu, 4 Feb 2021 15:53:55 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: John Hubbard <jhubbard@...dia.com>
CC: Joao Martins <joao.m.martins@...cle.com>, <linux-mm@...ck.org>,
<linux-kernel@...r.kernel.org>, <linux-rdma@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Doug Ledford <dledford@...hat.com>,
"Matthew Wilcox" <willy@...radead.org>
Subject: Re: [PATCH 1/4] mm/gup: add compound page list iterator
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.
The usual worry is you might collide with other #defines, but we don't
seem to worry about that in the kernel
Jason
Powered by blists - more mailing lists