diff mbox

[linux-cifs-client,3/5] cifs: clean up cifs_oplock_thread loop

Message ID 1250618822-6131-4-git-send-email-jlayton@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jeff Layton Aug. 18, 2009, 6:07 p.m. UTC 
Minor cleanups and optimizations mostly. Also fix it so that if a
kthread_stop races in at the right time, it won't make the thread
sleep unnecessarily for 39s.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/cifsfs.c |   66 +++++++++++++++++++++++++++++------------------------
 1 files changed, 36 insertions(+), 30 deletions(-)

Comments

Shirish Pargaonkar Aug. 29, 2009, 1:04 p.m. UTC | #1 
On Tue, Aug 18, 2009 at 1:07 PM, Jeff Layton<jlayton@redhat.com> wrote:
> Minor cleanups and optimizations mostly. Also fix it so that if a
> kthread_stop races in at the right time, it won't make the thread
> sleep unnecessarily for 39s.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/cifsfs.c |   66 +++++++++++++++++++++++++++++------------------------
>  1 files changed, 36 insertions(+), 30 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 7ce8dcd..2fcc722 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -972,10 +972,10 @@ cifs_destroy_mids(void)
>
>  static int cifs_oplock_thread(void *dummyarg)
>  {
> -       struct oplock_q_entry *oplock_item;
> -       struct cifsTconInfo *pTcon;
> +       struct oplock_q_entry *oplock;
> +       struct cifsTconInfo *tcon;
>        struct inode *inode;
> -       __u16  netfid;
> +       struct cifsInodeInfo *cifsi;
>        int rc, waitrc = 0;
>
>        set_freezable();
> @@ -984,34 +984,30 @@ static int cifs_oplock_thread(void *dummyarg)
>                        continue;
>
>                spin_lock(&cifs_oplock_lock);
> -               if (list_empty(&cifs_oplock_list)) {
> +               while(!list_empty(&cifs_oplock_list)) {
> +                       oplock = list_entry(cifs_oplock_list.next,
> +                                           struct oplock_q_entry, qhead);
> +                       list_del(&oplock->qhead);
>                        spin_unlock(&cifs_oplock_lock);
> -                       set_current_state(TASK_INTERRUPTIBLE);
> -                       schedule_timeout(39*HZ);
> -               } else {
> -                       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;
> -                       list_del(&oplock_item->qhead);
> -                       spin_unlock(&cifs_oplock_lock);
> -                       kmem_cache_free(cifs_oplock_cachep, oplock_item);
> +                       tcon = oplock->tcon;
> +                       inode = oplock->pinode;
>                        /* can not grab inode sem here since it would
>                                deadlock when oplock received on delete
>                                since vfs_unlink holds the i_mutex across
>                                the call */
>                        /* mutex_lock(&inode->i_mutex);*/
> +                       cifsi = CIFS_I(inode);
>                        if (S_ISREG(inode->i_mode)) {
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
> -                               if (CIFS_I(inode)->clientCanCacheAll == 0)
> +                               if (cifsi->clientCanCacheAll == 0)
>                                        break_lease(inode, FMODE_READ);
> -                               else if (CIFS_I(inode)->clientCanCacheRead == 0)
> +                               else if (cifsi->clientCanCacheRead == 0)
>                                        break_lease(inode, FMODE_WRITE);
>  #endif
>                                rc = filemap_fdatawrite(inode->i_mapping);
> -                               if (CIFS_I(inode)->clientCanCacheRead == 0) {
> +                               if (cifsi->clientCanCacheRead == 0) {
>                                        waitrc = filemap_fdatawait(
>                                                              inode->i_mapping);
>                                        invalidate_remote_inode(inode);
> @@ -1022,27 +1018,37 @@ static int cifs_oplock_thread(void *dummyarg)
>                                rc = 0;
>                        /* mutex_unlock(&inode->i_mutex);*/
>                        if (rc)
> -                               CIFS_I(inode)->write_behind_rc = rc;
> +                               cifsi->write_behind_rc = rc;
>                        cFYI(1, ("Oplock flush inode %p rc %d",
>                                inode, rc));
>
> -                               /* releasing stale oplock after recent reconnect
> -                               of smb session using a now incorrect file
> -                               handle is not a data integrity issue but do
> -                               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,
> +                       /*
> +                        * releasing stale oplock after recent reconnect
> +                        * of smb session using a now incorrect file
> +                        * handle is not a data integrity issue but do
> +                        * not bother sending an oplock release if session
> +                        * to server still is disconnected since oplock
> +                        * already released by the server in that case
> +                        */
> +                       if (!tcon->need_reconnect) {
> +                               rc = CIFSSMBLock(0, tcon, oplock->netfid,
>                                                0 /* len */ , 0 /* offset */, 0,
>                                                0, LOCKING_ANDX_OPLOCK_RELEASE,
>                                                false /* wait flag */);
>                                cFYI(1, ("Oplock release rc = %d", rc));
>                        }
> -                       cifs_put_tcon(pTcon);
> -                       set_current_state(TASK_INTERRUPTIBLE);
> -                       schedule_timeout(1);  /* yield in case q were corrupt */
> +                       cifs_put_tcon(tcon);
> +                       kmem_cache_free(cifs_oplock_cachep, oplock);
> +                       spin_lock(&cifs_oplock_lock);
> +               }
> +               spin_unlock(&cifs_oplock_lock);
> +               set_current_state(TASK_INTERRUPTIBLE);
> +               if (kthread_should_stop()) {
> +                       set_current_state(TASK_RUNNING);
> +                       break;
>                }
> +               /* FIXME: Why 39s here? This should be a named constant. */
> +               schedule_timeout(39*HZ);
>        } while (!kthread_should_stop());
>
>        return 0;
> --
> 1.6.0.6
>
> _______________________________________________
> linux-cifs-client mailing list
> linux-cifs-client@lists.samba.org
> https://lists.samba.org/mailman/listinfo/linux-cifs-client
>

Acked-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
diff mbox

Patch

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 7ce8dcd..2fcc722 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -972,10 +972,10 @@  cifs_destroy_mids(void)
 
 static int cifs_oplock_thread(void *dummyarg)
 {
-	struct oplock_q_entry *oplock_item;
-	struct cifsTconInfo *pTcon;
+	struct oplock_q_entry *oplock;
+	struct cifsTconInfo *tcon;
 	struct inode *inode;
-	__u16  netfid;
+	struct cifsInodeInfo *cifsi;
 	int rc, waitrc = 0;
 
 	set_freezable();
@@ -984,34 +984,30 @@  static int cifs_oplock_thread(void *dummyarg)
 			continue;
 
 		spin_lock(&cifs_oplock_lock);
-		if (list_empty(&cifs_oplock_list)) {
+		while(!list_empty(&cifs_oplock_list)) {
+			oplock = list_entry(cifs_oplock_list.next,
+					    struct oplock_q_entry, qhead);
+			list_del(&oplock->qhead);
 			spin_unlock(&cifs_oplock_lock);
-			set_current_state(TASK_INTERRUPTIBLE);
-			schedule_timeout(39*HZ);
-		} else {
-			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;
-			list_del(&oplock_item->qhead);
-			spin_unlock(&cifs_oplock_lock);
-			kmem_cache_free(cifs_oplock_cachep, oplock_item);
+			tcon = oplock->tcon;
+			inode = oplock->pinode;
 			/* can not grab inode sem here since it would
 				deadlock when oplock received on delete
 				since vfs_unlink holds the i_mutex across
 				the call */
 			/* mutex_lock(&inode->i_mutex);*/
+			cifsi = CIFS_I(inode);
 			if (S_ISREG(inode->i_mode)) {
 #ifdef CONFIG_CIFS_EXPERIMENTAL
-				if (CIFS_I(inode)->clientCanCacheAll == 0)
+				if (cifsi->clientCanCacheAll == 0)
 					break_lease(inode, FMODE_READ);
-				else if (CIFS_I(inode)->clientCanCacheRead == 0)
+				else if (cifsi->clientCanCacheRead == 0)
 					break_lease(inode, FMODE_WRITE);
 #endif
 				rc = filemap_fdatawrite(inode->i_mapping);
-				if (CIFS_I(inode)->clientCanCacheRead == 0) {
+				if (cifsi->clientCanCacheRead == 0) {
 					waitrc = filemap_fdatawait(
 							      inode->i_mapping);
 					invalidate_remote_inode(inode);
@@ -1022,27 +1018,37 @@  static int cifs_oplock_thread(void *dummyarg)
 				rc = 0;
 			/* mutex_unlock(&inode->i_mutex);*/
 			if (rc)
-				CIFS_I(inode)->write_behind_rc = rc;
+				cifsi->write_behind_rc = rc;
 			cFYI(1, ("Oplock flush inode %p rc %d",
 				inode, rc));
 
-				/* releasing stale oplock after recent reconnect
-				of smb session using a now incorrect file
-				handle is not a data integrity issue but do
-				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,
+			/*
+			 * releasing stale oplock after recent reconnect
+			 * of smb session using a now incorrect file
+			 * handle is not a data integrity issue but do
+			 * not bother sending an oplock release if session
+			 * to server still is disconnected since oplock
+			 * already released by the server in that case
+			 */
+			if (!tcon->need_reconnect) {
+				rc = CIFSSMBLock(0, tcon, oplock->netfid,
 						0 /* len */ , 0 /* offset */, 0,
 						0, LOCKING_ANDX_OPLOCK_RELEASE,
 						false /* wait flag */);
 				cFYI(1, ("Oplock release rc = %d", rc));
 			}
-			cifs_put_tcon(pTcon);
-			set_current_state(TASK_INTERRUPTIBLE);
-			schedule_timeout(1);  /* yield in case q were corrupt */
+			cifs_put_tcon(tcon);
+			kmem_cache_free(cifs_oplock_cachep, oplock);
+			spin_lock(&cifs_oplock_lock);
+		}
+		spin_unlock(&cifs_oplock_lock);
+		set_current_state(TASK_INTERRUPTIBLE);
+		if (kthread_should_stop()) {
+			set_current_state(TASK_RUNNING);
+			break;
 		}
+		/* FIXME: Why 39s here? This should be a named constant. */
+		schedule_timeout(39*HZ);
 	} while (!kthread_should_stop());
 
 	return 0;