Re: [PATCH 1/5] cifs: protect GlobalOplock_Q with its own spinlock

Starting to look through this set

ACK on this one.   Doesn't look like this would affect behavior, but slight
cosmetic improvement


On Tue, Aug 18, 2009 at 1:06 PM, Jeff Layton <[email protected]> wrote:

> Right now, the GlobalOplock_Q is protected by the GlobalMid_Lock. That
> lock is also used for completely unrelated purposes (mostly for managing
> the global mid queue). Give the list its own dedicated spinlock
> (cifs_oplock_lock) and rename the list to cifs_oplock_list to
> eliminate the camel-case.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
>  fs/cifs/cifsfs.c    |   13 +++++++------
>  fs/cifs/cifsglob.h  |    6 +++++-
>  fs/cifs/transport.c |   17 ++++++++---------
>  3 files changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index b750aa5..3610e99 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -986,19 +986,19 @@ static int cifs_oplock_thread(void *dummyarg)
>                if (try_to_freeze())
>                        continue;
>
> -               spin_lock(&GlobalMid_Lock);
> -               if (list_empty(&GlobalOplock_Q)) {
> -                       spin_unlock(&GlobalMid_Lock);
> +               spin_lock(&cifs_oplock_lock);
> +               if (list_empty(&cifs_oplock_list)) {
> +                       spin_unlock(&cifs_oplock_lock);
>                        set_current_state(TASK_INTERRUPTIBLE);
>                        schedule_timeout(39*HZ);
>                } else {
> -                       oplock_item = list_entry(GlobalOplock_Q.next,
> +                       oplock_item = list_entry(cifs_oplock_list.next,
>                                                struct oplock_q_entry,
> qhead);
>                        cFYI(1, ("found oplock item to write out"));
>                        pTcon = oplock_item->tcon;
>                        inode = oplock_item->pinode;
>                        netfid = oplock_item->netfid;
> -                       spin_unlock(&GlobalMid_Lock);
> +                       spin_unlock(&cifs_oplock_lock);
>                        DeleteOplockQEntry(oplock_item);
>                        /* can not grab inode sem here since it would
>                                deadlock when oplock received on delete
> @@ -1055,7 +1055,7 @@ init_cifs(void)
>        int rc = 0;
>        cifs_proc_init();
>        INIT_LIST_HEAD(&cifs_tcp_ses_list);
> -       INIT_LIST_HEAD(&GlobalOplock_Q);
> +       INIT_LIST_HEAD(&cifs_oplock_list);
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
>        INIT_LIST_HEAD(&GlobalDnotifyReqList);
>        INIT_LIST_HEAD(&GlobalDnotifyRsp_Q);
> @@ -1084,6 +1084,7 @@ init_cifs(void)
>        rwlock_init(&GlobalSMBSeslock);
>        rwlock_init(&cifs_tcp_ses_lock);
>        spin_lock_init(&GlobalMid_Lock);
> +       spin_lock_init(&cifs_oplock_lock);
>
>        if (cifs_max_pending < 2) {
>                cifs_max_pending = 2;
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 6084d63..f100399 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -656,7 +656,11 @@ GLOBAL_EXTERN rwlock_t             cifs_tcp_ses_lock;
>  */
>  GLOBAL_EXTERN rwlock_t GlobalSMBSeslock;
>
> -GLOBAL_EXTERN struct list_head GlobalOplock_Q;
> +/* Global list of oplocks */
> +GLOBAL_EXTERN struct list_head cifs_oplock_list;
> +
> +/* Protects the cifs_oplock_list */
> +GLOBAL_EXTERN spinlock_t cifs_oplock_lock;
>
>  /* Outstanding dir notify requests */
>  GLOBAL_EXTERN struct list_head GlobalDnotifyReqList;
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 0ad3e2d..1da4ab2 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -119,20 +119,19 @@ AllocOplockQEntry(struct inode *pinode, __u16 fid,
> struct cifsTconInfo *tcon)
>                temp->pinode = pinode;
>                temp->tcon = tcon;
>                temp->netfid = fid;
> -               spin_lock(&GlobalMid_Lock);
> -               list_add_tail(&temp->qhead, &GlobalOplock_Q);
> -               spin_unlock(&GlobalMid_Lock);
> +               spin_lock(&cifs_oplock_lock);
> +               list_add_tail(&temp->qhead, &cifs_oplock_list);
> +               spin_unlock(&cifs_oplock_lock);
>        }
>        return temp;
> -
>  }
>
>  void DeleteOplockQEntry(struct oplock_q_entry *oplockEntry)
>  {
> -       spin_lock(&GlobalMid_Lock);
> +       spin_lock(&cifs_oplock_lock);
>     /* should we check if list empty first? */
>        list_del(&oplockEntry->qhead);
> -       spin_unlock(&GlobalMid_Lock);
> +       spin_unlock(&cifs_oplock_lock);
>        kmem_cache_free(cifs_oplock_cachep, oplockEntry);
>  }
>
> @@ -144,14 +143,14 @@ void DeleteTconOplockQEntries(struct cifsTconInfo
> *tcon)
>        if (tcon == NULL)
>                return;
>
> -       spin_lock(&GlobalMid_Lock);
> -       list_for_each_entry(temp, &GlobalOplock_Q, qhead) {
> +       spin_lock(&cifs_oplock_lock);
> +       list_for_each_entry(temp, &cifs_oplock_list, qhead) {
>                if ((temp->tcon) && (temp->tcon == tcon)) {
>                        list_del(&temp->qhead);
>                        kmem_cache_free(cifs_oplock_cachep, temp);
>                }
>        }
> -       spin_unlock(&GlobalMid_Lock);
> +       spin_unlock(&cifs_oplock_lock);
>  }
>
>  static int
> --
> 1.6.0.6
>
>


-- 
Thanks,

Steve

_______________________________________________
linux-cifs-client mailing list
[email protected]
https://lists.samba.org/mailman/listinfo/linux-cifs-client