Hi Daniel,
Daniel Debonzi, on 05/16/2009 01:41 AM wrote:
This patch implements the creation of the subdirectories sessions, luns, ini_group
inside targets/<target_name>/target/.
Also it makes some changes on scst_unregister since the scst_tgt can't just be
kfreed as long as it has a kobject embedded on it. Resuming, the scst_unregister
call kobject_put and on the kobject release functions there is a call to scst_release
(which is the same as scst_unregister was).
Unfortunately, your patch still contain a whitespace in the beginning of
each line. Do you use mouse to paste the patch in the message? I
personally use "Copy" and "Paste" through menu. Copy from KScope's editor.
Signed-off-by: Daniel Debonzi <debonzi@xxxxxxxxxxxxxxxxxx>
You forgot diffstat here.
Looking at your patch I realized that, since we export our internal
objects, we need to decide their lifetime rules as well as decide some
unified agreement for the names of functions and error recover (your
patch has problems in this area). I suggest the following. Your thought
are welcome.
1. All kobjects without attributes will be dynamic kobjects. This is the
same as you did. Example is tgtt_kobj.
2. All kobjects with attributes will have dedicated kobj_type.
3. For ease of error recovery all sysfs content for each object will be
created in a single function with name scst_create_object_name_sysfs(),
like scst_create_tgt_sysfs()
4. All sysfs content of an object will also be cleaned by a single
function. For simple dynamic kobjects it will be called
scst_cleanup_object_name_sysfs(), like scst_cleanup_tgtt_sysfs(). For
other objects it will be called
scst_cleanup_object_name_sysfs_put_object_name(), like
scst_cleanup_tgt_sysfs_put_tgt(). It will also put reference to the
corresponding kobject and, hence, can destroy the whole object. This is
why I chose so big and inconvenient name: to emphasize that the object
can be destroyed after this function returned.
5. In the unregister functions the object will be fully cleaned as much
as possible for its sysfs attributes to work and not oops.
I committed your patch with the above changes. Hopefully, now there is a
simple and straightforward path to implement other SCST sysfs files and
directories.
See also below.
Index: scst/include/scst.h
===================================================================
--- scst/include/scst.h (revision 851)
+++ scst/include/scst.h (working copy)
@@ -961,7 +961,10 @@ struct scst_tgt {
/* Name on the default security group ("Default_target_name") */
char *default_group_name;
- struct kobject *tgt_kobj; /* kobject for this struct. */
+ struct kobject tgt_kobj; /* main kobject for this struct. */
+ struct kobject *tgt_sess_kobj;
+ struct kobject *tgt_luns_kobj;
+ struct kobject *tgt_ini_grp_kobj;
};
/* Hash size and hash fn for hash based lun translation */
Index: scst/src/scst_main.c
===================================================================
--- scst/src/scst_main.c (revision 851)
+++ scst/src/scst_main.c (working copy)
@@ -375,6 +375,10 @@ struct scst_tgt *scst_register(struct sc
mutex_unlock(&scst_mutex);
scst_resume_activity();
+ rc = scst_create_tgt_child_kobjs(tgt);
+ if (rc < 0)
+ goto out_child_kobjs_err;
+
PRINT_INFO("Target %s (%p) for template %s registered successfully",
target_name, tgt, vtt->name);
@@ -382,6 +386,11 @@ out:
TRACE_EXIT();
return tgt;
+out_child_kobjs_err:
+ scst_destroy_tgt_kobj(tgt);
+ TRACE_EXIT();
+ return NULL;
+
out_kobj_err:
scst_cleanup_proc_target_entries(tgt);
@@ -416,6 +425,13 @@ static inline int test_sess_list(struct
void scst_unregister(struct scst_tgt *tgt)
{
+ scst_destroy_tgt_child_kobjs(tgt);
+ scst_destroy_tgt_kobj(tgt);
+}
+EXPORT_SYMBOL(scst_unregister);
+
+void scst_release(struct scst_tgt *tgt)
+{
struct scst_session *sess;
struct scst_tgt_template *vtt = tgt->tgtt;
@@ -452,7 +468,6 @@ again:
list_del(&tgt->tgt_list_entry);
scst_cleanup_proc_target_entries(tgt);
- scst_destroy_tgt_kobj(tgt);
kfree(tgt->default_group_name);
@@ -468,8 +483,8 @@ again:
TRACE_EXIT();
return;
+
}
-EXPORT_SYMBOL(scst_unregister);
static int scst_susp_wait(bool interruptible)
{
Index: scst/src/scst_priv.h
===================================================================
--- scst/src/scst_priv.h (revision 851)
+++ scst/src/scst_priv.h (working copy)
@@ -390,6 +390,12 @@ int scst_create_tgtt_kobj(struct scst_tg
void scst_destroy_tgtt_kobj(struct scst_tgt_template *vtt);
int scst_create_tgt_kobj(struct scst_tgt *tgt);
void scst_destroy_tgt_kobj(struct scst_tgt *tgt);
+int scst_create_tgt_child_kobjs(struct scst_tgt *tgt);
+void scst_destroy_tgt_child_kobjs(struct scst_tgt *tgt);
+
+/* kobjects release functions */
+/*releases the scst_tgt sent to scst_unregister */
+void scst_release(struct scst_tgt *tgt);
int scst_get_cdb_len(const uint8_t *cdb);
Index: scst/src/scst_sysfs.c
===================================================================
--- scst/src/scst_sysfs.c (revision 851)
+++ scst/src/scst_sysfs.c (working copy)
@@ -43,16 +43,31 @@ void scst_destroy_tgtt_kobj(struct scst_
kobject_put(vtt->tgtt_kobj);
}
+void scst_tgt_release(struct kobject *kobj)
+{
+ struct scst_tgt *tgt;
+
+ TRACE_ENTRY();
+
+ tgt = container_of(kobj, struct scst_tgt, tgt_kobj);
+ scst_release(tgt);
+
+ TRACE_EXIT();
+}
+
+struct kobj_type tgt_ktype = {
+ .release = scst_tgt_release,
+};
+
int scst_create_tgt_kobj(struct scst_tgt *tgt)
{
int retval = 0;
TRACE_ENTRY();
- tgt->tgt_kobj = kobject_create_and_add(tgt->default_group_name,
- tgt->tgtt->tgtt_kobj);
- if (!tgt->tgt_kobj)
- retval = -EINVAL;
+ retval = kobject_init_and_add(&tgt->tgt_kobj, &tgt_ktype,
+ tgt->tgtt->tgtt_kobj,
+ tgt->default_group_name);
TRACE_EXIT_RES(retval);
return retval;
@@ -60,7 +75,50 @@ int scst_create_tgt_kobj(struct scst_tgt
void scst_destroy_tgt_kobj(struct scst_tgt *tgt)
{
- kobject_put(tgt->tgt_kobj);
+ kobject_put(&tgt->tgt_kobj);
+}
+
+int scst_create_tgt_child_kobjs(struct scst_tgt *tgt)
+{
+ int retval = 0;
+
+ TRACE_ENTRY();
+
+ tgt->tgt_sess_kobj = kobject_create_and_add("sessions", &tgt->tgt_kobj);
+ if (!tgt->tgt_sess_kobj) {
+ retval = -EINVAL;
+ goto sess_kobj_err;
+ }
+
+ tgt->tgt_luns_kobj = kobject_create_and_add("luns", &tgt->tgt_kobj);
+ if (!tgt->tgt_luns_kobj) {
+ retval = -EINVAL;
+ goto luns_kobj_err;
+ }
+
+ tgt->tgt_ini_grp_kobj = kobject_create_and_add("ini_group",
+ &tgt->tgt_kobj);
+ if (!tgt->tgt_ini_grp_kobj) {
+ retval = -EINVAL;
+ goto ini_grp_kobj_err;
+ }
+
+out:
+ TRACE_EXIT_RES(retval);
+ return retval;
+ini_grp_kobj_err:
+ kobject_put(tgt->tgt_luns_kobj);
+luns_kobj_err:
+ kobject_put(tgt->tgt_sess_kobj);
+sess_kobj_err:
+ goto out;
Better have empty lines between labels.
+}
+
+void scst_destroy_tgt_child_kobjs(struct scst_tgt *tgt)
+{
+ kobject_put(tgt->tgt_sess_kobj);
+ kobject_put(tgt->tgt_luns_kobj);
+ kobject_put(tgt->tgt_ini_grp_kobj);
}
static ssize_t scst_threads_show(struct kobject *kobj, struct kobj_attribute *attr,
Thanks,
Vlad
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
