From patchwork Mon Aug 17 12:16:06 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 42017 Received: from lists.samba.org (fn.samba.org [216.83.154.106]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n7HCGIvx011534 for ; Mon, 17 Aug 2009 12:16:18 GMT Received: from fn.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 1507DAD07A; Mon, 17 Aug 2009 06:09:59 -0600 (MDT) X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on fn.samba.org X-Spam-Level: X-Spam-Status: No, score=-5.9 required=3.8 tests=AWL,BAYES_00, RCVD_IN_DNSWL_MED, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.2.5 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from mx2.redhat.com (mx2.redhat.com [66.187.237.31]) by lists.samba.org (Postfix) with ESMTP id 9D6CFACF81 for ; Mon, 17 Aug 2009 06:09:52 -0600 (MDT) Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n7HCGAkj003949; Mon, 17 Aug 2009 08:16:10 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n7HCG9gi030129; Mon, 17 Aug 2009 08:16:09 -0400 Received: from localhost.localdomain (vpn-12-141.rdu.redhat.com [10.11.12.141]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n7HCG8pe015393; Mon, 17 Aug 2009 08:16:09 -0400 From: Jeff Layton To: smfrench@gmail.com Date: Mon, 17 Aug 2009 08:16:06 -0400 Message-Id: <1250511368-9812-3-git-send-email-jlayton@redhat.com> In-Reply-To: <1250511368-9812-1-git-send-email-jlayton@redhat.com> References: <1250511368-9812-1-git-send-email-jlayton@redhat.com> X-Scanned-By: MIMEDefang 2.58 on 172.16.27.26 Cc: linux-cifs-client@lists.samba.org Subject: [linux-cifs-client] [PATCH 2/4] cifs: iterate over cifs_oplock_list using list_for_each_entry_safe X-BeenThere: linux-cifs-client@lists.samba.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: The Linux CIFS VFS client List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: linux-cifs-client-bounces@lists.samba.org Errors-To: linux-cifs-client-bounces@lists.samba.org Removing an entry from a list while you're iterating over it can be problematic and racy, so use the _safe version of the list iteration routine in DeleteTconOplockQEntries. Also restructure the oplock_thread loop to make sure that if a kthread_stop races in just as the thread goes to sleep, then it won't just sit there for 39s. Finally, remove DeleteOplockQEntry(). It's only called from one place and we can avoid a function call this way. Signed-off-by: Jeff Layton --- fs/cifs/cifsfs.c | 37 +++++++++++++++++++++++-------------- fs/cifs/cifsproto.h | 1 - fs/cifs/transport.c | 23 +++++------------------ 3 files changed, 28 insertions(+), 33 deletions(-) diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 0d4e0da..ab4b373 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -976,7 +976,7 @@ cifs_destroy_mids(void) static int cifs_oplock_thread(void *dummyarg) { struct oplock_q_entry *oplock_item; - struct cifsTconInfo *pTcon; + struct cifsTconInfo *tcon; struct inode *inode; __u16 netfid; int rc, waitrc = 0; @@ -986,20 +986,24 @@ static int cifs_oplock_thread(void *dummyarg) if (try_to_freeze()) continue; + /* + * can't reasonably use list_for_each macros here. It's + * possible that another thread could come along and remove + * some of the entries while the lock is released. It's fine + * though since we're just popping one off the head on each + * iteration anyway. + */ mutex_lock(&cifs_oplock_mutex); - if (list_empty(&cifs_oplock_list)) { - mutex_unlock(&cifs_oplock_mutex); - set_current_state(TASK_INTERRUPTIBLE); - schedule_timeout(39*HZ); - } else { - oplock_item = list_entry(cifs_oplock_list.next, - struct oplock_q_entry, qhead); + while(!list_empty(&cifs_oplock_list)) { cFYI(1, ("found oplock item to write out")); - pTcon = oplock_item->tcon; + oplock_item = list_entry(cifs_oplock_list.next, + struct oplock_q_entry, qhead); + tcon = oplock_item->tcon; inode = oplock_item->pinode; netfid = oplock_item->netfid; + list_del(&oplock_item->qhead); + kmem_cache_free(cifs_oplock_cachep, oplock_item); mutex_unlock(&cifs_oplock_mutex); - DeleteOplockQEntry(oplock_item); /* can not grab inode sem here since it would deadlock when oplock received on delete since vfs_unlink holds the i_mutex across @@ -1034,16 +1038,21 @@ static int cifs_oplock_thread(void *dummyarg) not bother sending an oplock release if session to server still is disconnected since oplock already released by the server in that case */ - if (!pTcon->need_reconnect) { - rc = CIFSSMBLock(0, pTcon, netfid, + if (!tcon->need_reconnect) { + rc = CIFSSMBLock(0, tcon, netfid, 0 /* len */ , 0 /* offset */, 0, 0, LOCKING_ANDX_OPLOCK_RELEASE, false /* wait flag */); cFYI(1, ("Oplock release rc = %d", rc)); } - set_current_state(TASK_INTERRUPTIBLE); - schedule_timeout(1); /* yield in case q were corrupt */ + mutex_lock(&cifs_oplock_mutex); } + mutex_unlock(&cifs_oplock_mutex); + set_current_state(TASK_INTERRUPTIBLE); + if (kthread_should_stop()) + break; + /* FIXME: why 39s here? Turn this into a #define constant? */ + schedule_timeout(39*HZ); } while (!kthread_should_stop()); return 0; diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index da8fbf5..b7554a7 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -88,7 +88,6 @@ extern int CIFS_SessSetup(unsigned int xid, struct cifsSesInfo *ses, extern __u16 GetNextMid(struct TCP_Server_Info *server); extern struct oplock_q_entry *AllocOplockQEntry(struct inode *, u16, struct cifsTconInfo *); -extern void DeleteOplockQEntry(struct oplock_q_entry *); extern void DeleteTconOplockQEntries(struct cifsTconInfo *); extern struct timespec cifs_NTtimeToUnix(__le64 utc_nanoseconds_since_1601); extern u64 cifs_UnixTimeToNT(struct timespec); diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index 92e1538..59f0e95 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -126,28 +126,15 @@ AllocOplockQEntry(struct inode *pinode, __u16 fid, struct cifsTconInfo *tcon) return temp; } -void DeleteOplockQEntry(struct oplock_q_entry *oplockEntry) -{ - mutex_lock(&cifs_oplock_mutex); - /* should we check if list empty first? */ - list_del(&oplockEntry->qhead); - mutex_unlock(&cifs_oplock_mutex); - kmem_cache_free(cifs_oplock_cachep, oplockEntry); -} - - void DeleteTconOplockQEntries(struct cifsTconInfo *tcon) { - struct oplock_q_entry *temp; - - if (tcon == NULL) - return; + struct oplock_q_entry *entry, *next; mutex_lock(&cifs_oplock_mutex); - 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); + list_for_each_entry_safe(entry, next, &cifs_oplock_list, qhead) { + if (entry->tcon && entry->tcon == tcon) { + list_del(&entry->qhead); + kmem_cache_free(cifs_oplock_cachep, entry); } } mutex_unlock(&cifs_oplock_mutex);