xfs
[Top] [All Lists]

Re: [PATCH] Remove l_flushsema

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH] Remove l_flushsema
From: Matthew Wilcox <matthew@xxxxxx>
Date: Wed, 30 Apr 2008 09:17:14 -0600
Cc: David Chinner <dgc@xxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20080430113753.GA17871@infradead.org>
References: <20080430090502.GH14976@parisc-linux.org> <20080430104125.GM108924158@sgi.com> <20080430105832.GA20442@infradead.org> <20080430111154.GO108924158@sgi.com> <20080430111521.GA16571@infradead.org> <20080430113418.GP108924158@sgi.com> <20080430113753.GA17871@infradead.org>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.13 (2006-08-11)
On Wed, Apr 30, 2008 at 07:37:53AM -0400, Christoph Hellwig wrote:
> On Wed, Apr 30, 2008 at 09:34:18PM +1000, David Chinner wrote:
> > > probably loose some arguments).
> > 
> > Yep, much cleaner. Who's signoff goes on this?
> 
> You can have mine:
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> 
> but I think it's till essentially willy's and he should be credited for
> it.

I'm fine with adding my S-o-B to this version:

Signed-off-by: Matthew Wilcox <willy@xxxxxxxxxxxxxxx>

Here's a little twist on the idea to avoid the thundering herd.
A vigorous review of this might not be a bad idea -- the idea is to only
wake up sleeping processes when there seems to be enough space in the
log to make it worthwhile.  So there's a few places where we unlock the
l_icloglock and jump back to restart; I didn't add an sv_signal there.
But there should be an sv_signal before each exit from the function,
and I think I've done that.

Signed-off-by: Matthew Wilcox <willy@xxxxxxxxxxxxxxx>

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 1bfe3f9..4533a10 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -2282,7 +2282,7 @@ xlog_state_do_callback(
        if (log->l_iclog->ic_state & (XLOG_STATE_ACTIVE|XLOG_STATE_IOERROR)) 
                wait = 1;
        spin_unlock(&log->l_icloglock);
-       sv_broadcast(&log->l_flush_wait);
+       sv_signal(&log->l_flush_wait);
 }      /* xlog_state_do_callback */
 
 
@@ -2377,6 +2377,7 @@ restart:
        spin_lock(&log->l_icloglock);
        if (XLOG_FORCED_SHUTDOWN(log)) {
                spin_unlock(&log->l_icloglock);
+               sv_signal(&log->l_flush_wait);
                return XFS_ERROR(EIO);
        }
 
@@ -2425,8 +2426,11 @@ restart:
                /* If I'm the only one writing to this iclog, sync it to disk */
                if (atomic_read(&iclog->ic_refcnt) == 1) {
                        spin_unlock(&log->l_icloglock);
-                       if ((error = xlog_state_release_iclog(log, iclog)))
+                       error = xlog_state_release_iclog(log, iclog);
+                       if (error) {
+                               sv_signal(&log->l_flush_wait);
                                return error;
+                       }
                } else {
                        atomic_dec(&iclog->ic_refcnt);
                        spin_unlock(&log->l_icloglock);
@@ -2452,6 +2456,7 @@ restart:
        ASSERT(iclog->ic_offset <= iclog->ic_size);
 
        spin_unlock(&log->l_icloglock);
+       sv_signal(&log->l_flush_wait);
 
        *logoffsetp = log_offset;
        return 0;

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."


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