On 2023/6/9 23:02, Jesper Dangaard Brouer wrote: ... >> PP_FLAG_DMA_SYNC_DEV |\ >> PP_FLAG_PAGE_FRAG) >> +#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \ >> + (sizeof(dma_addr_t) > sizeof(unsigned long)) >> + > > I have a problem with the name PAGE_POOL_DMA_USE_PP_FRAG_COUNT > because it is confusing to read in an if-statement. Actually, it is already in an if-statement before this patch:) Maybe starting to use it in the driver is confusing to you? If not, maybe we can keep it that for now, and change it when we come up with a better name. > > Proposals rename to: DMA_OVERLAP_PP_FRAG_COUNT > Or: MM_DMA_OVERLAP_PP_FRAG_COUNT > Or: DMA_ADDR_OVERLAP_PP_FRAG_COUNT It seems DMA_ADDR_OVERLAP_PP_FRAG_COUNT is better, and DMA_ADDR_UPPER_OVERLAP_PP_FRAG_COUNT seems more accurate if a longer macro name is not an issue here. > > Notice how I also removed the prefix PAGE_POOL_ because this is a MM-layer constraint and not a property of page_pool. I am not sure if it is a MM-layer constraint yet. Do you mean 'MM-layer constraint' as 'struct page' not having enough space for page pool with 32-bit arch with 64-bit DMA? If that is the case, we may need a more generic name for that constraint instead of 'DMA_ADDR_OVERLAP_PP_FRAG_COUNT'? And a more generic name seems confusing for page pool too, as it doesn't tell that we only have that problem for 32-bit arch with 64-bit DMA. So if the above makes sense, it seems we may need to keep the PAGE_POOL_ prefix, which would be 'PAGE_POOL_DMA_ADDR_UPPER_OVERLAP_PP_FRAG_COUNT' if the long name is not issue here. Anyway, naming is hard, we may need a seperate patch to explain it, which is not really related to this patchset IHMO, so I'd rather keep it as before if we can not come up with a name which is not confusing to most people. > > > --Jesper > >
