xfs
[Top] [All Lists]

Re: [PATCH] Kernel janitor cleanup

To: Brian Pontz <brian.pontz@xxxxxxxx>
Subject: Re: [PATCH] Kernel janitor cleanup
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Tue, 3 Sep 2002 09:54:58 +0100
Cc: linux-xfs@xxxxxxxxxxx
In-reply-to: <3D74023E.1010208@osdn.com>; from brian.pontz@osdn.com on Mon, Sep 02, 2002 at 08:28:46PM -0400
References: <3D74023E.1010208@osdn.com>
Sender: linux-xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.2.5.1i
On Mon, Sep 02, 2002 at 08:28:46PM -0400, Brian Pontz wrote:
> http://www.axehind.com/xfs/xfs-cleanup-1-linux-2.4.patch
> 
> The patch is mainly a partial janitor patch.

The return cleanups make sense, that was inconsisten even inside XFS.
I'm not entirely sure wheter the int->uint stuff makes sense.  Have
you seen differences in the generated assembly?

The set_current_state patch is wrong, it adds another unnessecary test.
But that piece of code is rather wrong anyway and sets the states in
the wrong places.  Using wait_event() cleans up the code more and gets
it right:


Index: fs/xfs/xfs_inode.c
===================================================================
RCS file: /cvs/linux-2.4-xfs/linux/fs/xfs/xfs_inode.c,v
retrieving revision 1.348
diff -u -p -r1.348 xfs_inode.c
--- fs/xfs/xfs_inode.c  2002/09/01 12:43:34     1.348
+++ fs/xfs/xfs_inode.c  2002/09/03 08:49:50
@@ -2579,7 +2579,6 @@ xfs_iunpin_wait(
 {
        xfs_inode_log_item_t    *iip;
        xfs_lsn_t       lsn;
-       DECLARE_WAITQUEUE(wait, current);
 
        ASSERT(ismrlocked(&ip->i_lock, MR_UPDATE | MR_ACCESS));
 
@@ -2599,15 +2598,7 @@ xfs_iunpin_wait(
         */
        xfs_log_force(ip->i_mount, lsn, XFS_LOG_FORCE);
 
-       add_wait_queue(&ip->i_ipin_wait, &wait);
-repeat:
-       set_current_state(TASK_UNINTERRUPTIBLE);
-       if (atomic_read(&ip->i_pincount)) {
-               schedule();
-               goto repeat;
-       }
-       remove_wait_queue(&ip->i_ipin_wait, &wait);
-       current->state = TASK_RUNNING;
+       wait_event(ip->i_ipin_wait, (atomic_read(&ip->i_pincount) == 0));
 }
 
 


<Prev in Thread] Current Thread [Next in Thread>