cds_list_for_each_entry_rcu should use RCU_READ_LOCK_this (that already uses 'this' instead of calling 'THIS')

[mykaul]

It seems like in most, if not all cases, the list used is from this->private->... (peers for example). This is always done via something like:

    this = THIS;
    GF_ASSERT(this);
    conf = this->private;
    GF_ASSERT(conf);
...
    RCU_READ_LOCK;
    cds_list_for_each_entry_rcu(peerinfo, &conf->peers, uuid_list)

So why call 'THIS' again in the RCU_READ_LOCK (twice actually)?
(same for the unlock of course)
A simple change to add something like:

#define RCU_READ_LOCK_this                                                          \
    pthread_mutex_lock(&(this->ctx)->cleanup_lock);                            \
    {                                                                          \
        rcu_read_lock();                                                       \
    }                                                                          \
    pthread_mutex_unlock(&(this->ctx)->cleanup_lock);

And the same to the RCU_READ_UNLOCK, should reduce some calls to 'THIS'.

Unsure if the compiler takes care of these redundant calls.

[mohit84]

I have checked compiler does not optimize it.

With current macro
/home/mohit84/upstream_gluster/shd_inode/glusterfs/xlators/mgmt/glusterd/src/glusterd-handler.c:798
1f938: 48 83 c4 20 add $0x20,%rsp
1f93c: e8 8f 5b ff ff callq 154d0 __glusterfs_this_location@plt
1f941: 48 8b 00 mov (%rax),%rax
1f944: 48 8b b8 80 0f 00 00 mov 0xf80(%rax),%rdi
1f94b: 48 81 c7 20 14 00 00 add $0x1420,%rdi
1f952: e8 29 57 ff ff callq 15080 pthread_mutex_lock@plt

After changed the macro
/home/mohit84/upstream_gluster/mux_patch/glusterfs/xlators/mgmt/glusterd/src/glusterd-handler.c:798
1f938: 48 8b 83 80 0f 00 00 mov 0xf80(%rbx),%rax
1f93f: 48 83 c4 20 add $0x20,%rsp
1f943: 48 8d b8 20 14 00 00 lea 0x1420(%rax),%rdi
1f94a: e8 31 57 ff ff callq 15080 pthread_mutex_lock@plt

I will send a patch to handle it.

[stale]

Thank you for your contributions.
Noticed that this issue is not having any activity in last ~6 months! We are marking this issue as stale because it has not had recent activity.
It will be closed in 2 weeks if no one responds with a comment here.

[mykaul]

unstale - looks like an easy fix to me.

[stale]

Thank you for your contributions.
Noticed that this issue is not having any activity in last ~6 months! We are marking this issue as stale because it has not had recent activity.
It will be closed in 2 weeks if no one responds with a comment here.

[stale]

Closing this issue as there was no update since my last update on issue. If this is an issue which is still valid, feel free to open it.