Skip to contentSkip to conversation

ovl_dir_read_merged: Redundant call to list_add()

DISCUSSION Chandan Rajendra 2 participants 3 messages May 30, 2017

lore
Share & tools
lore
#0 lore raw

ovl_dir_read_merged() has the following loop,

        for (idx = 0; idx != -1; idx = next) {
                next = ovl_path_next(idx, dentry, &realpath);

                if (next != -1) {
                        err = ovl_dir_read(&realpath, &rdd);
                        if (err)
                                break;
                } else {
                        /*                                                                                                                                                                                                                     
                         * Insert lowest layer entries before upper ones, this                                                                                                                                                                 
                         * allows offsets to be reasonably constant                                                                                                                                                                            
                         */
                        list_add(&rdd.middle, rdd.list);
                        rdd.is_lowest = true;
                        err = ovl_dir_read(&realpath, &rdd);
                        list_del(&rdd.middle);
                }
        }

rdd.middle is populated inside ovl_fill_lowest() which is indirectly invoked by the call to ovl_dir_read() inside the "else" part of the above "if" condition. Before this invocation of ovl_dir_read(), rdd.middle would basically be an empty list. So this would mean that the call list_add(&rdd.middle, rdd.list) is unnecessary.

Please let me know if I am missing something here.

Signature · 1 line
chandan
#1 lore raw
Quoted 22 lines · Chandan Rajendra

ovl_dir_read_merged() has the following loop,

        for (idx = 0; idx != -1; idx = next) {
                next = ovl_path_next(idx, dentry, &realpath);

                if (next != -1) {
                        err = ovl_dir_read(&realpath, &rdd);
                        if (err)
                                break;
                } else {
                        /*
                         * Insert lowest layer entries before upper ones, this
                         * allows offsets to be reasonably constant
                         */
                        list_add(&rdd.middle, rdd.list);
                        rdd.is_lowest = true;
                        err = ovl_dir_read(&realpath, &rdd);
                        list_del(&rdd.middle);
                }
        }
Quoted 8 lines

rdd.middle is populated inside ovl_fill_lowest() which is indirectly invoked by the call to ovl_dir_read() inside the "else" part of the above "if" condition. Before this invocation of ovl_dir_read(), rdd.middle would basically be an empty list. So this would mean that the call list_add(&rdd.middle, rdd.list) is unnecessary.

Please let me know if I am missing something here.

You are missing the fact that rdd.middle is not a new list. its a list element inserted into the circular list rdd.list as a marker to the head of the non-lowest entries list.
When referencing &rdd.middle inside ovl_fill_lowest() code is referencing the same list that was populated in rdd.list, but with a different head/tail.

#2 lore raw
Quoted 33 lines · IST Amir Goldstein
Quoted 22 lines · Chandan Rajendra

ovl_dir_read_merged() has the following loop,

        for (idx = 0; idx != -1; idx = next) {
                next = ovl_path_next(idx, dentry, &realpath);

                if (next != -1) {
                        err = ovl_dir_read(&realpath, &rdd);
                        if (err)
                                break;
                } else {
                        /*
                         * Insert lowest layer entries before upper ones, this
                         * allows offsets to be reasonably constant
                         */
                        list_add(&rdd.middle, rdd.list);
                        rdd.is_lowest = true;
                        err = ovl_dir_read(&realpath, &rdd);
                        list_del(&rdd.middle);
                }
        }
Quoted 8 lines

rdd.middle is populated inside ovl_fill_lowest() which is indirectly invoked by the call to ovl_dir_read() inside the "else" part of the above "if" condition. Before this invocation of ovl_dir_read(), rdd.middle would basically be an empty list. So this would mean that the call list_add(&rdd.middle, rdd.list) is unnecessary.

Please let me know if I am missing something here.

Quoted 8 lines

You are missing the fact that rdd.middle is not a new list. its a list element inserted into the circular list rdd.list as a marker to the head of the non-lowest entries list.
When referencing &rdd.middle inside ovl_fill_lowest() code is referencing the same list that was populated in rdd.list, but with a different head/tail.

Amir, Thanks for your clarification.

Signature · 1 line
chandan