On Mon, Oct 17, 2022 at 12:38:50AM -0700, Christoph Hellwig wrote: > > > +DEFINE_XARRAY_ALLOC(tpg_xa); > > I think this wants to be marked static. Agree. > > +static int core_tpg_register_rtpi(struct se_portal_group *se_tpg) > > Can you use target_ instead of the weird historic core_ prefixes for > everything here? Every function in this file is with core_ prefix, but OK, I will change name of new functions to target_*. > > +int core_tpg_enable(struct se_portal_group *se_tpg, bool enable) > > +{ > > + int ret; > > + > > + if (enable) { > > + ret = core_tpg_register_rtpi(se_tpg); > > + if (ret) > > + return ret; > > + } else { > > + core_tpg_deregister_rtpi(se_tpg); > > + } > > + ret = se_tpg->se_tpg_tfo->fabric_enable_tpg(se_tpg, enable); > > + if (ret) { > > + core_tpg_deregister_rtpi(se_tpg); > > + return ret; > > + } > > + > > + se_tpg->enabled = enable; > > This bool enable logic is a bit weird and splitting the enable and > disable case would seem more sensible to me, but maybe there is > something later on that makes it more relevant. Ok, will split this function to enable/disable functions. BR, Dmitry