On 27-Mar-19 11:33, Leon Romanovsky wrote: > On Wed, Mar 27, 2019 at 11:01:21AM +0200, Gal Pressman wrote: >> On 26-Mar-19 16:56, Leon Romanovsky wrote: >>> From: Leon Romanovsky <leonro@xxxxxxxxxxxx> >>> >>> Hi, >>> >>> This is continuation of our conversion from driver allocations to core >>> responsibility. Current series converts SRQ and AH, while latter >>> required some code preparation. >>> >>> The most challenge was in supporting AH to be allocated with GFP_ATOMIC >>> or GFP_KERNEL according to context. The common solution is to use in_atomic() >>> and in_interrupt() already used in many drivers, but unfortunately >>> checkpatch produces warning. >>> >>> WARNING: use of in_atomic() is incorrect outside core kernel code >>> #37: FILE: include/rdma/ib_verbs.h:2288: >>> + (in_atomic() || in_interrupt()) ? GFP_ATOMIC : GFP_KERNEL)) >> >> Hi Leon, >> >> The existing method to test for create AH atomic flow is using the flags >> parameters (RDMA_CREATE_AH_SLEEPABLE). >> I suggest you add a gfp_flags parameter to rdma_zalloc_drv_obj() and pass >> GFP_ATOMIC in case RDMA_CREATE_AH_SLEEPABLE flag is set. > > Hi Gal, > > It can be a possible solution too. > > Before I'm changing all rdma_zalloc_drv_obj() or introduce new similar helper, > can you give me the rationale behind such change? > > Most of the objects need GFP_KERNEL allocations and I'm having a hard time > to justify for myself the need "to punish" API just because of checkpatch > warning, while this "automatic" mode does its job to determine context. I don't know if the checkpatch warning is justified or not but if there's an existing way to identify the atomic flow then it should be used. Using in_atomic() for some cases and the flags for other cases makes for inconsistent code. Perhaps the "automatic" mode should also replace the 'flags' parameter, but IMO as long as the flags exist they should be used.
