Re: [PATCH 11/17] target: add session sysfs class support

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Jun 08, 2020 at 10:35:56AM -0500, Mike Christie wrote:
> 
> 
> On 6/8/20 12:32 AM, Greg Kroah-Hartman wrote:
> > On Sun, Jun 07, 2020 at 03:35:58PM -0500, Mike Christie wrote:
> > > +int target_sysfs_add_session(struct se_portal_group *se_tpg,
> > > +			     struct se_session *se_sess)
> > > +{
> > > +	struct t10_transport_id *tpt_id = se_sess->tpt_id;
> > > +	int ret;
> > > +
> > > +	if (!try_module_get(se_sess->tfo->module))
> > > +		return -EINVAL;
> > > +
> > > +	ret = target_cp_endpoint_strs(se_tpg, se_sess);
> > > +	if (ret)
> > > +		goto put_mod;
> > > +
> > > +	se_sess->dev.groups = se_sess->tfo->session_attr_groups;
> > > +	ret = device_add(&se_sess->dev);
> > > +	if (ret) {
> > > +		pr_err("Could not add session%d to sysfs. Error %d.\n",
> > > +		       se_sess->sid, ret);
> > > +		goto free_ep_strs;
> > > +	}
> > > +
> > > +	pr_info("TCM added session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]",
> > > +		se_sess->sid, se_sess->fabric_name, se_sess->target_name,
> > > +		se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic",
> > > +		tpt_id->name, tpt_id->session_id ? "," : "",
> > > +		tpt_id->session_id ? tpt_id->session_id : "");
> > 
> > You have a 'struct device', so please use it, no need for pr_info(),
> > always use the dev_*() calls instead.
> > 
> > but, when drivers and kernel code is all working properly, no need to be
> > noisy at all, this should just be a dev_dbg() call, right?
> 
> I liked the info one, because the the code can work correctly, but the
> remote devices we are connecting to are normally going to hit issues.
> 
> For example every time the storage network goes down temporarily the driver
> code will call remove function, then call the add again when it comes back
> up. Having it always logged helps us figure out the root problem later when
> the customer only has logs available.

Then make this a tracepoint or something, again, do not be noisy for
normal system operations.  Do you want this to be the case for all of
your hardware devices all the time?

thanks,

greg k-h