Cursor in readdir

Cursor in readdir

[hujianyang]

Hi Miklos,

Sorry for disturb you. I'm confused with *cursor* in struct
ovl_dir_file. I know this struct ovl_cache_entry *cursor* is
presented to indicate current pos when performing readdir at
a MERGE type directory.

Why this additional entry is needed? Can't we use a pointer
to record which entry is read or is ready for reading? After
finish reading lower and upper directories, entries in struct
ovl_dir_cache is stabilized. Changing of directories will
cause the later .readdir() to create a new ovl_dir_cache but
the old cache will not be released until all the holders of
it call .release().

So I think a pointer points to an entry in the entry list of
ovl_dir_cache is enough to each process sharing the access
of the same cache. We don't need to keep and move an special
entry, *cursor*. That's my thought.

I've considered a lot and googled some key words but found
nothing. Did I miss something important?

It will be fine if you'd like to explain the requirement of
*cursor* in readdir. Thanks a lot~!


Hu

Re: Cursor in readdir

[Miklos Szeredi]

On Sat, Nov 29, 2014 at 11:11 AM, hujianyang <hujianyang@huawei.com> wrote:
> Hi Miklos,
>
> Sorry for disturb you. I'm confused with *cursor* in struct
> ovl_dir_file. I know this struct ovl_cache_entry *cursor* is
> presented to indicate current pos when performing readdir at
> a MERGE type directory.
>
> Why this additional entry is needed? Can't we use a pointer
> to record which entry is read or is ready for reading? After
> finish reading lower and upper directories, entries in struct
> ovl_dir_cache is stabilized. Changing of directories will
> cause the later .readdir() to create a new ovl_dir_cache but
> the old cache will not be released until all the holders of
> it call .release().
>
> So I think a pointer points to an entry in the entry list of
> ovl_dir_cache is enough to each process sharing the access
> of the same cache. We don't need to keep and move an special
> entry, *cursor*. That's my thought.

Cursors are safe against removal of current entry.   However, we don't
do that here, AFAICS, so I think you are correct in that the cursor is
not actually needed here.

Thanks,
Miklos

Re: Cursor in readdir

[hujianyang]

On 2014/12/1 23:26, Miklos Szeredi wrote:
> On Sat, Nov 29, 2014 at 11:11 AM, hujianyang <hujianyang@huawei.com> wrote:
>> Hi Miklos,
>>
>> Sorry for disturb you. I'm confused with *cursor* in struct
>> ovl_dir_file. I know this struct ovl_cache_entry *cursor* is
>> presented to indicate current pos when performing readdir at
>> a MERGE type directory.
>>
>> Why this additional entry is needed? Can't we use a pointer
>> to record which entry is read or is ready for reading? After
>> finish reading lower and upper directories, entries in struct
>> ovl_dir_cache is stabilized. Changing of directories will
>> cause the later .readdir() to create a new ovl_dir_cache but
>> the old cache will not be released until all the holders of
>> it call .release().
>>
>> So I think a pointer points to an entry in the entry list of
>> ovl_dir_cache is enough to each process sharing the access
>> of the same cache. We don't need to keep and move an special
>> entry, *cursor*. That's my thought.
> 
> Cursors are safe against removal of current entry.   However, we don't
> do that here, AFAICS, so I think you are correct in that the cursor is
> not actually needed here.
> 

Thanks~!

Do you think it's valuable to discard the using of *cursor* and
use a pointer instead? Or maybe we will enable some remove
operations for ovl_dir_cache in the future so we should keep
this mechanism.

What's your opinion? I'd like to change this part and see if
the usage of a pointer could speed up .readdir() on a merged
directory.

Cheers,

Hu

Re: Cursor in readdir

[Miklos Szeredi]

On Fri, Dec 5, 2014 at 10:47 AM, hujianyang <hujianyang@huawei.com> wrote:
> On 2014/12/1 23:26, Miklos Szeredi wrote:
>
> Do you think it's valuable to discard the using of *cursor* and
> use a pointer instead? Or maybe we will enable some remove
> operations for ovl_dir_cache in the future so we should keep
> this mechanism.

Will probably need to do remove operations on merged layers as
whiteouts anyway, which does not involve actual removal of entry.  So
I guess the cursor won't be needed.

> What's your opinion? I'd like to change this part and see if
> the usage of a pointer could speed up .readdir() on a merged
> directory.

If it's a cleanup (removes more complexity than it adds) then a patch
is welcome.

Thanks,
Miklos