Messages in this thread | | | Subject | Re: [PATCH 1/2 v2] dma-iommu: use static-key to minimize the impact in the fast-path | From | Robin Murphy <> | Date | Tue, 19 Jan 2021 15:52:41 +0000 |
| |
On 2021-01-19 15:26, Christoph Hellwig wrote: > On Tue, Jan 19, 2021 at 07:16:15PM +0800, Lianbo Jiang wrote: >> +static DEFINE_STATIC_KEY_FALSE(__deferred_attach); > > Why the strange underscores? Wouldn't iommu_deferred_attach_enabled > be a better name? > >> - if (unlikely(iommu_dma_deferred_attach(dev, domain))) >> + if (static_branch_unlikely(&__deferred_attach) && >> + iommu_dma_deferred_attach(dev, domain)) > > Also insted of duplicating this logic in three places, maybe rename > iommu_dma_deferred_attach to __iommu_dma_deferred_attach and create > a small inline wrapper for it?
Once patch #2 is in place, I really don't see any point. The "helper" would add a minimum of 5 lines to save at most 3, and would have to be annotated as always_inline - which a whole other camp of people would probably object to - in order for the static branch to be properly useful.
It's not as if this is a complex or hard-to-read expression, so IMO having 3 lines repeated 3 times is objectively better than having 2 lines necessarily repeated 3 times plus having to scroll up and find several more lines to follow what it's doing.
Robin.
| |