Re: [PATCH v9 11/24] mm/hmm: provide generic DMA managing logic

[Leon Romanovsky <leon@xxxxxxxxxx>]

On Thu, Apr 24, 2025 at 10:15:45AM +0300, Leon Romanovsky wrote:
> On Wed, Apr 23, 2025 at 02:28:56PM -0300, Jason Gunthorpe wrote:
> > On Wed, Apr 23, 2025 at 11:13:02AM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@xxxxxxxxxx>

<...>

> > > +bool hmm_dma_unmap_pfn(struct device *dev, struct hmm_dma_map *map, size_t idx)
> > > +{
> > > +	struct dma_iova_state *state = &map->state;
> > > +	dma_addr_t *dma_addrs = map->dma_list;
> > > +	unsigned long *pfns = map->pfn_list;
> > > +	unsigned long attrs = 0;
> > > +
> > > +#define HMM_PFN_VALID_DMA (HMM_PFN_VALID | HMM_PFN_DMA_MAPPED)
> > > +	if ((pfns[idx] & HMM_PFN_VALID_DMA) != HMM_PFN_VALID_DMA)
> > > +		return false;
> > > +#undef HMM_PFN_VALID_DMA
> > 
> > If a v10 comes I'd put this in a const function level variable:
> > 
> >           const unsigned int HMM_PFN_VALID_DMA = HMM_PFN_VALID | HMM_PFN_DMA_MAPPED;

diff --git a/mm/hmm.c b/mm/hmm.c
index c0bee5aa00fc..a8bf097677f3 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -807,15 +807,14 @@ EXPORT_SYMBOL_GPL(hmm_dma_map_pfn);
  */
 bool hmm_dma_unmap_pfn(struct device *dev, struct hmm_dma_map *map, size_t idx)
 {
+       const unsigned long valid_dma = HMM_PFN_VALID | HMM_PFN_DMA_MAPPED;
        struct dma_iova_state *state = &map->state;
        dma_addr_t *dma_addrs = map->dma_list;
        unsigned long *pfns = map->pfn_list;
        unsigned long attrs = 0;
 
-#define HMM_PFN_VALID_DMA (HMM_PFN_VALID | HMM_PFN_DMA_MAPPED)
-       if ((pfns[idx] & HMM_PFN_VALID_DMA) != HMM_PFN_VALID_DMA)
+       if ((pfns[idx] & valid_dma) != valid_dma)
                return false;
-#undef HMM_PFN_VALID_DMA
 
        if (pfns[idx] & HMM_PFN_P2PDMA_BUS)
                ; /* no need to unmap bus address P2P mappings */