ovl_dir_read_merged: Redundant call to list_add()

ovl_dir_read_merged: Redundant call to list_add()

[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);
                }
        }


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.

-- 
chandan

Re: ovl_dir_read_merged: Redundant call to list_add()

[Amir Goldstein]

On Tue, May 30, 2017 at 9:39 AM, Chandan Rajendra
<chandan@linux.vnet.ibm.com> wrote:
> 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.
>

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.

Re: ovl_dir_read_merged: Redundant call to list_add()

[Chandan Rajendra]

On Tuesday, May 30, 2017 1:35:55 PM IST Amir Goldstein wrote:
> On Tue, May 30, 2017 at 9:39 AM, Chandan Rajendra
> <chandan@linux.vnet.ibm.com> wrote:
> > 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.
> >
> 
> 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.

-- 
chandan