On Wed, May 06, 2020 at 03:57:37PM -0300, Jason Gunthorpe wrote: > On Wed, May 06, 2020 at 09:41:23PM +0300, jackm wrote: > > On Wed, 6 May 2020 15:09:36 -0300 > > Jason Gunthorpe <jgg@xxxxxxxx> wrote: > > > > > > > > +out: > > > > > > + ib_cache_release_one(device); > > > > > > + return err; > > > > > > > > > > ib_cache_release_once can be called only once, and it is always > > > > > called by ib_device_release(), it should not be called here > > > > > > > > It doesn't sound right if we rely on ib_device_release() to unwind > > > > error in ib_cache_setup_one(). I don't think that we need to return > > > > from ib_cache_setup_one() without cleaning it. > > > > > > We do as ib_cache_release_one() cannot be called multiple times > > > > > > The general design of all this pre-registration stuff is that the > > > release function does the clean up and the individual functions should > > > not error unwind cleanup done in the unconditional release. > > > > > > Other schemes were too complicated > > > > > > Jason > > > > What about calling gid_table_release_one(device) instead of > > ib_cache_release_one(device) in the error flow ? > > Why? Because it doesn't look clean. > > That is not the design, everything that is freed by release is defered > to release, even on error paths. I'll resend now. Thanks > > Jason
