xfs
[Top] [All Lists]

Re: [PATCH] Kernel janitor cleanup

To: linux-xfs@xxxxxxxxxxx
Subject: Re: [PATCH] Kernel janitor cleanup
From: Brian Pontz <brian.pontz@xxxxxxxx>
Date: 03 Sep 2002 17:35:18 -0400
In-reply-to: <20020903095458.A551@infradead.org>
References: <3D74023E.1010208@osdn.com> <20020903095458.A551@infradead.org>
Sender: linux-xfs-bounce@xxxxxxxxxxx
http://www.axehind.com/xfs/xfs-cleanup-2-linux-2.4.patch

return() cleanup partial patch.

Brian


On Tue, 2002-09-03 at 04:54, Christoph Hellwig wrote:
> 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>