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 ? gid_table_release_one() calls gid_table_release -- which frees the gid table and sets its pointer to NULL. Then, if ib_cache_release_one is called later, gid_table_release_one() will simply do nothing (it calls gid_table_release, which returns without doing anything if the table pointer argument is NULL -- which it will be). Thus, unlike ib_cache_release_one() -- gid_table_release_one() is callable multiple times. This also has the advantage of unwinding the gid_table_setup_one() in the ib_cache_setup_one() error flow -- which is a symmetric unwind. -Jack
