On Fri, 2005-04-01 at 07:27 -0500, Steven Rostedt wrote:
> On Fri, 2005-04-01 at 07:19 +0200, Ingo Molnar wrote:
> > * Steven Rostedt <[EMAIL PROTECTED]> wrote:
> >
> > > > could you send me your latest patch for the bit-spin issue? My main
> > > > issue was cleanliness, so that the patch doesnt get stuck in the -RT
> > > > tree forever.
> > >
> > > I think that's the main problem. Without changing the design of the
> > > ext3 system, I don't think there is a clean patch. The implementation
> > > that I finally settled down with was to make the j_state and
> > > j_journal_head locks two global locks. I had to make a few
> > > modifications to some spots to avoid deadlocks, but this worked out
> > > well. The problem I was worried about was this causing too much
> > > overhead. So the ext3 buffers would have to contend with each other. I
> > > don't have any tests to see if this had too much of an impact.
> >
> > yeah - i think Andrew said that a global lock at that particular place
> > might not be that much of an issue.
> >
>
> OK, I'll start stripping it out of my kernel today and make a clean
> patch for you.
>
Ingo, I haven't forgotten about this, I just been heavily bug wacking
lately and just haven't had the time to do this.
I've pulled out both the lock_bh_state and lock_bh_journal_head and made
them two global locks. I haven't noticed any slowing down here, but
then again I haven't ran any real benchmarks. There's a BH flag set to
know when the lock is pending on a specific buffer head.
I don't know how acceptable this patch is. Take a look and if you have
any better ideas then let me know. I prefer this patch over the
wait_on_bit patch I sent you earlier since this patch still accounts for
priority inheritance, as the wait_on_bits don't.
-- Steve
Index: include/linux/jbd.h
===================================================================
--- include/linux/jbd.h (revision 113)
+++ include/linux/jbd.h (working copy)
@@ -313,47 +313,100 @@
BUFFER_FNS(RevokeValid, revokevalid)
TAS_BUFFER_FNS(RevokeValid, revokevalid)
BUFFER_FNS(Freed, freed)
+#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK) ||
defined(CONFIG_PREEMPT)
+BUFFER_FNS(State,state)
-static inline struct buffer_head *jh2bh(struct journal_head *jh)
+extern spinlock_t journal_bh_state_lock;
+extern spinlock_t journal_bh_journal_lock;
+
+static inline void jbd_lock_bh_state(struct buffer_head *bh)
{
- return jh->b_bh;
+ spin_lock(&journal_bh_state_lock);
+ BUG_ON(buffer_state(bh));
+ set_buffer_state(bh);
+ __acquire(journal_bh_state_lock);
}
-static inline struct journal_head *bh2jh(struct buffer_head *bh)
+static inline int jbd_trylock_bh_state(struct buffer_head *bh)
{
- return bh->b_private;
+ if (!spin_trylock(&journal_bh_state_lock))
+ return 0;
+
+ BUG_ON(buffer_state(bh));
+ set_buffer_state(bh);
+ __acquire(journal_bh_state_lock);
+ return 1;
}
+static inline int jbd_is_locked_bh_state(struct buffer_head *bh)
+{
+ return buffer_state(bh);
+}
+
+static inline void jbd_unlock_bh_state(struct buffer_head *bh)
+{
+ BUG_ON(!buffer_state(bh));
+ clear_buffer_state(bh);
+ spin_unlock(&journal_bh_state_lock);
+ __release(journal_bh_state_lock);
+}
+
+static inline void jbd_lock_bh_journal_head(struct buffer_head *bh)
+{
+ spin_lock(&journal_bh_journal_lock);
+ __acquire(journal_bh_journal_lock);
+}
+
+static inline void jbd_unlock_bh_journal_head(struct buffer_head *bh)
+{
+ spin_unlock(&journal_bh_journal_lock);
+ __release(journal_bh_journal_lock);
+}
+
+#else /* ! (CONFIG_SMP || CONFIG_DEBUG_SPINLOCK || CONFIG_PREEMPT) */
+
static inline void jbd_lock_bh_state(struct buffer_head *bh)
{
- bit_spin_lock(BH_State, &bh->b_state);
+ __acquire(journal_bh_state_lock);
}
static inline int jbd_trylock_bh_state(struct buffer_head *bh)
{
- return bit_spin_trylock(BH_State, &bh->b_state);
+ __acquire(journal_bh_state_lock);
+ return 1;
}
static inline int jbd_is_locked_bh_state(struct buffer_head *bh)
{
- return bit_spin_is_locked(BH_State, &bh->b_state);
+ return 1;
}
static inline void jbd_unlock_bh_state(struct buffer_head *bh)
{
- bit_spin_unlock(BH_State, &bh->b_state);
+ __release(journal_bh_state_lock);
}
static inline void jbd_lock_bh_journal_head(struct buffer_head *bh)
{
- bit_spin_lock(BH_JournalHead, &bh->b_state);
+ __acquire(journal_bh_journal_lock);
}
static inline void jbd_unlock_bh_journal_head(struct buffer_head *bh)
{
- bit_spin_unlock(BH_JournalHead, &bh->b_state);
+ __release(journal_bh_journal_lock);
}
+#endif /* (CONFIG_SMP || CONFIG_DEBUG_SPINLOCK || CONFIG_PREEMPT) */
+static inline struct buffer_head *jh2bh(struct journal_head *jh)
+{
+ return jh->b_bh;
+}
+
+static inline struct journal_head *bh2jh(struct buffer_head *bh)
+{
+ return bh->b_private;
+}
+
struct jbd_revoke_table_s;
/**
Index: fs/jbd/journal.c
===================================================================
--- fs/jbd/journal.c (revision 113)
+++ fs/jbd/journal.c (working copy)
@@ -82,6 +82,14 @@
static int journal_convert_superblock_v1(journal_t *, journal_superblock_t *);
+#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK) ||
defined(CONFIG_PREEMPT)
+spinlock_t journal_bh_state_lock = SPIN_LOCK_UNLOCKED;
+spinlock_t journal_bh_journal_lock = SPIN_LOCK_UNLOCKED;
+
+EXPORT_SYMBOL(journal_bh_state_lock);
+EXPORT_SYMBOL(journal_bh_journal_lock);
+#endif
+
/*
* Helper function used to manage commit timeouts
*/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
