| Message ID | 20230501165450.15352-20-surenb@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Memory allocation profiling | expand |
On Mon, 1 May 2023 09:54:29 -0700 Suren Baghdasaryan <surenb@google.com> wrote: > After redefining alloc_pages, all uses of that name are being replaced. > Change the conflicting names to prevent preprocessor from replacing them > when it's not intended. > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > --- > arch/x86/kernel/amd_gart_64.c | 2 +- > drivers/iommu/dma-iommu.c | 2 +- > drivers/xen/grant-dma-ops.c | 2 +- > drivers/xen/swiotlb-xen.c | 2 +- > include/linux/dma-map-ops.h | 2 +- > kernel/dma/mapping.c | 4 ++-- > 6 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c > index 56a917df410d..842a0ec5eaa9 100644 > --- a/arch/x86/kernel/amd_gart_64.c > +++ b/arch/x86/kernel/amd_gart_64.c > @@ -676,7 +676,7 @@ static const struct dma_map_ops gart_dma_ops = { > .get_sgtable = dma_common_get_sgtable, > .dma_supported = dma_direct_supported, > .get_required_mask = dma_direct_get_required_mask, > - .alloc_pages = dma_direct_alloc_pages, > + .alloc_pages_op = dma_direct_alloc_pages, > .free_pages = dma_direct_free_pages, > }; > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 7a9f0b0bddbd..76a9d5ca4eee 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -1556,7 +1556,7 @@ static const struct dma_map_ops iommu_dma_ops = { > .flags = DMA_F_PCI_P2PDMA_SUPPORTED, > .alloc = iommu_dma_alloc, > .free = iommu_dma_free, > - .alloc_pages = dma_common_alloc_pages, > + .alloc_pages_op = dma_common_alloc_pages, > .free_pages = dma_common_free_pages, > .alloc_noncontiguous = iommu_dma_alloc_noncontiguous, > .free_noncontiguous = iommu_dma_free_noncontiguous, > diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c > index 9784a77fa3c9..6c7d984f164d 100644 > --- a/drivers/xen/grant-dma-ops.c > +++ b/drivers/xen/grant-dma-ops.c > @@ -282,7 +282,7 @@ static int xen_grant_dma_supported(struct device *dev, u64 mask) > static const struct dma_map_ops xen_grant_dma_ops = { > .alloc = xen_grant_dma_alloc, > .free = xen_grant_dma_free, > - .alloc_pages = xen_grant_dma_alloc_pages, > + .alloc_pages_op = xen_grant_dma_alloc_pages, > .free_pages = xen_grant_dma_free_pages, > .mmap = dma_common_mmap, > .get_sgtable = dma_common_get_sgtable, > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c > index 67aa74d20162..5ab2616153f0 100644 > --- a/drivers/xen/swiotlb-xen.c > +++ b/drivers/xen/swiotlb-xen.c > @@ -403,6 +403,6 @@ const struct dma_map_ops xen_swiotlb_dma_ops = { > .dma_supported = xen_swiotlb_dma_supported, > .mmap = dma_common_mmap, > .get_sgtable = dma_common_get_sgtable, > - .alloc_pages = dma_common_alloc_pages, > + .alloc_pages_op = dma_common_alloc_pages, > .free_pages = dma_common_free_pages, > }; > diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h > index 31f114f486c4..d741940dcb3b 100644 > --- a/include/linux/dma-map-ops.h > +++ b/include/linux/dma-map-ops.h > @@ -27,7 +27,7 @@ struct dma_map_ops { > unsigned long attrs); > void (*free)(struct device *dev, size_t size, void *vaddr, > dma_addr_t dma_handle, unsigned long attrs); > - struct page *(*alloc_pages)(struct device *dev, size_t size, > + struct page *(*alloc_pages_op)(struct device *dev, size_t size, > dma_addr_t *dma_handle, enum dma_data_direction dir, > gfp_t gfp); > void (*free_pages)(struct device *dev, size_t size, struct page *vaddr, > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c > index 9a4db5cce600..fc42930af14b 100644 > --- a/kernel/dma/mapping.c > +++ b/kernel/dma/mapping.c > @@ -570,9 +570,9 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size, > size = PAGE_ALIGN(size); > if (dma_alloc_direct(dev, ops)) > return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp); > - if (!ops->alloc_pages) > + if (!ops->alloc_pages_op) > return NULL; > - return ops->alloc_pages(dev, size, dma_handle, dir, gfp); > + return ops->alloc_pages_op(dev, size, dma_handle, dir, gfp); > } > > struct page *dma_alloc_pages(struct device *dev, size_t size, I'm not impressed. This patch increases churn for code which does not (directly) benefit from the change, and that for limitations in your tooling? Why not just rename the conflicting uses in your local tree, but then remove the rename from the final patch series? Just my two cents, Petr T
On Tue, May 2, 2023 at 8:50 AM Petr Tesařík <petr@tesarici.cz> wrote: > > On Mon, 1 May 2023 09:54:29 -0700 > Suren Baghdasaryan <surenb@google.com> wrote: > > > After redefining alloc_pages, all uses of that name are being replaced. > > Change the conflicting names to prevent preprocessor from replacing them > > when it's not intended. > > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > --- > > arch/x86/kernel/amd_gart_64.c | 2 +- > > drivers/iommu/dma-iommu.c | 2 +- > > drivers/xen/grant-dma-ops.c | 2 +- > > drivers/xen/swiotlb-xen.c | 2 +- > > include/linux/dma-map-ops.h | 2 +- > > kernel/dma/mapping.c | 4 ++-- > > 6 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c > > index 56a917df410d..842a0ec5eaa9 100644 > > --- a/arch/x86/kernel/amd_gart_64.c > > +++ b/arch/x86/kernel/amd_gart_64.c > > @@ -676,7 +676,7 @@ static const struct dma_map_ops gart_dma_ops = { > > .get_sgtable = dma_common_get_sgtable, > > .dma_supported = dma_direct_supported, > > .get_required_mask = dma_direct_get_required_mask, > > - .alloc_pages = dma_direct_alloc_pages, > > + .alloc_pages_op = dma_direct_alloc_pages, > > .free_pages = dma_direct_free_pages, > > }; > > > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > index 7a9f0b0bddbd..76a9d5ca4eee 100644 > > --- a/drivers/iommu/dma-iommu.c > > +++ b/drivers/iommu/dma-iommu.c > > @@ -1556,7 +1556,7 @@ static const struct dma_map_ops iommu_dma_ops = { > > .flags = DMA_F_PCI_P2PDMA_SUPPORTED, > > .alloc = iommu_dma_alloc, > > .free = iommu_dma_free, > > - .alloc_pages = dma_common_alloc_pages, > > + .alloc_pages_op = dma_common_alloc_pages, > > .free_pages = dma_common_free_pages, > > .alloc_noncontiguous = iommu_dma_alloc_noncontiguous, > > .free_noncontiguous = iommu_dma_free_noncontiguous, > > diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c > > index 9784a77fa3c9..6c7d984f164d 100644 > > --- a/drivers/xen/grant-dma-ops.c > > +++ b/drivers/xen/grant-dma-ops.c > > @@ -282,7 +282,7 @@ static int xen_grant_dma_supported(struct device *dev, u64 mask) > > static const struct dma_map_ops xen_grant_dma_ops = { > > .alloc = xen_grant_dma_alloc, > > .free = xen_grant_dma_free, > > - .alloc_pages = xen_grant_dma_alloc_pages, > > + .alloc_pages_op = xen_grant_dma_alloc_pages, > > .free_pages = xen_grant_dma_free_pages, > > .mmap = dma_common_mmap, > > .get_sgtable = dma_common_get_sgtable, > > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c > > index 67aa74d20162..5ab2616153f0 100644 > > --- a/drivers/xen/swiotlb-xen.c > > +++ b/drivers/xen/swiotlb-xen.c > > @@ -403,6 +403,6 @@ const struct dma_map_ops xen_swiotlb_dma_ops = { > > .dma_supported = xen_swiotlb_dma_supported, > > .mmap = dma_common_mmap, > > .get_sgtable = dma_common_get_sgtable, > > - .alloc_pages = dma_common_alloc_pages, > > + .alloc_pages_op = dma_common_alloc_pages, > > .free_pages = dma_common_free_pages, > > }; > > diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h > > index 31f114f486c4..d741940dcb3b 100644 > > --- a/include/linux/dma-map-ops.h > > +++ b/include/linux/dma-map-ops.h > > @@ -27,7 +27,7 @@ struct dma_map_ops { > > unsigned long attrs); > > void (*free)(struct device *dev, size_t size, void *vaddr, > > dma_addr_t dma_handle, unsigned long attrs); > > - struct page *(*alloc_pages)(struct device *dev, size_t size, > > + struct page *(*alloc_pages_op)(struct device *dev, size_t size, > > dma_addr_t *dma_handle, enum dma_data_direction dir, > > gfp_t gfp); > > void (*free_pages)(struct device *dev, size_t size, struct page *vaddr, > > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c > > index 9a4db5cce600..fc42930af14b 100644 > > --- a/kernel/dma/mapping.c > > +++ b/kernel/dma/mapping.c > > @@ -570,9 +570,9 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size, > > size = PAGE_ALIGN(size); > > if (dma_alloc_direct(dev, ops)) > > return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp); > > - if (!ops->alloc_pages) > > + if (!ops->alloc_pages_op) > > return NULL; > > - return ops->alloc_pages(dev, size, dma_handle, dir, gfp); > > + return ops->alloc_pages_op(dev, size, dma_handle, dir, gfp); > > } > > > > struct page *dma_alloc_pages(struct device *dev, size_t size, > > I'm not impressed. This patch increases churn for code which does not > (directly) benefit from the change, and that for limitations in your > tooling? > > Why not just rename the conflicting uses in your local tree, but then > remove the rename from the final patch series? With alloc_pages function becoming a macro, the preprocessor ends up replacing all instances of that name, even when it's not used as a function. That what necessitates this change. If there is a way to work around this issue without changing all alloc_pages() calls in the source base I would love to learn it but I'm not quite clear about your suggestion and if it solves the issue. Could you please provide more details? > > Just my two cents, > Petr T > > -- > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com. >
On Tue, 2 May 2023 11:38:49 -0700 Suren Baghdasaryan <surenb@google.com> wrote: > On Tue, May 2, 2023 at 8:50 AM Petr Tesařík <petr@tesarici.cz> wrote: > > > > On Mon, 1 May 2023 09:54:29 -0700 > > Suren Baghdasaryan <surenb@google.com> wrote: > > > > > After redefining alloc_pages, all uses of that name are being replaced. > > > Change the conflicting names to prevent preprocessor from replacing them > > > when it's not intended. > > > > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > > --- > > > arch/x86/kernel/amd_gart_64.c | 2 +- > > > drivers/iommu/dma-iommu.c | 2 +- > > > drivers/xen/grant-dma-ops.c | 2 +- > > > drivers/xen/swiotlb-xen.c | 2 +- > > > include/linux/dma-map-ops.h | 2 +- > > > kernel/dma/mapping.c | 4 ++-- > > > 6 files changed, 7 insertions(+), 7 deletions(-) > > > > > > diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c > > > index 56a917df410d..842a0ec5eaa9 100644 > > > --- a/arch/x86/kernel/amd_gart_64.c > > > +++ b/arch/x86/kernel/amd_gart_64.c > > > @@ -676,7 +676,7 @@ static const struct dma_map_ops gart_dma_ops = { > > > .get_sgtable = dma_common_get_sgtable, > > > .dma_supported = dma_direct_supported, > > > .get_required_mask = dma_direct_get_required_mask, > > > - .alloc_pages = dma_direct_alloc_pages, > > > + .alloc_pages_op = dma_direct_alloc_pages, > > > .free_pages = dma_direct_free_pages, > > > }; > > > > > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > > index 7a9f0b0bddbd..76a9d5ca4eee 100644 > > > --- a/drivers/iommu/dma-iommu.c > > > +++ b/drivers/iommu/dma-iommu.c > > > @@ -1556,7 +1556,7 @@ static const struct dma_map_ops iommu_dma_ops = { > > > .flags = DMA_F_PCI_P2PDMA_SUPPORTED, > > > .alloc = iommu_dma_alloc, > > > .free = iommu_dma_free, > > > - .alloc_pages = dma_common_alloc_pages, > > > + .alloc_pages_op = dma_common_alloc_pages, > > > .free_pages = dma_common_free_pages, > > > .alloc_noncontiguous = iommu_dma_alloc_noncontiguous, > > > .free_noncontiguous = iommu_dma_free_noncontiguous, > > > diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c > > > index 9784a77fa3c9..6c7d984f164d 100644 > > > --- a/drivers/xen/grant-dma-ops.c > > > +++ b/drivers/xen/grant-dma-ops.c > > > @@ -282,7 +282,7 @@ static int xen_grant_dma_supported(struct device *dev, u64 mask) > > > static const struct dma_map_ops xen_grant_dma_ops = { > > > .alloc = xen_grant_dma_alloc, > > > .free = xen_grant_dma_free, > > > - .alloc_pages = xen_grant_dma_alloc_pages, > > > + .alloc_pages_op = xen_grant_dma_alloc_pages, > > > .free_pages = xen_grant_dma_free_pages, > > > .mmap = dma_common_mmap, > > > .get_sgtable = dma_common_get_sgtable, > > > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c > > > index 67aa74d20162..5ab2616153f0 100644 > > > --- a/drivers/xen/swiotlb-xen.c > > > +++ b/drivers/xen/swiotlb-xen.c > > > @@ -403,6 +403,6 @@ const struct dma_map_ops xen_swiotlb_dma_ops = { > > > .dma_supported = xen_swiotlb_dma_supported, > > > .mmap = dma_common_mmap, > > > .get_sgtable = dma_common_get_sgtable, > > > - .alloc_pages = dma_common_alloc_pages, > > > + .alloc_pages_op = dma_common_alloc_pages, > > > .free_pages = dma_common_free_pages, > > > }; > > > diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h > > > index 31f114f486c4..d741940dcb3b 100644 > > > --- a/include/linux/dma-map-ops.h > > > +++ b/include/linux/dma-map-ops.h > > > @@ -27,7 +27,7 @@ struct dma_map_ops { > > > unsigned long attrs); > > > void (*free)(struct device *dev, size_t size, void *vaddr, > > > dma_addr_t dma_handle, unsigned long attrs); > > > - struct page *(*alloc_pages)(struct device *dev, size_t size, > > > + struct page *(*alloc_pages_op)(struct device *dev, size_t size, > > > dma_addr_t *dma_handle, enum dma_data_direction dir, > > > gfp_t gfp); > > > void (*free_pages)(struct device *dev, size_t size, struct page *vaddr, > > > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c > > > index 9a4db5cce600..fc42930af14b 100644 > > > --- a/kernel/dma/mapping.c > > > +++ b/kernel/dma/mapping.c > > > @@ -570,9 +570,9 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size, > > > size = PAGE_ALIGN(size); > > > if (dma_alloc_direct(dev, ops)) > > > return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp); > > > - if (!ops->alloc_pages) > > > + if (!ops->alloc_pages_op) > > > return NULL; > > > - return ops->alloc_pages(dev, size, dma_handle, dir, gfp); > > > + return ops->alloc_pages_op(dev, size, dma_handle, dir, gfp); > > > } > > > > > > struct page *dma_alloc_pages(struct device *dev, size_t size, > > > > I'm not impressed. This patch increases churn for code which does not > > (directly) benefit from the change, and that for limitations in your > > tooling? > > > > Why not just rename the conflicting uses in your local tree, but then > > remove the rename from the final patch series? > > With alloc_pages function becoming a macro, the preprocessor ends up > replacing all instances of that name, even when it's not used as a > function. That what necessitates this change. If there is a way to > work around this issue without changing all alloc_pages() calls in the > source base I would love to learn it but I'm not quite clear about > your suggestion and if it solves the issue. Could you please provide > more details? Ah, right, I admit I did not quite understand why this change is needed. However, this is exactly what I don't like about preprocessor macros. Each macro effectively adds a new keyword to the language. I believe everything can be solved with inline functions. What exactly does not work if you rename alloc_pages() to e.g. alloc_pages_caller() and then add an alloc_pages() inline function which calls alloc_pages_caller() with _RET_IP_ as a parameter? Petr T
On Tue, May 02, 2023 at 10:09:09PM +0200, Petr Tesařík wrote: > Ah, right, I admit I did not quite understand why this change is > needed. However, this is exactly what I don't like about preprocessor > macros. Each macro effectively adds a new keyword to the language. > > I believe everything can be solved with inline functions. What exactly > does not work if you rename alloc_pages() to e.g. alloc_pages_caller() > and then add an alloc_pages() inline function which calls > alloc_pages_caller() with _RET_IP_ as a parameter? Perhaps you should spend a little more time reading the patchset and learning how the code works before commenting.
On Tue, May 2, 2023 at 1:09 PM Petr Tesařík <petr@tesarici.cz> wrote: > > On Tue, 2 May 2023 11:38:49 -0700 > Suren Baghdasaryan <surenb@google.com> wrote: > > > On Tue, May 2, 2023 at 8:50 AM Petr Tesařík <petr@tesarici.cz> wrote: > > > > > > On Mon, 1 May 2023 09:54:29 -0700 > > > Suren Baghdasaryan <surenb@google.com> wrote: > > > > > > > After redefining alloc_pages, all uses of that name are being replaced. > > > > Change the conflicting names to prevent preprocessor from replacing them > > > > when it's not intended. > > > > > > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > > > --- > > > > arch/x86/kernel/amd_gart_64.c | 2 +- > > > > drivers/iommu/dma-iommu.c | 2 +- > > > > drivers/xen/grant-dma-ops.c | 2 +- > > > > drivers/xen/swiotlb-xen.c | 2 +- > > > > include/linux/dma-map-ops.h | 2 +- > > > > kernel/dma/mapping.c | 4 ++-- > > > > 6 files changed, 7 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c > > > > index 56a917df410d..842a0ec5eaa9 100644 > > > > --- a/arch/x86/kernel/amd_gart_64.c > > > > +++ b/arch/x86/kernel/amd_gart_64.c > > > > @@ -676,7 +676,7 @@ static const struct dma_map_ops gart_dma_ops = { > > > > .get_sgtable = dma_common_get_sgtable, > > > > .dma_supported = dma_direct_supported, > > > > .get_required_mask = dma_direct_get_required_mask, > > > > - .alloc_pages = dma_direct_alloc_pages, > > > > + .alloc_pages_op = dma_direct_alloc_pages, > > > > .free_pages = dma_direct_free_pages, > > > > }; > > > > > > > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > > > index 7a9f0b0bddbd..76a9d5ca4eee 100644 > > > > --- a/drivers/iommu/dma-iommu.c > > > > +++ b/drivers/iommu/dma-iommu.c > > > > @@ -1556,7 +1556,7 @@ static const struct dma_map_ops iommu_dma_ops = { > > > > .flags = DMA_F_PCI_P2PDMA_SUPPORTED, > > > > .alloc = iommu_dma_alloc, > > > > .free = iommu_dma_free, > > > > - .alloc_pages = dma_common_alloc_pages, > > > > + .alloc_pages_op = dma_common_alloc_pages, > > > > .free_pages = dma_common_free_pages, > > > > .alloc_noncontiguous = iommu_dma_alloc_noncontiguous, > > > > .free_noncontiguous = iommu_dma_free_noncontiguous, > > > > diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c > > > > index 9784a77fa3c9..6c7d984f164d 100644 > > > > --- a/drivers/xen/grant-dma-ops.c > > > > +++ b/drivers/xen/grant-dma-ops.c > > > > @@ -282,7 +282,7 @@ static int xen_grant_dma_supported(struct device *dev, u64 mask) > > > > static const struct dma_map_ops xen_grant_dma_ops = { > > > > .alloc = xen_grant_dma_alloc, > > > > .free = xen_grant_dma_free, > > > > - .alloc_pages = xen_grant_dma_alloc_pages, > > > > + .alloc_pages_op = xen_grant_dma_alloc_pages, > > > > .free_pages = xen_grant_dma_free_pages, > > > > .mmap = dma_common_mmap, > > > > .get_sgtable = dma_common_get_sgtable, > > > > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c > > > > index 67aa74d20162..5ab2616153f0 100644 > > > > --- a/drivers/xen/swiotlb-xen.c > > > > +++ b/drivers/xen/swiotlb-xen.c > > > > @@ -403,6 +403,6 @@ const struct dma_map_ops xen_swiotlb_dma_ops = { > > > > .dma_supported = xen_swiotlb_dma_supported, > > > > .mmap = dma_common_mmap, > > > > .get_sgtable = dma_common_get_sgtable, > > > > - .alloc_pages = dma_common_alloc_pages, > > > > + .alloc_pages_op = dma_common_alloc_pages, > > > > .free_pages = dma_common_free_pages, > > > > }; > > > > diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h > > > > index 31f114f486c4..d741940dcb3b 100644 > > > > --- a/include/linux/dma-map-ops.h > > > > +++ b/include/linux/dma-map-ops.h > > > > @@ -27,7 +27,7 @@ struct dma_map_ops { > > > > unsigned long attrs); > > > > void (*free)(struct device *dev, size_t size, void *vaddr, > > > > dma_addr_t dma_handle, unsigned long attrs); > > > > - struct page *(*alloc_pages)(struct device *dev, size_t size, > > > > + struct page *(*alloc_pages_op)(struct device *dev, size_t size, > > > > dma_addr_t *dma_handle, enum dma_data_direction dir, > > > > gfp_t gfp); > > > > void (*free_pages)(struct device *dev, size_t size, struct page *vaddr, > > > > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c > > > > index 9a4db5cce600..fc42930af14b 100644 > > > > --- a/kernel/dma/mapping.c > > > > +++ b/kernel/dma/mapping.c > > > > @@ -570,9 +570,9 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size, > > > > size = PAGE_ALIGN(size); > > > > if (dma_alloc_direct(dev, ops)) > > > > return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp); > > > > - if (!ops->alloc_pages) > > > > + if (!ops->alloc_pages_op) > > > > return NULL; > > > > - return ops->alloc_pages(dev, size, dma_handle, dir, gfp); > > > > + return ops->alloc_pages_op(dev, size, dma_handle, dir, gfp); > > > > } > > > > > > > > struct page *dma_alloc_pages(struct device *dev, size_t size, > > > > > > I'm not impressed. This patch increases churn for code which does not > > > (directly) benefit from the change, and that for limitations in your > > > tooling? > > > > > > Why not just rename the conflicting uses in your local tree, but then > > > remove the rename from the final patch series? > > > > With alloc_pages function becoming a macro, the preprocessor ends up > > replacing all instances of that name, even when it's not used as a > > function. That what necessitates this change. If there is a way to > > work around this issue without changing all alloc_pages() calls in the > > source base I would love to learn it but I'm not quite clear about > > your suggestion and if it solves the issue. Could you please provide > > more details? > > Ah, right, I admit I did not quite understand why this change is > needed. However, this is exactly what I don't like about preprocessor > macros. Each macro effectively adds a new keyword to the language. > > I believe everything can be solved with inline functions. What exactly > does not work if you rename alloc_pages() to e.g. alloc_pages_caller() > and then add an alloc_pages() inline function which calls > alloc_pages_caller() with _RET_IP_ as a parameter? I don't think that would work because we need to inject the codetag at the file/line of the actual allocation call. If we pass _REP_IT_ then we would have to lookup the codetag associated with that _RET_IP_ which results in additional runtime overhead. > > Petr T > > -- > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com. >
On Tue, 2 May 2023 13:24:37 -0700 Suren Baghdasaryan <surenb@google.com> wrote: > On Tue, May 2, 2023 at 1:09 PM Petr Tesařík <petr@tesarici.cz> wrote: > > > > On Tue, 2 May 2023 11:38:49 -0700 > > Suren Baghdasaryan <surenb@google.com> wrote: > > > > > On Tue, May 2, 2023 at 8:50 AM Petr Tesařík <petr@tesarici.cz> wrote: > > > > > > > > On Mon, 1 May 2023 09:54:29 -0700 > > > > Suren Baghdasaryan <surenb@google.com> wrote: > > > > > > > > > After redefining alloc_pages, all uses of that name are being replaced. > > > > > Change the conflicting names to prevent preprocessor from replacing them > > > > > when it's not intended. > > > > > > > > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > > > > --- > > > > > arch/x86/kernel/amd_gart_64.c | 2 +- > > > > > drivers/iommu/dma-iommu.c | 2 +- > > > > > drivers/xen/grant-dma-ops.c | 2 +- > > > > > drivers/xen/swiotlb-xen.c | 2 +- > > > > > include/linux/dma-map-ops.h | 2 +- > > > > > kernel/dma/mapping.c | 4 ++-- > > > > > 6 files changed, 7 insertions(+), 7 deletions(-) > > > > > > > > > > diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c > > > > > index 56a917df410d..842a0ec5eaa9 100644 > > > > > --- a/arch/x86/kernel/amd_gart_64.c > > > > > +++ b/arch/x86/kernel/amd_gart_64.c > > > > > @@ -676,7 +676,7 @@ static const struct dma_map_ops gart_dma_ops = { > > > > > .get_sgtable = dma_common_get_sgtable, > > > > > .dma_supported = dma_direct_supported, > > > > > .get_required_mask = dma_direct_get_required_mask, > > > > > - .alloc_pages = dma_direct_alloc_pages, > > > > > + .alloc_pages_op = dma_direct_alloc_pages, > > > > > .free_pages = dma_direct_free_pages, > > > > > }; > > > > > > > > > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > > > > index 7a9f0b0bddbd..76a9d5ca4eee 100644 > > > > > --- a/drivers/iommu/dma-iommu.c > > > > > +++ b/drivers/iommu/dma-iommu.c > > > > > @@ -1556,7 +1556,7 @@ static const struct dma_map_ops iommu_dma_ops = { > > > > > .flags = DMA_F_PCI_P2PDMA_SUPPORTED, > > > > > .alloc = iommu_dma_alloc, > > > > > .free = iommu_dma_free, > > > > > - .alloc_pages = dma_common_alloc_pages, > > > > > + .alloc_pages_op = dma_common_alloc_pages, > > > > > .free_pages = dma_common_free_pages, > > > > > .alloc_noncontiguous = iommu_dma_alloc_noncontiguous, > > > > > .free_noncontiguous = iommu_dma_free_noncontiguous, > > > > > diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c > > > > > index 9784a77fa3c9..6c7d984f164d 100644 > > > > > --- a/drivers/xen/grant-dma-ops.c > > > > > +++ b/drivers/xen/grant-dma-ops.c > > > > > @@ -282,7 +282,7 @@ static int xen_grant_dma_supported(struct device *dev, u64 mask) > > > > > static const struct dma_map_ops xen_grant_dma_ops = { > > > > > .alloc = xen_grant_dma_alloc, > > > > > .free = xen_grant_dma_free, > > > > > - .alloc_pages = xen_grant_dma_alloc_pages, > > > > > + .alloc_pages_op = xen_grant_dma_alloc_pages, > > > > > .free_pages = xen_grant_dma_free_pages, > > > > > .mmap = dma_common_mmap, > > > > > .get_sgtable = dma_common_get_sgtable, > > > > > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c > > > > > index 67aa74d20162..5ab2616153f0 100644 > > > > > --- a/drivers/xen/swiotlb-xen.c > > > > > +++ b/drivers/xen/swiotlb-xen.c > > > > > @@ -403,6 +403,6 @@ const struct dma_map_ops xen_swiotlb_dma_ops = { > > > > > .dma_supported = xen_swiotlb_dma_supported, > > > > > .mmap = dma_common_mmap, > > > > > .get_sgtable = dma_common_get_sgtable, > > > > > - .alloc_pages = dma_common_alloc_pages, > > > > > + .alloc_pages_op = dma_common_alloc_pages, > > > > > .free_pages = dma_common_free_pages, > > > > > }; > > > > > diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h > > > > > index 31f114f486c4..d741940dcb3b 100644 > > > > > --- a/include/linux/dma-map-ops.h > > > > > +++ b/include/linux/dma-map-ops.h > > > > > @@ -27,7 +27,7 @@ struct dma_map_ops { > > > > > unsigned long attrs); > > > > > void (*free)(struct device *dev, size_t size, void *vaddr, > > > > > dma_addr_t dma_handle, unsigned long attrs); > > > > > - struct page *(*alloc_pages)(struct device *dev, size_t size, > > > > > + struct page *(*alloc_pages_op)(struct device *dev, size_t size, > > > > > dma_addr_t *dma_handle, enum dma_data_direction dir, > > > > > gfp_t gfp); > > > > > void (*free_pages)(struct device *dev, size_t size, struct page *vaddr, > > > > > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c > > > > > index 9a4db5cce600..fc42930af14b 100644 > > > > > --- a/kernel/dma/mapping.c > > > > > +++ b/kernel/dma/mapping.c > > > > > @@ -570,9 +570,9 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size, > > > > > size = PAGE_ALIGN(size); > > > > > if (dma_alloc_direct(dev, ops)) > > > > > return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp); > > > > > - if (!ops->alloc_pages) > > > > > + if (!ops->alloc_pages_op) > > > > > return NULL; > > > > > - return ops->alloc_pages(dev, size, dma_handle, dir, gfp); > > > > > + return ops->alloc_pages_op(dev, size, dma_handle, dir, gfp); > > > > > } > > > > > > > > > > struct page *dma_alloc_pages(struct device *dev, size_t size, > > > > > > > > I'm not impressed. This patch increases churn for code which does not > > > > (directly) benefit from the change, and that for limitations in your > > > > tooling? > > > > > > > > Why not just rename the conflicting uses in your local tree, but then > > > > remove the rename from the final patch series? > > > > > > With alloc_pages function becoming a macro, the preprocessor ends up > > > replacing all instances of that name, even when it's not used as a > > > function. That what necessitates this change. If there is a way to > > > work around this issue without changing all alloc_pages() calls in the > > > source base I would love to learn it but I'm not quite clear about > > > your suggestion and if it solves the issue. Could you please provide > > > more details? > > > > Ah, right, I admit I did not quite understand why this change is > > needed. However, this is exactly what I don't like about preprocessor > > macros. Each macro effectively adds a new keyword to the language. > > > > I believe everything can be solved with inline functions. What exactly > > does not work if you rename alloc_pages() to e.g. alloc_pages_caller() > > and then add an alloc_pages() inline function which calls > > alloc_pages_caller() with _RET_IP_ as a parameter? > > I don't think that would work because we need to inject the codetag at > the file/line of the actual allocation call. If we pass _REP_IT_ then > we would have to lookup the codetag associated with that _RET_IP_ > which results in additional runtime overhead. OK. If the reference to source code itself must be recorded in the kernel, and not resolved later (either by the debugfs read fops, or by a tool which reads the file), then this information can only be obtained with a preprocessor macro. I was hoping that a debugging feature could be less intrusive. OTOH it's not my call to balance the tradeoffs. Thank you for your patient explanations. Petr T
On Tue, May 2, 2023 at 1:39 PM Petr Tesařík <petr@tesarici.cz> wrote: > > On Tue, 2 May 2023 13:24:37 -0700 > Suren Baghdasaryan <surenb@google.com> wrote: > > > On Tue, May 2, 2023 at 1:09 PM Petr Tesařík <petr@tesarici.cz> wrote: > > > > > > On Tue, 2 May 2023 11:38:49 -0700 > > > Suren Baghdasaryan <surenb@google.com> wrote: > > > > > > > On Tue, May 2, 2023 at 8:50 AM Petr Tesařík <petr@tesarici.cz> wrote: > > > > > > > > > > On Mon, 1 May 2023 09:54:29 -0700 > > > > > Suren Baghdasaryan <surenb@google.com> wrote: > > > > > > > > > > > After redefining alloc_pages, all uses of that name are being replaced. > > > > > > Change the conflicting names to prevent preprocessor from replacing them > > > > > > when it's not intended. > > > > > > > > > > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > > > > > --- > > > > > > arch/x86/kernel/amd_gart_64.c | 2 +- > > > > > > drivers/iommu/dma-iommu.c | 2 +- > > > > > > drivers/xen/grant-dma-ops.c | 2 +- > > > > > > drivers/xen/swiotlb-xen.c | 2 +- > > > > > > include/linux/dma-map-ops.h | 2 +- > > > > > > kernel/dma/mapping.c | 4 ++-- > > > > > > 6 files changed, 7 insertions(+), 7 deletions(-) > > > > > > > > > > > > diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c > > > > > > index 56a917df410d..842a0ec5eaa9 100644 > > > > > > --- a/arch/x86/kernel/amd_gart_64.c > > > > > > +++ b/arch/x86/kernel/amd_gart_64.c > > > > > > @@ -676,7 +676,7 @@ static const struct dma_map_ops gart_dma_ops = { > > > > > > .get_sgtable = dma_common_get_sgtable, > > > > > > .dma_supported = dma_direct_supported, > > > > > > .get_required_mask = dma_direct_get_required_mask, > > > > > > - .alloc_pages = dma_direct_alloc_pages, > > > > > > + .alloc_pages_op = dma_direct_alloc_pages, > > > > > > .free_pages = dma_direct_free_pages, > > > > > > }; > > > > > > > > > > > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > > > > > index 7a9f0b0bddbd..76a9d5ca4eee 100644 > > > > > > --- a/drivers/iommu/dma-iommu.c > > > > > > +++ b/drivers/iommu/dma-iommu.c > > > > > > @@ -1556,7 +1556,7 @@ static const struct dma_map_ops iommu_dma_ops = { > > > > > > .flags = DMA_F_PCI_P2PDMA_SUPPORTED, > > > > > > .alloc = iommu_dma_alloc, > > > > > > .free = iommu_dma_free, > > > > > > - .alloc_pages = dma_common_alloc_pages, > > > > > > + .alloc_pages_op = dma_common_alloc_pages, > > > > > > .free_pages = dma_common_free_pages, > > > > > > .alloc_noncontiguous = iommu_dma_alloc_noncontiguous, > > > > > > .free_noncontiguous = iommu_dma_free_noncontiguous, > > > > > > diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c > > > > > > index 9784a77fa3c9..6c7d984f164d 100644 > > > > > > --- a/drivers/xen/grant-dma-ops.c > > > > > > +++ b/drivers/xen/grant-dma-ops.c > > > > > > @@ -282,7 +282,7 @@ static int xen_grant_dma_supported(struct device *dev, u64 mask) > > > > > > static const struct dma_map_ops xen_grant_dma_ops = { > > > > > > .alloc = xen_grant_dma_alloc, > > > > > > .free = xen_grant_dma_free, > > > > > > - .alloc_pages = xen_grant_dma_alloc_pages, > > > > > > + .alloc_pages_op = xen_grant_dma_alloc_pages, > > > > > > .free_pages = xen_grant_dma_free_pages, > > > > > > .mmap = dma_common_mmap, > > > > > > .get_sgtable = dma_common_get_sgtable, > > > > > > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c > > > > > > index 67aa74d20162..5ab2616153f0 100644 > > > > > > --- a/drivers/xen/swiotlb-xen.c > > > > > > +++ b/drivers/xen/swiotlb-xen.c > > > > > > @@ -403,6 +403,6 @@ const struct dma_map_ops xen_swiotlb_dma_ops = { > > > > > > .dma_supported = xen_swiotlb_dma_supported, > > > > > > .mmap = dma_common_mmap, > > > > > > .get_sgtable = dma_common_get_sgtable, > > > > > > - .alloc_pages = dma_common_alloc_pages, > > > > > > + .alloc_pages_op = dma_common_alloc_pages, > > > > > > .free_pages = dma_common_free_pages, > > > > > > }; > > > > > > diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h > > > > > > index 31f114f486c4..d741940dcb3b 100644 > > > > > > --- a/include/linux/dma-map-ops.h > > > > > > +++ b/include/linux/dma-map-ops.h > > > > > > @@ -27,7 +27,7 @@ struct dma_map_ops { > > > > > > unsigned long attrs); > > > > > > void (*free)(struct device *dev, size_t size, void *vaddr, > > > > > > dma_addr_t dma_handle, unsigned long attrs); > > > > > > - struct page *(*alloc_pages)(struct device *dev, size_t size, > > > > > > + struct page *(*alloc_pages_op)(struct device *dev, size_t size, > > > > > > dma_addr_t *dma_handle, enum dma_data_direction dir, > > > > > > gfp_t gfp); > > > > > > void (*free_pages)(struct device *dev, size_t size, struct page *vaddr, > > > > > > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c > > > > > > index 9a4db5cce600..fc42930af14b 100644 > > > > > > --- a/kernel/dma/mapping.c > > > > > > +++ b/kernel/dma/mapping.c > > > > > > @@ -570,9 +570,9 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size, > > > > > > size = PAGE_ALIGN(size); > > > > > > if (dma_alloc_direct(dev, ops)) > > > > > > return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp); > > > > > > - if (!ops->alloc_pages) > > > > > > + if (!ops->alloc_pages_op) > > > > > > return NULL; > > > > > > - return ops->alloc_pages(dev, size, dma_handle, dir, gfp); > > > > > > + return ops->alloc_pages_op(dev, size, dma_handle, dir, gfp); > > > > > > } > > > > > > > > > > > > struct page *dma_alloc_pages(struct device *dev, size_t size, > > > > > > > > > > I'm not impressed. This patch increases churn for code which does not > > > > > (directly) benefit from the change, and that for limitations in your > > > > > tooling? > > > > > > > > > > Why not just rename the conflicting uses in your local tree, but then > > > > > remove the rename from the final patch series? > > > > > > > > With alloc_pages function becoming a macro, the preprocessor ends up > > > > replacing all instances of that name, even when it's not used as a > > > > function. That what necessitates this change. If there is a way to > > > > work around this issue without changing all alloc_pages() calls in the > > > > source base I would love to learn it but I'm not quite clear about > > > > your suggestion and if it solves the issue. Could you please provide > > > > more details? > > > > > > Ah, right, I admit I did not quite understand why this change is > > > needed. However, this is exactly what I don't like about preprocessor > > > macros. Each macro effectively adds a new keyword to the language. > > > > > > I believe everything can be solved with inline functions. What exactly > > > does not work if you rename alloc_pages() to e.g. alloc_pages_caller() > > > and then add an alloc_pages() inline function which calls > > > alloc_pages_caller() with _RET_IP_ as a parameter? > > > > I don't think that would work because we need to inject the codetag at > > the file/line of the actual allocation call. If we pass _REP_IT_ then > > we would have to lookup the codetag associated with that _RET_IP_ > > which results in additional runtime overhead. > > OK. If the reference to source code itself must be recorded in the > kernel, and not resolved later (either by the debugfs read fops, or by > a tool which reads the file), then this information can only be > obtained with a preprocessor macro. > > I was hoping that a debugging feature could be less intrusive. OTOH > it's not my call to balance the tradeoffs. > > Thank you for your patient explanations. Thanks for reviewing and the suggestions! I'll address the actionable ones in the next version. Suren. > > Petr T > > -- > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com. >
On Mon, 1 May 2023 09:54:29 -0700 Suren Baghdasaryan <surenb@google.com> wrote: > After redefining alloc_pages, all uses of that name are being replaced. > Change the conflicting names to prevent preprocessor from replacing them > when it's not intended. Note, every change log should have enough information in it to know why it is being done. This says what the patch does, but does not fully explain "why". It should never be assumed that one must read other patches to get the context. A year from now, investigating git history, this may be the only thing someone sees for why this change occurred. The "why" above is simply "prevent preprocessor from replacing them when it's not intended". What does that mean? -- Steve > > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
On Wed, May 3, 2023 at 9:25 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Mon, 1 May 2023 09:54:29 -0700 > Suren Baghdasaryan <surenb@google.com> wrote: > > > After redefining alloc_pages, all uses of that name are being replaced. > > Change the conflicting names to prevent preprocessor from replacing them > > when it's not intended. > > Note, every change log should have enough information in it to know why it > is being done. This says what the patch does, but does not fully explain > "why". It should never be assumed that one must read other patches to get > the context. A year from now, investigating git history, this may be the > only thing someone sees for why this change occurred. > > The "why" above is simply "prevent preprocessor from replacing them > when it's not intended". What does that mean? Thanks for the feedback, Steve. I'll make appropriate modifications to the description. > > -- Steve > > > > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > -- > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com. >
diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c index 56a917df410d..842a0ec5eaa9 100644 --- a/arch/x86/kernel/amd_gart_64.c +++ b/arch/x86/kernel/amd_gart_64.c @@ -676,7 +676,7 @@ static const struct dma_map_ops gart_dma_ops = { .get_sgtable = dma_common_get_sgtable, .dma_supported = dma_direct_supported, .get_required_mask = dma_direct_get_required_mask, - .alloc_pages = dma_direct_alloc_pages, + .alloc_pages_op = dma_direct_alloc_pages, .free_pages = dma_direct_free_pages, }; diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 7a9f0b0bddbd..76a9d5ca4eee 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -1556,7 +1556,7 @@ static const struct dma_map_ops iommu_dma_ops = { .flags = DMA_F_PCI_P2PDMA_SUPPORTED, .alloc = iommu_dma_alloc, .free = iommu_dma_free, - .alloc_pages = dma_common_alloc_pages, + .alloc_pages_op = dma_common_alloc_pages, .free_pages = dma_common_free_pages, .alloc_noncontiguous = iommu_dma_alloc_noncontiguous, .free_noncontiguous = iommu_dma_free_noncontiguous, diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c index 9784a77fa3c9..6c7d984f164d 100644 --- a/drivers/xen/grant-dma-ops.c +++ b/drivers/xen/grant-dma-ops.c @@ -282,7 +282,7 @@ static int xen_grant_dma_supported(struct device *dev, u64 mask) static const struct dma_map_ops xen_grant_dma_ops = { .alloc = xen_grant_dma_alloc, .free = xen_grant_dma_free, - .alloc_pages = xen_grant_dma_alloc_pages, + .alloc_pages_op = xen_grant_dma_alloc_pages, .free_pages = xen_grant_dma_free_pages, .mmap = dma_common_mmap, .get_sgtable = dma_common_get_sgtable, diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index 67aa74d20162..5ab2616153f0 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -403,6 +403,6 @@ const struct dma_map_ops xen_swiotlb_dma_ops = { .dma_supported = xen_swiotlb_dma_supported, .mmap = dma_common_mmap, .get_sgtable = dma_common_get_sgtable, - .alloc_pages = dma_common_alloc_pages, + .alloc_pages_op = dma_common_alloc_pages, .free_pages = dma_common_free_pages, }; diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h index 31f114f486c4..d741940dcb3b 100644 --- a/include/linux/dma-map-ops.h +++ b/include/linux/dma-map-ops.h @@ -27,7 +27,7 @@ struct dma_map_ops { unsigned long attrs); void (*free)(struct device *dev, size_t size, void *vaddr, dma_addr_t dma_handle, unsigned long attrs); - struct page *(*alloc_pages)(struct device *dev, size_t size, + struct page *(*alloc_pages_op)(struct device *dev, size_t size, dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp); void (*free_pages)(struct device *dev, size_t size, struct page *vaddr, diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c index 9a4db5cce600..fc42930af14b 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -570,9 +570,9 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size, size = PAGE_ALIGN(size); if (dma_alloc_direct(dev, ops)) return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp); - if (!ops->alloc_pages) + if (!ops->alloc_pages_op) return NULL; - return ops->alloc_pages(dev, size, dma_handle, dir, gfp); + return ops->alloc_pages_op(dev, size, dma_handle, dir, gfp); } struct page *dma_alloc_pages(struct device *dev, size_t size,
After redefining alloc_pages, all uses of that name are being replaced. Change the conflicting names to prevent preprocessor from replacing them when it's not intended. Signed-off-by: Suren Baghdasaryan <surenb@google.com> --- arch/x86/kernel/amd_gart_64.c | 2 +- drivers/iommu/dma-iommu.c | 2 +- drivers/xen/grant-dma-ops.c | 2 +- drivers/xen/swiotlb-xen.c | 2 +- include/linux/dma-map-ops.h | 2 +- kernel/dma/mapping.c | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-)