[linux-cifs-client] [PATCH 1/5] cifs: protect GlobalOplock_Q with its own spinlock
Jeff Layton
jlayton at redhat.com
Fri Aug 21 04:31:10 MDT 2009
On Thu, 20 Aug 2009 19:32:17 -0500
Steve French <smfrench at gmail.com> wrote:
> Starting to look through this set
>
> ACK on this one. Doesn't look like this would affect behavior, but slight
> cosmetic improvement
>
Well, it could affect behavior. It changes the lock from the
GlobalMid_lock to a completely different one. It's always hard to tell
exactly what the effect will be of breaking up a coarse-grained lock
like that.
The locks are taken in the same place however. If there truly is no
relationship between the cifs_oplock_list and the other things
protected by the mid lock, then yes there should be no behavior change.
>
> On Tue, Aug 18, 2009 at 1:06 PM, Jeff Layton <jlayton at redhat.com> 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 <jlayton at redhat.com>
> > ---
> > 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
> >
> >
>
>
--
Jeff Layton <jlayton at redhat.com>
More information about the linux-cifs-client
mailing list