Re: [PATCH rdma-next 6/6] IB/mlx5: Move locks initialization to the corresponding stage

Mark Bloch
2017-12-28T17:22:52Z

On 28/12/2017 19:14, Jason Gunthorpe wrote:
> On Thu, Dec 28, 2017 at 07:10:31PM +0200, Mark Bloch wrote:
>
>> Well, it's not only that, moving the init srcu to the INIT stage
>> will also make me wrap it around with CONFIG_INFINIBAND_ON_DEMAND_PAGING
>> which I really don't like seeing in .c files, but we are already doing that
>> so .... :)
>
> Don't micro-optimize so much. Who cares if the few struct members are
> allocated when !CONFIG_INFINIBAND_ON_DEMAND_PAGING ? Always allocate
> the SRCU and always initialize it.
>
Well, I didn't write the ODP code, so I don't know why they did it this way.
Any place today that accesses that srcu is already under that CONFIG option, so
the init srcu can also stay under that CONFIG.

> Maybe even get rid of CONFIG_INFINIBAND_ON_DEMAND_PAGING ?
> Never understood why we had it in the first place.
>

I'll talk with the guys that did/touch last the ODP code and see what they have
to say, if there isn't a valid reason I'll post a patch that cleans this entire thing.

>> If you feel that strongly about it, let me know and I'll change that,
>> as well as the comment you had about the const in a different patch
>> and we'll send v1 with the fixes.
>
> The function call back thing is really not nice, so I would be happy
> to see it go away.
>
So I'll move the lock to INIT, but leave it under the ODP CONFIG option for now.
If there isn't (It's already on my TODO list to ask around) a good reason for this CONFIG
I'll send a patch that addresses that, sound good?

> Jason
>

Mark