lkml.org 
[lkml]   [2005]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: -mm -> 2.6.13 merge status
    Vladimir Saveliev wrote:

    >
    >
    >>>>+/*
    >>>>+ * Initialization stages for reiser4.
    >>>>+ *
    >>>>+ * These enumerate various things that have to be done during reiser4
    >>>>+ * startup. Initialization code (init_reiser4()) keeps track of what stage was
    >>>>+ * reached, so that proper undo can be done if error occurs during
    >>>>+ * initialization.
    >>>>+ */
    >>>>+typedef enum {
    >>>>+ INIT_NONE, /* nothing is initialized yet */
    >>>>+ INIT_INODECACHE, /* inode cache created */
    >>>>+ INIT_CONTEXT_MGR, /* list of active contexts created */
    >>>>+ INIT_ZNODES, /* znode slab created */
    >>>>+ INIT_PLUGINS, /* plugins initialized */
    >>>>+ INIT_PLUGIN_SET, /* psets initialized */
    >>>>+ INIT_TXN, /* transaction manager initialized */
    >>>>+ INIT_FAKES, /* fake inode initialized */
    >>>>+ INIT_JNODES, /* jnode slab initialized */
    >>>>+ INIT_EFLUSH, /* emergency flush initialized */
    >>>>+ INIT_FQS, /* flush queues initialized */
    >>>>+ INIT_DENTRY_FSDATA, /* dentry_fsdata slab initialized */
    >>>>+ INIT_FILE_FSDATA, /* file_fsdata slab initialized */
    >>>>+ INIT_D_CURSOR, /* d_cursor suport initialized */
    >>>>+ INIT_FS_REGISTERED, /* reiser4 file system type registered */
    >>>>+} reiser4_init_stage;
    >>>>
    >>>>
    >>>>
    >>>>
    >>>Please use regular gotos instead. This is a silly hack especially since you
    >>>don't have release function for all of the stages.
    >>>
    >>>
    >>>
    >>>
    >>I'll let vs comment.
    >>
    >>
    >>
    >
    >IMHO, it is convinient. But lets change it as requested.
    >
    >
    No, if you like it, say so and it stays.

    >>>>+ *
    >>>>+ * kmalloc/kfree leak detection: reiser4_kmalloc(), reiser4_kfree(),
    >>>>+ * reiser4_kfree_in_sb().
    >>>>
    >>>>
    >>>>
    >>>>
    >>>Please don't do this! We've had enough trouble trying to get the
    >>>current subsystem specific allocators out, so do not introduce a new one. If
    >>>you need memory leak detection, make it generic and submit that. Reiser4, like
    >>>all other subsystems, should use kmalloc() and friends directly.
    >>>
    >>>
    >>>
    >>>
    >>I will let vs comment.
    >>
    >>
    >>
    >I agree with Pekka.
    >
    >
    Ok, can you make it generic easily?

    >
    >
    >>>
    >>>
    >>>
    >>>
    >>>>--- /dev/null 2003-09-23 21:59:22.000000000 +0400
    >>>>+++ linux-2.6.11-vs/fs/reiser4/debug.h 2005-06-03 17:49:38.297184283 +0400
    >>>>+
    >>>>+/*
    >>>>+ * Error code tracing facility. (Idea is borrowed from XFS code.)
    >>>>
    >>>>
    >>>>
    >>>>
    >>>Could this be turned into a generic facility?
    >>>
    >>>
    >>>
    >
    >I do not think that it will ever become accepted.
    >To get use of that task_t would have to be extended with fields to store
    >error code, file name and line in it, and several return addresses.
    >Moreover lines like
    >return -ENOENT;
    >would have to be replaced with:
    >return RETERR(-ENOENT);
    >
    >This is debugging feature, we should probably move it to our internal
    >debugging patch.
    >
    >
    >
    >>>
    >>>
    >>>
    >>>
    >>>>+#define reiser4_internal
    >>>>
    >>>>
    >>>>
    >>>>
    >>>Please drop the above useless #define.
    >>>
    >>>
    >>>
    >
    >ok
    >
    >
    >
    >>>>--- /dev/null 2003-09-23 21:59:22.000000000 +0400
    >>>>+++ linux-2.6.11-vs/fs/reiser4/init_super.c 2005-06-03 17:51:27.959201950 +0400
    >>>>+
    >>>>+#define _INIT_PARAM_LIST (struct super_block * s, reiser4_context * ctx, void * data, int silent)
    >>>>+#define _DONE_PARAM_LIST (struct super_block * s)
    >>>>+
    >>>>+#define _INIT_(subsys) static int _init_##subsys _INIT_PARAM_LIST
    >>>>+#define _DONE_(subsys) static void _done_##subsys _DONE_PARAM_LIST
    >>>>
    >>>>
    >>>>
    >>>>
    >>>Please remove this macro obfuscation.
    >>>
    >>>
    >>>
    >>>
    >>vs, I think he is right, do you?
    >>
    >>
    >>
    >>>
    >>>
    >>>
    >>>
    >>>>+
    >>>>+_DONE_EMPTY(exit_context)
    >>>>+
    >>>>+struct reiser4_subsys {
    >>>>+ int (*init) _INIT_PARAM_LIST;
    >>>>+ void (*done) _DONE_PARAM_LIST;
    >>>>+};
    >>>>+
    >>>>+#define _SUBSYS(subsys) {.init = &_init_##subsys, .done = &_done_##subsys}
    >>>>+static struct reiser4_subsys subsys_array[] = {
    >>>>+ _SUBSYS(mount_flags_check),
    >>>>+ _SUBSYS(sinfo),
    >>>>+ _SUBSYS(context),
    >>>>+ _SUBSYS(parse_options),
    >>>>+ _SUBSYS(object_ops),
    >>>>+ _SUBSYS(read_super),
    >>>>+ _SUBSYS(tree0),
    >>>>+ _SUBSYS(txnmgr),
    >>>>+ _SUBSYS(ktxnmgrd_context),
    >>>>+ _SUBSYS(ktxnmgrd),
    >>>>+ _SUBSYS(entd),
    >>>>+ _SUBSYS(formatted_fake),
    >>>>+ _SUBSYS(disk_format),
    >>>>+ _SUBSYS(sb_counters),
    >>>>+ _SUBSYS(d_cursor),
    >>>>+ _SUBSYS(fs_root),
    >>>>+ _SUBSYS(safelink),
    >>>>+ _SUBSYS(exit_context)
    >>>>+};
    >>>>
    >>>>
    >>>>
    >>>>
    >>>The above is overkill and silly. parse_options and read_super, for
    >>>example, are _not_ a subsystem inits but regular fs ops. Please consider
    >>>dropping this altogether but at least trim it down to something sane.
    >>>
    >>>
    >>>
    >
    >Pekka, would you prefer something like:
    >
    >reiser4_fill_super()
    >{
    > if (init_a() == 0) {
    > if (init_b() == 0) {
    > if (init_c() == 0) {
    > if (init_last() == 0)
    > return 0;
    > else {
    > done_c();
    > done_b();
    > done_a();
    > }
    > } else {
    > done_b();
    > done_a();
    > }
    > } else {
    > done_a();
    > }
    > }
    >}
    >
    >
    I think the above is easier to read than the below. Macros can obscure
    sometimes, and one of our weaknesses is a tendency to use macros in such
    a way that it frustrates meta-. use in emacs. Nikita did however
    mention that there was something that could understand macros when
    constructing tags files, but I forgot what that was.

    >With these macros we have reiser4_fill_super to look like the below, and
    >it remains unchanged when something new is added for initilizing on
    >filesystem mounting.
    >
    >reiser4_fill_super()
    >{
    > for (i = 0; i < REISER4_NR_SUBSYS; i++) {
    > ret = subsys_array[i].init(s, &ctx, data, silent);
    > if (ret) {
    > done_super(s, i - 1);
    > return ret;
    > }
    > }
    >}
    >
    >
    >
    >
    >
    >

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-06-23 19:35    [W:0.072 / U:1.568 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site