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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/8/20 1:14 AM, Hannes Reinecke wrote:
+    se_sess->target_name = kstrdup(se_tpg->se_tpg_wwn->wwn_group.cg_item.ci_name,
+                       GFP_KERNEL);
+    if (!se_sess->target_name)
+        goto free_acl;
+
+    if (se_sess->tfo->fabric_alias)
+        se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_alias,
+                           GFP_KERNEL);
+    else
+        se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_name,
+                           GFP_KERNEL);
+    if (!se_sess->fabric_name)
+        goto free_target;
+
+    se_sess->tpg_name = kstrdup(se_tpg->tpg_group.cg_item.ci_name,
+                    GFP_KERNEL);
+    if (!se_sess->tpg_name)
+        goto free_fabric;
+
+    return 0;

I wonder if we need to copy these variables.
Why can't we display them directly, returning an error if the respective
link is not available?
If one is worried about the sysfs/configfs reference counting we can get the reference in the _show() functions; wouldn't that be a better solution?

Do you mean in the sysfs show function do a configfs_depend_item() on the tpg, www, acl, etc? If so, I'm not sure that's safe, because for the tpg for example, we could do:

1. Userspace starts tpg removal with a rmdir.
2. Userspace also opens sysfs session file and has a ref to the session, but not the tpg yet. 3. kernel tears down tpg and sessions under it. The tpg is freed. The session is not because of the ref taken in #2. 4. sysfs session show function starts to reference se_sess->se_tpg so it can do a configfs_depend_item on the tpg_group.cg_item, but the se_tpg is freed in #3.

We either need to:
1. cp like above.
2. Handle both configfs and device struct refcounts for the same struct. So add a device struct to the www, tpg, acl and then coordinate the refcounting. 3. take a reference to the se_tpg when the session is created then drop it in the session release. 4. add some code and end up mix locking and state checks with refcounts. For example the tpg would have its configfs refcount like it does today (no new device struct in it), and when it deletes the sessions under it make sure the se_sess->se_tpg is NULL'd in a way that the session show function can do

lock()
if (!se_sess->se_tpg)
	return

se_sess->se_tpg access
unlock()

And then you have to do that up the stack for the other structs we want to ref.



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux