Re: [PATCH rdma-next 0/4] Convert AH and SRQ to core allocation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 27, 2019 at 11:44:52AM +0200, Gal Pressman wrote:
> 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.

We can revert b090c4e3a07c ("RDMA: Mark if create address handle is in
a sleepable context") if it is needed.

Thanks

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux