[PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[Josef 'Jeff' Sipek]

Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
---

I tested it with xfsqa, and things work as well as they do without it.

---
 fs/xfs/xfs_mount.h     |    2 +-
 fs/xfs/xfs_trans.h     |    7 +---
 fs/xfs/xfs_trans_ail.c |   91 +++++++++++++++++++++---------------------------
 3 files changed, 42 insertions(+), 58 deletions(-)

diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index f7c620e..435d625 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -220,7 +220,7 @@ extern void	xfs_icsb_sync_counters_flags(struct xfs_mount *, int);
 #endif
 
 typedef struct xfs_ail {
-	xfs_ail_entry_t		xa_ail;
+	struct list_head	xa_ail;
 	uint			xa_gen;
 	struct task_struct	*xa_task;
 	xfs_lsn_t		xa_target;
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 7f40628..50ce02b 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -113,13 +113,8 @@ struct xfs_mount;
 struct xfs_trans;
 struct xfs_dquot_acct;
 
-typedef struct xfs_ail_entry {
-	struct xfs_log_item	*ail_forw;	/* AIL forw pointer */
-	struct xfs_log_item	*ail_back;	/* AIL back pointer */
-} xfs_ail_entry_t;
-
 typedef struct xfs_log_item {
-	xfs_ail_entry_t			li_ail;		/* AIL pointers */
+	struct list_head		li_ail;		/* AIL pointers */
 	xfs_lsn_t			li_lsn;		/* last on-disk lsn */
 	struct xfs_log_item_desc	*li_desc;	/* ptr to current desc*/
 	struct xfs_mount		*li_mountp;	/* ptr to fs mount */
diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 4d6330e..c3d402d 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -28,10 +28,10 @@
 #include "xfs_trans_priv.h"
 #include "xfs_error.h"
 
-STATIC void xfs_ail_insert(xfs_ail_entry_t *, xfs_log_item_t *);
-STATIC xfs_log_item_t * xfs_ail_delete(xfs_ail_entry_t *, xfs_log_item_t *);
-STATIC xfs_log_item_t * xfs_ail_min(xfs_ail_entry_t *);
-STATIC xfs_log_item_t * xfs_ail_next(xfs_ail_entry_t *, xfs_log_item_t *);
+STATIC void xfs_ail_insert(struct list_head *, xfs_log_item_t *);
+STATIC xfs_log_item_t * xfs_ail_delete(struct list_head *, xfs_log_item_t *);
+STATIC xfs_log_item_t * xfs_ail_min(struct list_head *);
+STATIC xfs_log_item_t * xfs_ail_next(struct list_head *, xfs_log_item_t *);
 
 #ifdef DEBUG
 STATIC void xfs_ail_check(xfs_ail_entry_t *, xfs_log_item_t *);
@@ -116,10 +116,12 @@ xfs_trans_first_push_ail(
 	if (lsn == 0)
 		return lip;
 
-	while (lip && (XFS_LSN_CMP(lip->li_lsn, lsn) < 0))
-		lip = lip->li_ail.ail_forw;
+	list_for_each_entry(lip, &mp->m_ail.xa_ail, li_ail) {
+		if (XFS_LSN_CMP(lip->li_lsn, lsn) >= 0)
+			return lip;
+	}
 
-	return lip;
+	return NULL;
 }
 
 /*
@@ -354,15 +356,13 @@ xfs_trans_update_ail(
 	xfs_log_item_t	*lip,
 	xfs_lsn_t	lsn) __releases(mp->m_ail_lock)
 {
-	xfs_ail_entry_t		*ailp;
 	xfs_log_item_t		*dlip=NULL;
 	xfs_log_item_t		*mlip;	/* ptr to minimum lip */
 
-	ailp = &(mp->m_ail.xa_ail);
-	mlip = xfs_ail_min(ailp);
+	mlip = xfs_ail_min(&mp->m_ail.xa_ail);
 
 	if (lip->li_flags & XFS_LI_IN_AIL) {
-		dlip = xfs_ail_delete(ailp, lip);
+		dlip = xfs_ail_delete(&mp->m_ail.xa_ail, lip);
 		ASSERT(dlip == lip);
 	} else {
 		lip->li_flags |= XFS_LI_IN_AIL;
@@ -370,11 +370,11 @@ xfs_trans_update_ail(
 
 	lip->li_lsn = lsn;
 
-	xfs_ail_insert(ailp, lip);
+	xfs_ail_insert(&mp->m_ail.xa_ail, lip);
 	mp->m_ail.xa_gen++;
 
 	if (mlip == dlip) {
-		mlip = xfs_ail_min(&(mp->m_ail.xa_ail));
+		mlip = xfs_ail_min(&mp->m_ail.xa_ail);
 		spin_unlock(&mp->m_ail_lock);
 		xfs_log_move_tail(mp, mlip->li_lsn);
 	} else {
@@ -404,14 +404,12 @@ xfs_trans_delete_ail(
 	xfs_mount_t	*mp,
 	xfs_log_item_t	*lip) __releases(mp->m_ail_lock)
 {
-	xfs_ail_entry_t		*ailp;
 	xfs_log_item_t		*dlip;
 	xfs_log_item_t		*mlip;
 
 	if (lip->li_flags & XFS_LI_IN_AIL) {
-		ailp = &(mp->m_ail.xa_ail);
-		mlip = xfs_ail_min(ailp);
-		dlip = xfs_ail_delete(ailp, lip);
+		mlip = xfs_ail_min(&mp->m_ail.xa_ail);
+		dlip = xfs_ail_delete(&mp->m_ail.xa_ail, lip);
 		ASSERT(dlip == lip);
 
 
@@ -514,8 +512,7 @@ int
 xfs_trans_ail_init(
 	xfs_mount_t	*mp)
 {
-	mp->m_ail.xa_ail.ail_forw = (xfs_log_item_t*)&mp->m_ail.xa_ail;
-	mp->m_ail.xa_ail.ail_back = (xfs_log_item_t*)&mp->m_ail.xa_ail;
+	INIT_LIST_HEAD(&mp->m_ail.xa_ail);
 	return xfsaild_start(mp);
 }
 
@@ -534,8 +531,8 @@ xfs_trans_ail_destroy(
  */
 STATIC void
 xfs_ail_insert(
-	xfs_ail_entry_t	*base,
-	xfs_log_item_t	*lip)
+	struct list_head	*base,
+	xfs_log_item_t		*lip)
 /* ARGSUSED */
 {
 	xfs_log_item_t	*next_lip;
@@ -543,25 +540,20 @@ xfs_ail_insert(
 	/*
 	 * If the list is empty, just insert the item.
 	 */
-	if (base->ail_back == (xfs_log_item_t*)base) {
-		base->ail_forw = lip;
-		base->ail_back = lip;
-		lip->li_ail.ail_forw = (xfs_log_item_t*)base;
-		lip->li_ail.ail_back = (xfs_log_item_t*)base;
+	if (list_empty(base)) {
+		list_add(&lip->li_ail, base);
 		return;
 	}
 
-	next_lip = base->ail_back;
-	while ((next_lip != (xfs_log_item_t*)base) &&
-	       (XFS_LSN_CMP(next_lip->li_lsn, lip->li_lsn) > 0)) {
-		next_lip = next_lip->li_ail.ail_back;
+	list_for_each_entry_reverse(next_lip, base, li_ail) {
+		if (XFS_LSN_CMP(next_lip->li_lsn, lip->li_lsn) <= 0)
+			break;
 	}
-	ASSERT((next_lip == (xfs_log_item_t*)base) ||
+
+	ASSERT((&next_lip->li_ail == base) ||
 	       (XFS_LSN_CMP(next_lip->li_lsn, lip->li_lsn) <= 0));
-	lip->li_ail.ail_forw = next_lip->li_ail.ail_forw;
-	lip->li_ail.ail_back = next_lip;
-	next_lip->li_ail.ail_forw = lip;
-	lip->li_ail.ail_forw->li_ail.ail_back = lip;
+
+	list_add(&lip->li_ail, &next_lip->li_ail);
 
 	xfs_ail_check(base, lip);
 	return;
@@ -573,15 +565,13 @@ xfs_ail_insert(
 /*ARGSUSED*/
 STATIC xfs_log_item_t *
 xfs_ail_delete(
-	xfs_ail_entry_t	*base,
-	xfs_log_item_t	*lip)
+	struct list_head	*base,
+	xfs_log_item_t		*lip)
 /* ARGSUSED */
 {
 	xfs_ail_check(base, lip);
-	lip->li_ail.ail_forw->li_ail.ail_back = lip->li_ail.ail_back;
-	lip->li_ail.ail_back->li_ail.ail_forw = lip->li_ail.ail_forw;
-	lip->li_ail.ail_forw = NULL;
-	lip->li_ail.ail_back = NULL;
+
+	list_del(&lip->li_ail);
 
 	return lip;
 }
@@ -592,14 +582,13 @@ xfs_ail_delete(
  */
 STATIC xfs_log_item_t *
 xfs_ail_min(
-	xfs_ail_entry_t	*base)
+	struct list_head	*base)
 /* ARGSUSED */
 {
-	register xfs_log_item_t *forw = base->ail_forw;
-	if (forw == (xfs_log_item_t*)base) {
+	if (list_empty(base))
 		return NULL;
-	}
-	return forw;
+
+	return list_first_entry(base, xfs_log_item_t, li_ail);
 }
 
 /*
@@ -609,14 +598,14 @@ xfs_ail_min(
  */
 STATIC xfs_log_item_t *
 xfs_ail_next(
-	xfs_ail_entry_t	*base,
-	xfs_log_item_t	*lip)
+	struct list_head	*base,
+	xfs_log_item_t		*lip)
 /* ARGSUSED */
 {
-	if (lip->li_ail.ail_forw == (xfs_log_item_t*)base) {
+	if (lip->li_ail.next == base)
 		return NULL;
-	}
-	return lip->li_ail.ail_forw;
+
+	return list_first_entry(&lip->li_ail, xfs_log_item_t, li_ail);
 
 }
 
-- 
1.5.4.rc2.85.g9de45-dirty

Re: [PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[Christoph Hellwig]

On Sun, Jan 20, 2008 at 07:35:57PM -0500, Josef 'Jeff' Sipek wrote:
> Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
> ---
> 
> I tested it with xfsqa, and things work as well as they do without it.

I like this a lot, but I think Dave has plans to replace the linked list
with a more efficient data structure soon, so it might not actually be
worth applying.

Re: [PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[Josef 'Jeff' Sipek]

On Mon, Jan 21, 2008 at 04:04:23AM +0000, Christoph Hellwig wrote:
> On Sun, Jan 20, 2008 at 07:35:57PM -0500, Josef 'Jeff' Sipek wrote:
> > Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
> > ---
> > 
> > I tested it with xfsqa, and things work as well as they do without it.
> 
> I like this a lot, but I think Dave has plans to replace the linked list
> with a more efficient data structure soon, so it might not actually be
> worth applying.

I've spoken with him about this, and he wants to have a tree where the
leaves have linked lists (if I understood correctly). So this just makes it
easier/cleaner for him.

Josef 'Jeff' Sipek.

-- 
Hegh QaQ law'
quvHa'ghach QaQ puS

Re: [PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[David Chinner]

On Sun, Jan 20, 2008 at 11:07:40PM -0500, Josef 'Jeff' Sipek wrote:
> On Mon, Jan 21, 2008 at 04:04:23AM +0000, Christoph Hellwig wrote:
> > On Sun, Jan 20, 2008 at 07:35:57PM -0500, Josef 'Jeff' Sipek wrote:
> > > Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
> > > ---
> > > 
> > > I tested it with xfsqa, and things work as well as they do without it.
> > 
> > I like this a lot, but I think Dave has plans to replace the linked list
> > with a more efficient data structure soon, so it might not actually be
> > worth applying.
> 
> I've spoken with him about this, and he wants to have a tree where the
> leaves have linked lists (if I understood correctly). So this just makes it
> easier/cleaner for him.

Sort of.

Few things that really should be done in this first patch. Rather
than passing listheads to the xfs_ail_*() functions, it should
really be changed to pass the xfs_ail_t to those functions. The
structure of the list should be opaque to everything outside these
functions.

It also needs to build with XFS_DEBUG enabled - that means
xfs_ail_check needs updating, but I've already got a patch
for the other bit (xfsidbg.c) that works which is attached below.

Cheers,

Dave.

---
make xfsidbg.c compile and work with listhead based AIL.

Signed-off-by: Dave Chinner <dgc@sgi.com>
---
 fs/xfs/xfsidbg.c |   66 +++++++++++++++++++++++++------------------------------
 1 file changed, 30 insertions(+), 36 deletions(-)

Index: 2.6.x-xfs-new/fs/xfs/xfsidbg.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfsidbg.c	2008-01-21 16:23:35.000000000 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfsidbg.c	2008-01-21 18:16:11.450796254 +1100
@@ -6167,7 +6167,7 @@ xfsidbg_xlogitem(xfs_log_item_t *lip)
 	printflags((uint)(lip->li_flags), li_flags,"log");
 	kdb_printf("\n");
 	kdb_printf("ail forw 0x%p ail back 0x%p lsn %s\ndesc %p ops 0x%p",
-		lip->li_ail.ail_forw, lip->li_ail.ail_back,
+		lip->li_ail.next, lip->li_ail.next,
 		xfs_fmtlsn(&(lip->li_lsn)), lip->li_desc, lip->li_ops);
 	kdb_printf(" iodonefunc &0x%p\n", lip->li_cb);
 	if (lip->li_type == XFS_LI_BUF) {
@@ -6220,45 +6220,39 @@ xfsidbg_xaildump(xfs_mount_t *mp)
 		};
 	int count;
 
-	if ((mp->m_ail.xa_ail.ail_forw == NULL) ||
-	    (mp->m_ail.xa_ail.ail_forw == (xfs_log_item_t *)&mp->m_ail.xa_ail)) {
+	if (list_empty(&mp->m_ail.xa_ail)) {
 		kdb_printf("AIL is empty\n");
 		return;
 	}
 	kdb_printf("AIL for mp 0x%p, oldest first\n", mp);
-	lip = (xfs_log_item_t*)mp->m_ail.xa_ail.ail_forw;
-	for (count = 0; lip; count++) {
-		kdb_printf("[%d] type %s ", count, xfsidbg_item_type_str(lip));
-		printflags((uint)(lip->li_flags), li_flags, "flags:");
-		kdb_printf("  lsn %s\n   ", xfs_fmtlsn(&(lip->li_lsn)));
-		switch (lip->li_type) {
-		case XFS_LI_BUF:
-			xfs_buf_item_print((xfs_buf_log_item_t *)lip, 1);
-			break;
-		case XFS_LI_INODE:
-			xfs_inode_item_print((xfs_inode_log_item_t *)lip, 1);
-			break;
-		case XFS_LI_EFI:
-			xfs_efi_item_print((xfs_efi_log_item_t *)lip, 1);
-			break;
-		case XFS_LI_EFD:
-			xfs_efd_item_print((xfs_efd_log_item_t *)lip, 1);
-			break;
-		case XFS_LI_DQUOT:
-			xfs_dquot_item_print((xfs_dq_logitem_t *)lip, 1);
-			break;
-		case XFS_LI_QUOTAOFF:
-			xfs_qoff_item_print((xfs_qoff_logitem_t *)lip, 1);
-			break;
-		default:
-			kdb_printf("Unknown item type %d\n", lip->li_type);
-			break;
-		}
-
-		if (lip->li_ail.ail_forw == (xfs_log_item_t*)&mp->m_ail.xa_ail) {
-			lip = NULL;
-		} else {
-			lip = lip->li_ail.ail_forw;
+	list_for_each_entry(lip, &mp->m_ail.xa_ail, li_ail) {
+		for (count = 0; lip; count++) {
+			kdb_printf("[%d] type %s ", count, xfsidbg_item_type_str(lip));
+			printflags((uint)(lip->li_flags), li_flags, "flags:");
+			kdb_printf("  lsn %s\n   ", xfs_fmtlsn(&(lip->li_lsn)));
+			switch (lip->li_type) {
+			case XFS_LI_BUF:
+				xfs_buf_item_print((xfs_buf_log_item_t *)lip, 1);
+				break;
+			case XFS_LI_INODE:
+				xfs_inode_item_print((xfs_inode_log_item_t *)lip, 1);
+				break;
+			case XFS_LI_EFI:
+				xfs_efi_item_print((xfs_efi_log_item_t *)lip, 1);
+				break;
+			case XFS_LI_EFD:
+				xfs_efd_item_print((xfs_efd_log_item_t *)lip, 1);
+				break;
+			case XFS_LI_DQUOT:
+				xfs_dquot_item_print((xfs_dq_logitem_t *)lip, 1);
+				break;
+			case XFS_LI_QUOTAOFF:
+				xfs_qoff_item_print((xfs_qoff_logitem_t *)lip, 1);
+				break;
+			default:
+				kdb_printf("Unknown item type %d\n", lip->li_type);
+				break;
+			}
 		}
 	}
 }

Re: [PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[Josef 'Jeff' Sipek]

On Fri, Jan 25, 2008 at 06:08:00PM +1100, David Chinner wrote:
...
> Few things that really should be done in this first patch. Rather
> than passing listheads to the xfs_ail_*() functions, it should
> really be changed to pass the xfs_ail_t to those functions. The
> structure of the list should be opaque to everything outside these
> functions.
 
Agreed. I'm going to send a fixed up version of the patch sometime tomorrow.

> It also needs to build with XFS_DEBUG enabled - that means
> xfs_ail_check needs updating, but I've already got a patch
> for the other bit (xfsidbg.c) that works which is attached below.

Lesson learned...compile with XFS_DEBUG on.

Josef 'Jeff' Sipek.

-- 
Real Programmers consider "what you see is what you get" to be just as bad a
concept in Text Editors as it is in women. No, the Real Programmer wants a
"you asked for it, you got it" text editor -- complicated, cryptic,
powerful, unforgiving, dangerous.

[PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[Josef 'Jeff' Sipek]

Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
---
This patch assumes you already have Dave Chinner's patch for
xfsidbg_xlogitem and xfsidbg_xaildump is needed.

Changes since V1:

- Pass around a pointer to the AIL, not the struct list_head
- Make sure things compile & run with CONFIG_XFS_DEBUG
---
 fs/xfs/xfs_mount.h     |    2 +-
 fs/xfs/xfs_trans.h     |    7 +--
 fs/xfs/xfs_trans_ail.c |  149 +++++++++++++++++++----------------------------
 3 files changed, 62 insertions(+), 96 deletions(-)

diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index f7c620e..435d625 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -220,7 +220,7 @@ extern void	xfs_icsb_sync_counters_flags(struct xfs_mount *, int);
 #endif
 
 typedef struct xfs_ail {
-	xfs_ail_entry_t		xa_ail;
+	struct list_head	xa_ail;
 	uint			xa_gen;
 	struct task_struct	*xa_task;
 	xfs_lsn_t		xa_target;
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 7f40628..50ce02b 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -113,13 +113,8 @@ struct xfs_mount;
 struct xfs_trans;
 struct xfs_dquot_acct;
 
-typedef struct xfs_ail_entry {
-	struct xfs_log_item	*ail_forw;	/* AIL forw pointer */
-	struct xfs_log_item	*ail_back;	/* AIL back pointer */
-} xfs_ail_entry_t;
-
 typedef struct xfs_log_item {
-	xfs_ail_entry_t			li_ail;		/* AIL pointers */
+	struct list_head		li_ail;		/* AIL pointers */
 	xfs_lsn_t			li_lsn;		/* last on-disk lsn */
 	struct xfs_log_item_desc	*li_desc;	/* ptr to current desc*/
 	struct xfs_mount		*li_mountp;	/* ptr to fs mount */
diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 4d6330e..8b3fd60 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -28,13 +28,13 @@
 #include "xfs_trans_priv.h"
 #include "xfs_error.h"
 
-STATIC void xfs_ail_insert(xfs_ail_entry_t *, xfs_log_item_t *);
-STATIC xfs_log_item_t * xfs_ail_delete(xfs_ail_entry_t *, xfs_log_item_t *);
-STATIC xfs_log_item_t * xfs_ail_min(xfs_ail_entry_t *);
-STATIC xfs_log_item_t * xfs_ail_next(xfs_ail_entry_t *, xfs_log_item_t *);
+STATIC void xfs_ail_insert(xfs_ail_t *, xfs_log_item_t *);
+STATIC xfs_log_item_t * xfs_ail_delete(xfs_ail_t *, xfs_log_item_t *);
+STATIC xfs_log_item_t * xfs_ail_min(xfs_ail_t *);
+STATIC xfs_log_item_t * xfs_ail_next(xfs_ail_t *, xfs_log_item_t *);
 
 #ifdef DEBUG
-STATIC void xfs_ail_check(xfs_ail_entry_t *, xfs_log_item_t *);
+STATIC void xfs_ail_check(xfs_ail_t *, xfs_log_item_t *);
 #else
 #define	xfs_ail_check(a,l)
 #endif /* DEBUG */
@@ -57,7 +57,7 @@ xfs_trans_tail_ail(
 	xfs_log_item_t	*lip;
 
 	spin_lock(&mp->m_ail_lock);
-	lip = xfs_ail_min(&(mp->m_ail.xa_ail));
+	lip = xfs_ail_min(&(mp->m_ail));
 	if (lip == NULL) {
 		lsn = (xfs_lsn_t)0;
 	} else {
@@ -91,7 +91,7 @@ xfs_trans_push_ail(
 {
 	xfs_log_item_t		*lip;
 
-	lip = xfs_ail_min(&mp->m_ail.xa_ail);
+	lip = xfs_ail_min(&mp->m_ail);
 	if (lip && !XFS_FORCED_SHUTDOWN(mp)) {
 		if (XFS_LSN_CMP(threshold_lsn, mp->m_ail.xa_target) > 0)
 			xfsaild_wakeup(mp, threshold_lsn);
@@ -111,15 +111,17 @@ xfs_trans_first_push_ail(
 {
 	xfs_log_item_t	*lip;
 
-	lip = xfs_ail_min(&(mp->m_ail.xa_ail));
+	lip = xfs_ail_min(&(mp->m_ail));
 	*gen = (int)mp->m_ail.xa_gen;
 	if (lsn == 0)
 		return lip;
 
-	while (lip && (XFS_LSN_CMP(lip->li_lsn, lsn) < 0))
-		lip = lip->li_ail.ail_forw;
+	list_for_each_entry(lip, &mp->m_ail.xa_ail, li_ail) {
+		if (XFS_LSN_CMP(lip->li_lsn, lsn) >= 0)
+			return lip;
+	}
 
-	return lip;
+	return NULL;
 }
 
 /*
@@ -326,7 +328,7 @@ xfs_trans_unlocked_item(
 	 * the call to xfs_log_move_tail() doesn't do anything if there's
 	 * not enough free space to wake people up so we're safe calling it.
 	 */
-	min_lip = xfs_ail_min(&mp->m_ail.xa_ail);
+	min_lip = xfs_ail_min(&mp->m_ail);
 
 	if (min_lip == lip)
 		xfs_log_move_tail(mp, 1);
@@ -354,15 +356,13 @@ xfs_trans_update_ail(
 	xfs_log_item_t	*lip,
 	xfs_lsn_t	lsn) __releases(mp->m_ail_lock)
 {
-	xfs_ail_entry_t		*ailp;
 	xfs_log_item_t		*dlip=NULL;
 	xfs_log_item_t		*mlip;	/* ptr to minimum lip */
 
-	ailp = &(mp->m_ail.xa_ail);
-	mlip = xfs_ail_min(ailp);
+	mlip = xfs_ail_min(&mp->m_ail);
 
 	if (lip->li_flags & XFS_LI_IN_AIL) {
-		dlip = xfs_ail_delete(ailp, lip);
+		dlip = xfs_ail_delete(&mp->m_ail, lip);
 		ASSERT(dlip == lip);
 	} else {
 		lip->li_flags |= XFS_LI_IN_AIL;
@@ -370,11 +370,11 @@ xfs_trans_update_ail(
 
 	lip->li_lsn = lsn;
 
-	xfs_ail_insert(ailp, lip);
+	xfs_ail_insert(&mp->m_ail, lip);
 	mp->m_ail.xa_gen++;
 
 	if (mlip == dlip) {
-		mlip = xfs_ail_min(&(mp->m_ail.xa_ail));
+		mlip = xfs_ail_min(&mp->m_ail);
 		spin_unlock(&mp->m_ail_lock);
 		xfs_log_move_tail(mp, mlip->li_lsn);
 	} else {
@@ -404,14 +404,12 @@ xfs_trans_delete_ail(
 	xfs_mount_t	*mp,
 	xfs_log_item_t	*lip) __releases(mp->m_ail_lock)
 {
-	xfs_ail_entry_t		*ailp;
 	xfs_log_item_t		*dlip;
 	xfs_log_item_t		*mlip;
 
 	if (lip->li_flags & XFS_LI_IN_AIL) {
-		ailp = &(mp->m_ail.xa_ail);
-		mlip = xfs_ail_min(ailp);
-		dlip = xfs_ail_delete(ailp, lip);
+		mlip = xfs_ail_min(&mp->m_ail);
+		dlip = xfs_ail_delete(&mp->m_ail, lip);
 		ASSERT(dlip == lip);
 
 
@@ -420,7 +418,7 @@ xfs_trans_delete_ail(
 		mp->m_ail.xa_gen++;
 
 		if (mlip == dlip) {
-			mlip = xfs_ail_min(&(mp->m_ail.xa_ail));
+			mlip = xfs_ail_min(&(mp->m_ail));
 			spin_unlock(&mp->m_ail_lock);
 			xfs_log_move_tail(mp, (mlip ? mlip->li_lsn : 0));
 		} else {
@@ -458,7 +456,7 @@ xfs_trans_first_ail(
 {
 	xfs_log_item_t	*lip;
 
-	lip = xfs_ail_min(&(mp->m_ail.xa_ail));
+	lip = xfs_ail_min(&(mp->m_ail));
 	*gen = (int)mp->m_ail.xa_gen;
 
 	return lip;
@@ -482,9 +480,9 @@ xfs_trans_next_ail(
 
 	ASSERT(mp && lip && gen);
 	if (mp->m_ail.xa_gen == *gen) {
-		nlip = xfs_ail_next(&(mp->m_ail.xa_ail), lip);
+		nlip = xfs_ail_next(&(mp->m_ail), lip);
 	} else {
-		nlip = xfs_ail_min(&(mp->m_ail).xa_ail);
+		nlip = xfs_ail_min(&(mp->m_ail));
 		*gen = (int)mp->m_ail.xa_gen;
 		if (restarts != NULL) {
 			XFS_STATS_INC(xs_push_ail_restarts);
@@ -514,8 +512,7 @@ int
 xfs_trans_ail_init(
 	xfs_mount_t	*mp)
 {
-	mp->m_ail.xa_ail.ail_forw = (xfs_log_item_t*)&mp->m_ail.xa_ail;
-	mp->m_ail.xa_ail.ail_back = (xfs_log_item_t*)&mp->m_ail.xa_ail;
+	INIT_LIST_HEAD(&mp->m_ail.xa_ail);
 	return xfsaild_start(mp);
 }
 
@@ -534,7 +531,7 @@ xfs_trans_ail_destroy(
  */
 STATIC void
 xfs_ail_insert(
-	xfs_ail_entry_t	*base,
+	xfs_ail_t	*ailp,
 	xfs_log_item_t	*lip)
 /* ARGSUSED */
 {
@@ -543,27 +540,22 @@ xfs_ail_insert(
 	/*
 	 * If the list is empty, just insert the item.
 	 */
-	if (base->ail_back == (xfs_log_item_t*)base) {
-		base->ail_forw = lip;
-		base->ail_back = lip;
-		lip->li_ail.ail_forw = (xfs_log_item_t*)base;
-		lip->li_ail.ail_back = (xfs_log_item_t*)base;
+	if (list_empty(&ailp->xa_ail)) {
+		list_add(&lip->li_ail, &ailp->xa_ail);
 		return;
 	}
 
-	next_lip = base->ail_back;
-	while ((next_lip != (xfs_log_item_t*)base) &&
-	       (XFS_LSN_CMP(next_lip->li_lsn, lip->li_lsn) > 0)) {
-		next_lip = next_lip->li_ail.ail_back;
+	list_for_each_entry_reverse(next_lip, &ailp->xa_ail, li_ail) {
+		if (XFS_LSN_CMP(next_lip->li_lsn, lip->li_lsn) <= 0)
+			break;
 	}
-	ASSERT((next_lip == (xfs_log_item_t*)base) ||
+
+	ASSERT((&next_lip->li_ail == &ailp->xa_ail) ||
 	       (XFS_LSN_CMP(next_lip->li_lsn, lip->li_lsn) <= 0));
-	lip->li_ail.ail_forw = next_lip->li_ail.ail_forw;
-	lip->li_ail.ail_back = next_lip;
-	next_lip->li_ail.ail_forw = lip;
-	lip->li_ail.ail_forw->li_ail.ail_back = lip;
 
-	xfs_ail_check(base, lip);
+	list_add(&lip->li_ail, &next_lip->li_ail);
+
+	xfs_ail_check(ailp, lip);
 	return;
 }
 
@@ -573,15 +565,13 @@ xfs_ail_insert(
 /*ARGSUSED*/
 STATIC xfs_log_item_t *
 xfs_ail_delete(
-	xfs_ail_entry_t	*base,
+	xfs_ail_t	*ailp,
 	xfs_log_item_t	*lip)
 /* ARGSUSED */
 {
-	xfs_ail_check(base, lip);
-	lip->li_ail.ail_forw->li_ail.ail_back = lip->li_ail.ail_back;
-	lip->li_ail.ail_back->li_ail.ail_forw = lip->li_ail.ail_forw;
-	lip->li_ail.ail_forw = NULL;
-	lip->li_ail.ail_back = NULL;
+	xfs_ail_check(ailp, lip);
+
+	list_del(&lip->li_ail);
 
 	return lip;
 }
@@ -592,14 +582,13 @@ xfs_ail_delete(
  */
 STATIC xfs_log_item_t *
 xfs_ail_min(
-	xfs_ail_entry_t	*base)
+	xfs_ail_t	*ailp)
 /* ARGSUSED */
 {
-	register xfs_log_item_t *forw = base->ail_forw;
-	if (forw == (xfs_log_item_t*)base) {
+	if (list_empty(&ailp->xa_ail))
 		return NULL;
-	}
-	return forw;
+
+	return list_first_entry(&ailp->xa_ail, xfs_log_item_t, li_ail);
 }
 
 /*
@@ -609,15 +598,14 @@ xfs_ail_min(
  */
 STATIC xfs_log_item_t *
 xfs_ail_next(
-	xfs_ail_entry_t	*base,
+	xfs_ail_t	*ailp,
 	xfs_log_item_t	*lip)
 /* ARGSUSED */
 {
-	if (lip->li_ail.ail_forw == (xfs_log_item_t*)base) {
+	if (lip->li_ail.next == &ailp->xa_ail)
 		return NULL;
-	}
-	return lip->li_ail.ail_forw;
 
+	return list_first_entry(&lip->li_ail, xfs_log_item_t, li_ail);
 }
 
 #ifdef DEBUG
@@ -626,57 +614,40 @@ xfs_ail_next(
  */
 STATIC void
 xfs_ail_check(
-	xfs_ail_entry_t *base,
+	xfs_ail_t 	*ailp,
 	xfs_log_item_t	*lip)
 {
 	xfs_log_item_t	*prev_lip;
 
-	prev_lip = base->ail_forw;
-	if (prev_lip == (xfs_log_item_t*)base) {
-		/*
-		 * Make sure the pointers are correct when the list
-		 * is empty.
-		 */
-		ASSERT(base->ail_back == (xfs_log_item_t*)base);
+	if (list_empty(&ailp->xa_ail))
 		return;
-	}
 
 	/*
 	 * Check the next and previous entries are valid.
 	 */
 	ASSERT((lip->li_flags & XFS_LI_IN_AIL) != 0);
-	prev_lip = lip->li_ail.ail_back;
-	if (prev_lip != (xfs_log_item_t*)base) {
-		ASSERT(prev_lip->li_ail.ail_forw == lip);
+	prev_lip = list_entry(lip->li_ail.prev, xfs_log_item_t, li_ail);
+	if (&prev_lip->li_ail != &ailp->xa_ail)
 		ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) <= 0);
-	}
-	prev_lip = lip->li_ail.ail_forw;
-	if (prev_lip != (xfs_log_item_t*)base) {
-		ASSERT(prev_lip->li_ail.ail_back == lip);
+
+	prev_lip = list_entry(lip->li_ail.next, xfs_log_item_t, li_ail);
+	if (&prev_lip->li_ail != &ailp->xa_ail)
 		ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) >= 0);
-	}
 
 
 #ifdef XFS_TRANS_DEBUG
 	/*
-	 * Walk the list checking forward and backward pointers,
-	 * lsn ordering, and that every entry has the XFS_LI_IN_AIL
-	 * flag set. This is really expensive, so only do it when
-	 * specifically debugging the transaction subsystem.
+	 * Walk the list checking lsn ordering, and that every entry has the
+	 * XFS_LI_IN_AIL flag set. This is really expensive, so only do it
+	 * when specifically debugging the transaction subsystem.
 	 */
-	prev_lip = (xfs_log_item_t*)base;
-	while (lip != (xfs_log_item_t*)base) {
-		if (prev_lip != (xfs_log_item_t*)base) {
-			ASSERT(prev_lip->li_ail.ail_forw == lip);
+	prev_lip = list_entry(&ailp->xa_ail, xfs_log_item_t, li_ail);
+	list_for_each_entry(lip, &ailp->xa_ail, li_ail) {
+		if (&prev_lip->li_ail != &ailp->xa_ail)
 			ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) <= 0);
-		}
-		ASSERT(lip->li_ail.ail_back == prev_lip);
 		ASSERT((lip->li_flags & XFS_LI_IN_AIL) != 0);
 		prev_lip = lip;
-		lip = lip->li_ail.ail_forw;
 	}
-	ASSERT(lip == (xfs_log_item_t*)base);
-	ASSERT(base->ail_back == prev_lip);
 #endif /* XFS_TRANS_DEBUG */
 }
 #endif /* DEBUG */
-- 
1.5.4.rc2.85.g9de45-dirty

Re: [PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[Christoph Hellwig]

On Mon, Feb 04, 2008 at 01:28:08AM -0500, Josef 'Jeff' Sipek wrote:
> Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
> ---
> This patch assumes you already have Dave Chinner's patch for
> xfsidbg_xlogitem and xfsidbg_xaildump is needed.
> 
> Changes since V1:
> 
> - Pass around a pointer to the AIL, not the struct list_head
> - Make sure things compile & run with CONFIG_XFS_DEBUG

Does it work with XFS_TRANS_DEBUG defined aswell?

> -	lip = xfs_ail_min(&(mp->m_ail.xa_ail));
> +	lip = xfs_ail_min(&(mp->m_ail));

Care to remove these useless braces in all the places you touch while you're at it?

Re: [PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[Josef 'Jeff' Sipek]

On Mon, Feb 04, 2008 at 03:52:30PM -0500, Christoph Hellwig wrote:
> On Mon, Feb 04, 2008 at 01:28:08AM -0500, Josef 'Jeff' Sipek wrote:
> > Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
> > ---
> > This patch assumes you already have Dave Chinner's patch for
> > xfsidbg_xlogitem and xfsidbg_xaildump is needed.
> > 
> > Changes since V1:
> > 
> > - Pass around a pointer to the AIL, not the struct list_head
> > - Make sure things compile & run with CONFIG_XFS_DEBUG
> 
> Does it work with XFS_TRANS_DEBUG defined aswell?
 
With XFS_TRANS_DEBUG on, other places in XFS don't compile, but this does
and works (xfsqa ran fine).

> > -	lip = xfs_ail_min(&(mp->m_ail.xa_ail));
> > +	lip = xfs_ail_min(&(mp->m_ail));
> 
> Care to remove these useless braces in all the places you touch while you're at it?

Will do.

Josef 'Jeff' Sipek.

-- 
The box said "Windows XP or better required". So I installed Linux.

[PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[Josef 'Jeff' Sipek]

Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
---
This patch assumes you already have Dave Chinner's patch for
xfsidbg_xlogitem and xfsidbg_xaildump is needed.

Changes since V2:

- remove extra parenthesis

Changes since V1:

- Pass around a pointer to the AIL, not the struct list_head
- Make sure things compile & run with CONFIG_XFS_DEBUG
---
 fs/xfs/xfs_mount.h     |    2 +-
 fs/xfs/xfs_trans.h     |    7 +--
 fs/xfs/xfs_trans_ail.c |  149 +++++++++++++++++++----------------------------
 3 files changed, 62 insertions(+), 96 deletions(-)

diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index f7c620e..435d625 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -220,7 +220,7 @@ extern void	xfs_icsb_sync_counters_flags(struct xfs_mount *, int);
 #endif
 
 typedef struct xfs_ail {
-	xfs_ail_entry_t		xa_ail;
+	struct list_head	xa_ail;
 	uint			xa_gen;
 	struct task_struct	*xa_task;
 	xfs_lsn_t		xa_target;
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 7f40628..50ce02b 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -113,13 +113,8 @@ struct xfs_mount;
 struct xfs_trans;
 struct xfs_dquot_acct;
 
-typedef struct xfs_ail_entry {
-	struct xfs_log_item	*ail_forw;	/* AIL forw pointer */
-	struct xfs_log_item	*ail_back;	/* AIL back pointer */
-} xfs_ail_entry_t;
-
 typedef struct xfs_log_item {
-	xfs_ail_entry_t			li_ail;		/* AIL pointers */
+	struct list_head		li_ail;		/* AIL pointers */
 	xfs_lsn_t			li_lsn;		/* last on-disk lsn */
 	struct xfs_log_item_desc	*li_desc;	/* ptr to current desc*/
 	struct xfs_mount		*li_mountp;	/* ptr to fs mount */
diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 4d6330e..0fe9d59 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -28,13 +28,13 @@
 #include "xfs_trans_priv.h"
 #include "xfs_error.h"
 
-STATIC void xfs_ail_insert(xfs_ail_entry_t *, xfs_log_item_t *);
-STATIC xfs_log_item_t * xfs_ail_delete(xfs_ail_entry_t *, xfs_log_item_t *);
-STATIC xfs_log_item_t * xfs_ail_min(xfs_ail_entry_t *);
-STATIC xfs_log_item_t * xfs_ail_next(xfs_ail_entry_t *, xfs_log_item_t *);
+STATIC void xfs_ail_insert(xfs_ail_t *, xfs_log_item_t *);
+STATIC xfs_log_item_t * xfs_ail_delete(xfs_ail_t *, xfs_log_item_t *);
+STATIC xfs_log_item_t * xfs_ail_min(xfs_ail_t *);
+STATIC xfs_log_item_t * xfs_ail_next(xfs_ail_t *, xfs_log_item_t *);
 
 #ifdef DEBUG
-STATIC void xfs_ail_check(xfs_ail_entry_t *, xfs_log_item_t *);
+STATIC void xfs_ail_check(xfs_ail_t *, xfs_log_item_t *);
 #else
 #define	xfs_ail_check(a,l)
 #endif /* DEBUG */
@@ -57,7 +57,7 @@ xfs_trans_tail_ail(
 	xfs_log_item_t	*lip;
 
 	spin_lock(&mp->m_ail_lock);
-	lip = xfs_ail_min(&(mp->m_ail.xa_ail));
+	lip = xfs_ail_min(&mp->m_ail);
 	if (lip == NULL) {
 		lsn = (xfs_lsn_t)0;
 	} else {
@@ -91,7 +91,7 @@ xfs_trans_push_ail(
 {
 	xfs_log_item_t		*lip;
 
-	lip = xfs_ail_min(&mp->m_ail.xa_ail);
+	lip = xfs_ail_min(&mp->m_ail);
 	if (lip && !XFS_FORCED_SHUTDOWN(mp)) {
 		if (XFS_LSN_CMP(threshold_lsn, mp->m_ail.xa_target) > 0)
 			xfsaild_wakeup(mp, threshold_lsn);
@@ -111,15 +111,17 @@ xfs_trans_first_push_ail(
 {
 	xfs_log_item_t	*lip;
 
-	lip = xfs_ail_min(&(mp->m_ail.xa_ail));
+	lip = xfs_ail_min(&mp->m_ail);
 	*gen = (int)mp->m_ail.xa_gen;
 	if (lsn == 0)
 		return lip;
 
-	while (lip && (XFS_LSN_CMP(lip->li_lsn, lsn) < 0))
-		lip = lip->li_ail.ail_forw;
+	list_for_each_entry(lip, &mp->m_ail.xa_ail, li_ail) {
+		if (XFS_LSN_CMP(lip->li_lsn, lsn) >= 0)
+			return lip;
+	}
 
-	return lip;
+	return NULL;
 }
 
 /*
@@ -326,7 +328,7 @@ xfs_trans_unlocked_item(
 	 * the call to xfs_log_move_tail() doesn't do anything if there's
 	 * not enough free space to wake people up so we're safe calling it.
 	 */
-	min_lip = xfs_ail_min(&mp->m_ail.xa_ail);
+	min_lip = xfs_ail_min(&mp->m_ail);
 
 	if (min_lip == lip)
 		xfs_log_move_tail(mp, 1);
@@ -354,15 +356,13 @@ xfs_trans_update_ail(
 	xfs_log_item_t	*lip,
 	xfs_lsn_t	lsn) __releases(mp->m_ail_lock)
 {
-	xfs_ail_entry_t		*ailp;
 	xfs_log_item_t		*dlip=NULL;
 	xfs_log_item_t		*mlip;	/* ptr to minimum lip */
 
-	ailp = &(mp->m_ail.xa_ail);
-	mlip = xfs_ail_min(ailp);
+	mlip = xfs_ail_min(&mp->m_ail);
 
 	if (lip->li_flags & XFS_LI_IN_AIL) {
-		dlip = xfs_ail_delete(ailp, lip);
+		dlip = xfs_ail_delete(&mp->m_ail, lip);
 		ASSERT(dlip == lip);
 	} else {
 		lip->li_flags |= XFS_LI_IN_AIL;
@@ -370,11 +370,11 @@ xfs_trans_update_ail(
 
 	lip->li_lsn = lsn;
 
-	xfs_ail_insert(ailp, lip);
+	xfs_ail_insert(&mp->m_ail, lip);
 	mp->m_ail.xa_gen++;
 
 	if (mlip == dlip) {
-		mlip = xfs_ail_min(&(mp->m_ail.xa_ail));
+		mlip = xfs_ail_min(&mp->m_ail);
 		spin_unlock(&mp->m_ail_lock);
 		xfs_log_move_tail(mp, mlip->li_lsn);
 	} else {
@@ -404,14 +404,12 @@ xfs_trans_delete_ail(
 	xfs_mount_t	*mp,
 	xfs_log_item_t	*lip) __releases(mp->m_ail_lock)
 {
-	xfs_ail_entry_t		*ailp;
 	xfs_log_item_t		*dlip;
 	xfs_log_item_t		*mlip;
 
 	if (lip->li_flags & XFS_LI_IN_AIL) {
-		ailp = &(mp->m_ail.xa_ail);
-		mlip = xfs_ail_min(ailp);
-		dlip = xfs_ail_delete(ailp, lip);
+		mlip = xfs_ail_min(&mp->m_ail);
+		dlip = xfs_ail_delete(&mp->m_ail, lip);
 		ASSERT(dlip == lip);
 
 
@@ -420,7 +418,7 @@ xfs_trans_delete_ail(
 		mp->m_ail.xa_gen++;
 
 		if (mlip == dlip) {
-			mlip = xfs_ail_min(&(mp->m_ail.xa_ail));
+			mlip = xfs_ail_min(&mp->m_ail);
 			spin_unlock(&mp->m_ail_lock);
 			xfs_log_move_tail(mp, (mlip ? mlip->li_lsn : 0));
 		} else {
@@ -458,7 +456,7 @@ xfs_trans_first_ail(
 {
 	xfs_log_item_t	*lip;
 
-	lip = xfs_ail_min(&(mp->m_ail.xa_ail));
+	lip = xfs_ail_min(&mp->m_ail);
 	*gen = (int)mp->m_ail.xa_gen;
 
 	return lip;
@@ -482,9 +480,9 @@ xfs_trans_next_ail(
 
 	ASSERT(mp && lip && gen);
 	if (mp->m_ail.xa_gen == *gen) {
-		nlip = xfs_ail_next(&(mp->m_ail.xa_ail), lip);
+		nlip = xfs_ail_next(&mp->m_ail, lip);
 	} else {
-		nlip = xfs_ail_min(&(mp->m_ail).xa_ail);
+		nlip = xfs_ail_min(&mp->m_ail);
 		*gen = (int)mp->m_ail.xa_gen;
 		if (restarts != NULL) {
 			XFS_STATS_INC(xs_push_ail_restarts);
@@ -514,8 +512,7 @@ int
 xfs_trans_ail_init(
 	xfs_mount_t	*mp)
 {
-	mp->m_ail.xa_ail.ail_forw = (xfs_log_item_t*)&mp->m_ail.xa_ail;
-	mp->m_ail.xa_ail.ail_back = (xfs_log_item_t*)&mp->m_ail.xa_ail;
+	INIT_LIST_HEAD(&mp->m_ail.xa_ail);
 	return xfsaild_start(mp);
 }
 
@@ -534,7 +531,7 @@ xfs_trans_ail_destroy(
  */
 STATIC void
 xfs_ail_insert(
-	xfs_ail_entry_t	*base,
+	xfs_ail_t	*ailp,
 	xfs_log_item_t	*lip)
 /* ARGSUSED */
 {
@@ -543,27 +540,22 @@ xfs_ail_insert(
 	/*
 	 * If the list is empty, just insert the item.
 	 */
-	if (base->ail_back == (xfs_log_item_t*)base) {
-		base->ail_forw = lip;
-		base->ail_back = lip;
-		lip->li_ail.ail_forw = (xfs_log_item_t*)base;
-		lip->li_ail.ail_back = (xfs_log_item_t*)base;
+	if (list_empty(&ailp->xa_ail)) {
+		list_add(&lip->li_ail, &ailp->xa_ail);
 		return;
 	}
 
-	next_lip = base->ail_back;
-	while ((next_lip != (xfs_log_item_t*)base) &&
-	       (XFS_LSN_CMP(next_lip->li_lsn, lip->li_lsn) > 0)) {
-		next_lip = next_lip->li_ail.ail_back;
+	list_for_each_entry_reverse(next_lip, &ailp->xa_ail, li_ail) {
+		if (XFS_LSN_CMP(next_lip->li_lsn, lip->li_lsn) <= 0)
+			break;
 	}
-	ASSERT((next_lip == (xfs_log_item_t*)base) ||
+
+	ASSERT((&next_lip->li_ail == &ailp->xa_ail) ||
 	       (XFS_LSN_CMP(next_lip->li_lsn, lip->li_lsn) <= 0));
-	lip->li_ail.ail_forw = next_lip->li_ail.ail_forw;
-	lip->li_ail.ail_back = next_lip;
-	next_lip->li_ail.ail_forw = lip;
-	lip->li_ail.ail_forw->li_ail.ail_back = lip;
 
-	xfs_ail_check(base, lip);
+	list_add(&lip->li_ail, &next_lip->li_ail);
+
+	xfs_ail_check(ailp, lip);
 	return;
 }
 
@@ -573,15 +565,13 @@ xfs_ail_insert(
 /*ARGSUSED*/
 STATIC xfs_log_item_t *
 xfs_ail_delete(
-	xfs_ail_entry_t	*base,
+	xfs_ail_t	*ailp,
 	xfs_log_item_t	*lip)
 /* ARGSUSED */
 {
-	xfs_ail_check(base, lip);
-	lip->li_ail.ail_forw->li_ail.ail_back = lip->li_ail.ail_back;
-	lip->li_ail.ail_back->li_ail.ail_forw = lip->li_ail.ail_forw;
-	lip->li_ail.ail_forw = NULL;
-	lip->li_ail.ail_back = NULL;
+	xfs_ail_check(ailp, lip);
+
+	list_del(&lip->li_ail);
 
 	return lip;
 }
@@ -592,14 +582,13 @@ xfs_ail_delete(
  */
 STATIC xfs_log_item_t *
 xfs_ail_min(
-	xfs_ail_entry_t	*base)
+	xfs_ail_t	*ailp)
 /* ARGSUSED */
 {
-	register xfs_log_item_t *forw = base->ail_forw;
-	if (forw == (xfs_log_item_t*)base) {
+	if (list_empty(&ailp->xa_ail))
 		return NULL;
-	}
-	return forw;
+
+	return list_first_entry(&ailp->xa_ail, xfs_log_item_t, li_ail);
 }
 
 /*
@@ -609,15 +598,14 @@ xfs_ail_min(
  */
 STATIC xfs_log_item_t *
 xfs_ail_next(
-	xfs_ail_entry_t	*base,
+	xfs_ail_t	*ailp,
 	xfs_log_item_t	*lip)
 /* ARGSUSED */
 {
-	if (lip->li_ail.ail_forw == (xfs_log_item_t*)base) {
+	if (lip->li_ail.next == &ailp->xa_ail)
 		return NULL;
-	}
-	return lip->li_ail.ail_forw;
 
+	return list_first_entry(&lip->li_ail, xfs_log_item_t, li_ail);
 }
 
 #ifdef DEBUG
@@ -626,57 +614,40 @@ xfs_ail_next(
  */
 STATIC void
 xfs_ail_check(
-	xfs_ail_entry_t *base,
+	xfs_ail_t 	*ailp,
 	xfs_log_item_t	*lip)
 {
 	xfs_log_item_t	*prev_lip;
 
-	prev_lip = base->ail_forw;
-	if (prev_lip == (xfs_log_item_t*)base) {
-		/*
-		 * Make sure the pointers are correct when the list
-		 * is empty.
-		 */
-		ASSERT(base->ail_back == (xfs_log_item_t*)base);
+	if (list_empty(&ailp->xa_ail))
 		return;
-	}
 
 	/*
 	 * Check the next and previous entries are valid.
 	 */
 	ASSERT((lip->li_flags & XFS_LI_IN_AIL) != 0);
-	prev_lip = lip->li_ail.ail_back;
-	if (prev_lip != (xfs_log_item_t*)base) {
-		ASSERT(prev_lip->li_ail.ail_forw == lip);
+	prev_lip = list_entry(lip->li_ail.prev, xfs_log_item_t, li_ail);
+	if (&prev_lip->li_ail != &ailp->xa_ail)
 		ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) <= 0);
-	}
-	prev_lip = lip->li_ail.ail_forw;
-	if (prev_lip != (xfs_log_item_t*)base) {
-		ASSERT(prev_lip->li_ail.ail_back == lip);
+
+	prev_lip = list_entry(lip->li_ail.next, xfs_log_item_t, li_ail);
+	if (&prev_lip->li_ail != &ailp->xa_ail)
 		ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) >= 0);
-	}
 
 
 #ifdef XFS_TRANS_DEBUG
 	/*
-	 * Walk the list checking forward and backward pointers,
-	 * lsn ordering, and that every entry has the XFS_LI_IN_AIL
-	 * flag set. This is really expensive, so only do it when
-	 * specifically debugging the transaction subsystem.
+	 * Walk the list checking lsn ordering, and that every entry has the
+	 * XFS_LI_IN_AIL flag set. This is really expensive, so only do it
+	 * when specifically debugging the transaction subsystem.
 	 */
-	prev_lip = (xfs_log_item_t*)base;
-	while (lip != (xfs_log_item_t*)base) {
-		if (prev_lip != (xfs_log_item_t*)base) {
-			ASSERT(prev_lip->li_ail.ail_forw == lip);
+	prev_lip = list_entry(&ailp->xa_ail, xfs_log_item_t, li_ail);
+	list_for_each_entry(lip, &ailp->xa_ail, li_ail) {
+		if (&prev_lip->li_ail != &ailp->xa_ail)
 			ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) <= 0);
-		}
-		ASSERT(lip->li_ail.ail_back == prev_lip);
 		ASSERT((lip->li_flags & XFS_LI_IN_AIL) != 0);
 		prev_lip = lip;
-		lip = lip->li_ail.ail_forw;
 	}
-	ASSERT(lip == (xfs_log_item_t*)base);
-	ASSERT(base->ail_back == prev_lip);
 #endif /* XFS_TRANS_DEBUG */
 }
 #endif /* DEBUG */
-- 
1.5.4.rc2.85.g9de45-dirty

Re: [PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[Josef 'Jeff' Sipek]

ping?

On Tue, Feb 05, 2008 at 11:44:51PM -0500, Josef 'Jeff' Sipek wrote:
> Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
> ---
> This patch assumes you already have Dave Chinner's patch for
> xfsidbg_xlogitem and xfsidbg_xaildump is needed.
>
> Changes since V2:
> 
> - remove extra parenthesis
> 
> Changes since V1:
> 
> - Pass around a pointer to the AIL, not the struct list_head
> - Make sure things compile & run with CONFIG_XFS_DEBUG
> ---
>  fs/xfs/xfs_mount.h     |    2 +-
>  fs/xfs/xfs_trans.h     |    7 +--
>  fs/xfs/xfs_trans_ail.c |  149 +++++++++++++++++++----------------------------
>  3 files changed, 62 insertions(+), 96 deletions(-)
> 
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index f7c620e..435d625 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -220,7 +220,7 @@ extern void	xfs_icsb_sync_counters_flags(struct xfs_mount *, int);
>  #endif
>  
>  typedef struct xfs_ail {
> -	xfs_ail_entry_t		xa_ail;
> +	struct list_head	xa_ail;
>  	uint			xa_gen;
>  	struct task_struct	*xa_task;
>  	xfs_lsn_t		xa_target;
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index 7f40628..50ce02b 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -113,13 +113,8 @@ struct xfs_mount;
>  struct xfs_trans;
>  struct xfs_dquot_acct;
>  
> -typedef struct xfs_ail_entry {
> -	struct xfs_log_item	*ail_forw;	/* AIL forw pointer */
> -	struct xfs_log_item	*ail_back;	/* AIL back pointer */
> -} xfs_ail_entry_t;
> -
>  typedef struct xfs_log_item {
> -	xfs_ail_entry_t			li_ail;		/* AIL pointers */
> +	struct list_head		li_ail;		/* AIL pointers */
>  	xfs_lsn_t			li_lsn;		/* last on-disk lsn */
>  	struct xfs_log_item_desc	*li_desc;	/* ptr to current desc*/
>  	struct xfs_mount		*li_mountp;	/* ptr to fs mount */
> diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> index 4d6330e..0fe9d59 100644
> --- a/fs/xfs/xfs_trans_ail.c
> +++ b/fs/xfs/xfs_trans_ail.c
> @@ -28,13 +28,13 @@
>  #include "xfs_trans_priv.h"
>  #include "xfs_error.h"
>  
> -STATIC void xfs_ail_insert(xfs_ail_entry_t *, xfs_log_item_t *);
> -STATIC xfs_log_item_t * xfs_ail_delete(xfs_ail_entry_t *, xfs_log_item_t *);
> -STATIC xfs_log_item_t * xfs_ail_min(xfs_ail_entry_t *);
> -STATIC xfs_log_item_t * xfs_ail_next(xfs_ail_entry_t *, xfs_log_item_t *);
> +STATIC void xfs_ail_insert(xfs_ail_t *, xfs_log_item_t *);
> +STATIC xfs_log_item_t * xfs_ail_delete(xfs_ail_t *, xfs_log_item_t *);
> +STATIC xfs_log_item_t * xfs_ail_min(xfs_ail_t *);
> +STATIC xfs_log_item_t * xfs_ail_next(xfs_ail_t *, xfs_log_item_t *);
>  
>  #ifdef DEBUG
> -STATIC void xfs_ail_check(xfs_ail_entry_t *, xfs_log_item_t *);
> +STATIC void xfs_ail_check(xfs_ail_t *, xfs_log_item_t *);
>  #else
>  #define	xfs_ail_check(a,l)
>  #endif /* DEBUG */
> @@ -57,7 +57,7 @@ xfs_trans_tail_ail(
>  	xfs_log_item_t	*lip;
>  
>  	spin_lock(&mp->m_ail_lock);
> -	lip = xfs_ail_min(&(mp->m_ail.xa_ail));
> +	lip = xfs_ail_min(&mp->m_ail);
>  	if (lip == NULL) {
>  		lsn = (xfs_lsn_t)0;
>  	} else {
> @@ -91,7 +91,7 @@ xfs_trans_push_ail(
>  {
>  	xfs_log_item_t		*lip;
>  
> -	lip = xfs_ail_min(&mp->m_ail.xa_ail);
> +	lip = xfs_ail_min(&mp->m_ail);
>  	if (lip && !XFS_FORCED_SHUTDOWN(mp)) {
>  		if (XFS_LSN_CMP(threshold_lsn, mp->m_ail.xa_target) > 0)
>  			xfsaild_wakeup(mp, threshold_lsn);
> @@ -111,15 +111,17 @@ xfs_trans_first_push_ail(
>  {
>  	xfs_log_item_t	*lip;
>  
> -	lip = xfs_ail_min(&(mp->m_ail.xa_ail));
> +	lip = xfs_ail_min(&mp->m_ail);
>  	*gen = (int)mp->m_ail.xa_gen;
>  	if (lsn == 0)
>  		return lip;
>  
> -	while (lip && (XFS_LSN_CMP(lip->li_lsn, lsn) < 0))
> -		lip = lip->li_ail.ail_forw;
> +	list_for_each_entry(lip, &mp->m_ail.xa_ail, li_ail) {
> +		if (XFS_LSN_CMP(lip->li_lsn, lsn) >= 0)
> +			return lip;
> +	}
>  
> -	return lip;
> +	return NULL;
>  }
>  
>  /*
> @@ -326,7 +328,7 @@ xfs_trans_unlocked_item(
>  	 * the call to xfs_log_move_tail() doesn't do anything if there's
>  	 * not enough free space to wake people up so we're safe calling it.
>  	 */
> -	min_lip = xfs_ail_min(&mp->m_ail.xa_ail);
> +	min_lip = xfs_ail_min(&mp->m_ail);
>  
>  	if (min_lip == lip)
>  		xfs_log_move_tail(mp, 1);
> @@ -354,15 +356,13 @@ xfs_trans_update_ail(
>  	xfs_log_item_t	*lip,
>  	xfs_lsn_t	lsn) __releases(mp->m_ail_lock)
>  {
> -	xfs_ail_entry_t		*ailp;
>  	xfs_log_item_t		*dlip=NULL;
>  	xfs_log_item_t		*mlip;	/* ptr to minimum lip */
>  
> -	ailp = &(mp->m_ail.xa_ail);
> -	mlip = xfs_ail_min(ailp);
> +	mlip = xfs_ail_min(&mp->m_ail);
>  
>  	if (lip->li_flags & XFS_LI_IN_AIL) {
> -		dlip = xfs_ail_delete(ailp, lip);
> +		dlip = xfs_ail_delete(&mp->m_ail, lip);
>  		ASSERT(dlip == lip);
>  	} else {
>  		lip->li_flags |= XFS_LI_IN_AIL;
> @@ -370,11 +370,11 @@ xfs_trans_update_ail(
>  
>  	lip->li_lsn = lsn;
>  
> -	xfs_ail_insert(ailp, lip);
> +	xfs_ail_insert(&mp->m_ail, lip);
>  	mp->m_ail.xa_gen++;
>  
>  	if (mlip == dlip) {
> -		mlip = xfs_ail_min(&(mp->m_ail.xa_ail));
> +		mlip = xfs_ail_min(&mp->m_ail);
>  		spin_unlock(&mp->m_ail_lock);
>  		xfs_log_move_tail(mp, mlip->li_lsn);
>  	} else {
> @@ -404,14 +404,12 @@ xfs_trans_delete_ail(
>  	xfs_mount_t	*mp,
>  	xfs_log_item_t	*lip) __releases(mp->m_ail_lock)
>  {
> -	xfs_ail_entry_t		*ailp;
>  	xfs_log_item_t		*dlip;
>  	xfs_log_item_t		*mlip;
>  
>  	if (lip->li_flags & XFS_LI_IN_AIL) {
> -		ailp = &(mp->m_ail.xa_ail);
> -		mlip = xfs_ail_min(ailp);
> -		dlip = xfs_ail_delete(ailp, lip);
> +		mlip = xfs_ail_min(&mp->m_ail);
> +		dlip = xfs_ail_delete(&mp->m_ail, lip);
>  		ASSERT(dlip == lip);
>  
>  
> @@ -420,7 +418,7 @@ xfs_trans_delete_ail(
>  		mp->m_ail.xa_gen++;
>  
>  		if (mlip == dlip) {
> -			mlip = xfs_ail_min(&(mp->m_ail.xa_ail));
> +			mlip = xfs_ail_min(&mp->m_ail);
>  			spin_unlock(&mp->m_ail_lock);
>  			xfs_log_move_tail(mp, (mlip ? mlip->li_lsn : 0));
>  		} else {
> @@ -458,7 +456,7 @@ xfs_trans_first_ail(
>  {
>  	xfs_log_item_t	*lip;
>  
> -	lip = xfs_ail_min(&(mp->m_ail.xa_ail));
> +	lip = xfs_ail_min(&mp->m_ail);
>  	*gen = (int)mp->m_ail.xa_gen;
>  
>  	return lip;
> @@ -482,9 +480,9 @@ xfs_trans_next_ail(
>  
>  	ASSERT(mp && lip && gen);
>  	if (mp->m_ail.xa_gen == *gen) {
> -		nlip = xfs_ail_next(&(mp->m_ail.xa_ail), lip);
> +		nlip = xfs_ail_next(&mp->m_ail, lip);
>  	} else {
> -		nlip = xfs_ail_min(&(mp->m_ail).xa_ail);
> +		nlip = xfs_ail_min(&mp->m_ail);
>  		*gen = (int)mp->m_ail.xa_gen;
>  		if (restarts != NULL) {
>  			XFS_STATS_INC(xs_push_ail_restarts);
> @@ -514,8 +512,7 @@ int
>  xfs_trans_ail_init(
>  	xfs_mount_t	*mp)
>  {
> -	mp->m_ail.xa_ail.ail_forw = (xfs_log_item_t*)&mp->m_ail.xa_ail;
> -	mp->m_ail.xa_ail.ail_back = (xfs_log_item_t*)&mp->m_ail.xa_ail;
> +	INIT_LIST_HEAD(&mp->m_ail.xa_ail);
>  	return xfsaild_start(mp);
>  }
>  
> @@ -534,7 +531,7 @@ xfs_trans_ail_destroy(
>   */
>  STATIC void
>  xfs_ail_insert(
> -	xfs_ail_entry_t	*base,
> +	xfs_ail_t	*ailp,
>  	xfs_log_item_t	*lip)
>  /* ARGSUSED */
>  {
> @@ -543,27 +540,22 @@ xfs_ail_insert(
>  	/*
>  	 * If the list is empty, just insert the item.
>  	 */
> -	if (base->ail_back == (xfs_log_item_t*)base) {
> -		base->ail_forw = lip;
> -		base->ail_back = lip;
> -		lip->li_ail.ail_forw = (xfs_log_item_t*)base;
> -		lip->li_ail.ail_back = (xfs_log_item_t*)base;
> +	if (list_empty(&ailp->xa_ail)) {
> +		list_add(&lip->li_ail, &ailp->xa_ail);
>  		return;
>  	}
>  
> -	next_lip = base->ail_back;
> -	while ((next_lip != (xfs_log_item_t*)base) &&
> -	       (XFS_LSN_CMP(next_lip->li_lsn, lip->li_lsn) > 0)) {
> -		next_lip = next_lip->li_ail.ail_back;
> +	list_for_each_entry_reverse(next_lip, &ailp->xa_ail, li_ail) {
> +		if (XFS_LSN_CMP(next_lip->li_lsn, lip->li_lsn) <= 0)
> +			break;
>  	}
> -	ASSERT((next_lip == (xfs_log_item_t*)base) ||
> +
> +	ASSERT((&next_lip->li_ail == &ailp->xa_ail) ||
>  	       (XFS_LSN_CMP(next_lip->li_lsn, lip->li_lsn) <= 0));
> -	lip->li_ail.ail_forw = next_lip->li_ail.ail_forw;
> -	lip->li_ail.ail_back = next_lip;
> -	next_lip->li_ail.ail_forw = lip;
> -	lip->li_ail.ail_forw->li_ail.ail_back = lip;
>  
> -	xfs_ail_check(base, lip);
> +	list_add(&lip->li_ail, &next_lip->li_ail);
> +
> +	xfs_ail_check(ailp, lip);
>  	return;
>  }
>  
> @@ -573,15 +565,13 @@ xfs_ail_insert(
>  /*ARGSUSED*/
>  STATIC xfs_log_item_t *
>  xfs_ail_delete(
> -	xfs_ail_entry_t	*base,
> +	xfs_ail_t	*ailp,
>  	xfs_log_item_t	*lip)
>  /* ARGSUSED */
>  {
> -	xfs_ail_check(base, lip);
> -	lip->li_ail.ail_forw->li_ail.ail_back = lip->li_ail.ail_back;
> -	lip->li_ail.ail_back->li_ail.ail_forw = lip->li_ail.ail_forw;
> -	lip->li_ail.ail_forw = NULL;
> -	lip->li_ail.ail_back = NULL;
> +	xfs_ail_check(ailp, lip);
> +
> +	list_del(&lip->li_ail);
>  
>  	return lip;
>  }
> @@ -592,14 +582,13 @@ xfs_ail_delete(
>   */
>  STATIC xfs_log_item_t *
>  xfs_ail_min(
> -	xfs_ail_entry_t	*base)
> +	xfs_ail_t	*ailp)
>  /* ARGSUSED */
>  {
> -	register xfs_log_item_t *forw = base->ail_forw;
> -	if (forw == (xfs_log_item_t*)base) {
> +	if (list_empty(&ailp->xa_ail))
>  		return NULL;
> -	}
> -	return forw;
> +
> +	return list_first_entry(&ailp->xa_ail, xfs_log_item_t, li_ail);
>  }
>  
>  /*
> @@ -609,15 +598,14 @@ xfs_ail_min(
>   */
>  STATIC xfs_log_item_t *
>  xfs_ail_next(
> -	xfs_ail_entry_t	*base,
> +	xfs_ail_t	*ailp,
>  	xfs_log_item_t	*lip)
>  /* ARGSUSED */
>  {
> -	if (lip->li_ail.ail_forw == (xfs_log_item_t*)base) {
> +	if (lip->li_ail.next == &ailp->xa_ail)
>  		return NULL;
> -	}
> -	return lip->li_ail.ail_forw;
>  
> +	return list_first_entry(&lip->li_ail, xfs_log_item_t, li_ail);
>  }
>  
>  #ifdef DEBUG
> @@ -626,57 +614,40 @@ xfs_ail_next(
>   */
>  STATIC void
>  xfs_ail_check(
> -	xfs_ail_entry_t *base,
> +	xfs_ail_t 	*ailp,
>  	xfs_log_item_t	*lip)
>  {
>  	xfs_log_item_t	*prev_lip;
>  
> -	prev_lip = base->ail_forw;
> -	if (prev_lip == (xfs_log_item_t*)base) {
> -		/*
> -		 * Make sure the pointers are correct when the list
> -		 * is empty.
> -		 */
> -		ASSERT(base->ail_back == (xfs_log_item_t*)base);
> +	if (list_empty(&ailp->xa_ail))
>  		return;
> -	}
>  
>  	/*
>  	 * Check the next and previous entries are valid.
>  	 */
>  	ASSERT((lip->li_flags & XFS_LI_IN_AIL) != 0);
> -	prev_lip = lip->li_ail.ail_back;
> -	if (prev_lip != (xfs_log_item_t*)base) {
> -		ASSERT(prev_lip->li_ail.ail_forw == lip);
> +	prev_lip = list_entry(lip->li_ail.prev, xfs_log_item_t, li_ail);
> +	if (&prev_lip->li_ail != &ailp->xa_ail)
>  		ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) <= 0);
> -	}
> -	prev_lip = lip->li_ail.ail_forw;
> -	if (prev_lip != (xfs_log_item_t*)base) {
> -		ASSERT(prev_lip->li_ail.ail_back == lip);
> +
> +	prev_lip = list_entry(lip->li_ail.next, xfs_log_item_t, li_ail);
> +	if (&prev_lip->li_ail != &ailp->xa_ail)
>  		ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) >= 0);
> -	}
>  
>  
>  #ifdef XFS_TRANS_DEBUG
>  	/*
> -	 * Walk the list checking forward and backward pointers,
> -	 * lsn ordering, and that every entry has the XFS_LI_IN_AIL
> -	 * flag set. This is really expensive, so only do it when
> -	 * specifically debugging the transaction subsystem.
> +	 * Walk the list checking lsn ordering, and that every entry has the
> +	 * XFS_LI_IN_AIL flag set. This is really expensive, so only do it
> +	 * when specifically debugging the transaction subsystem.
>  	 */
> -	prev_lip = (xfs_log_item_t*)base;
> -	while (lip != (xfs_log_item_t*)base) {
> -		if (prev_lip != (xfs_log_item_t*)base) {
> -			ASSERT(prev_lip->li_ail.ail_forw == lip);
> +	prev_lip = list_entry(&ailp->xa_ail, xfs_log_item_t, li_ail);
> +	list_for_each_entry(lip, &ailp->xa_ail, li_ail) {
> +		if (&prev_lip->li_ail != &ailp->xa_ail)
>  			ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) <= 0);
> -		}
> -		ASSERT(lip->li_ail.ail_back == prev_lip);
>  		ASSERT((lip->li_flags & XFS_LI_IN_AIL) != 0);
>  		prev_lip = lip;
> -		lip = lip->li_ail.ail_forw;
>  	}
> -	ASSERT(lip == (xfs_log_item_t*)base);
> -	ASSERT(base->ail_back == prev_lip);
>  #endif /* XFS_TRANS_DEBUG */
>  }
>  #endif /* DEBUG */
> -- 
> 1.5.4.rc2.85.g9de45-dirty
> 

-- 
Linux, n.:
  Generous programmers from around the world all join forces to help
  you shoot yourself in the foot for free. 

Re: [PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[David Chinner]

On Mon, Jan 21, 2008 at 04:04:23AM +0000, Christoph Hellwig wrote:
> On Sun, Jan 20, 2008 at 07:35:57PM -0500, Josef 'Jeff' Sipek wrote:
> > Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
> > ---
> > 
> > I tested it with xfsqa, and things work as well as they do without it.
> 
> I like this a lot, but I think Dave has plans to replace the linked list
> with a more efficient data structure soon, so it might not actually be
> worth applying.

Doesn't really make any difference to me - i've got to rebase the patch
on the xfsaild thread commit anyway....

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

Re: [PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[Christoph Hellwig]

On Mon, Jan 21, 2008 at 03:12:58PM +1100, David Chinner wrote:
> Doesn't really make any difference to me - i've got to rebase the patch
> on the xfsaild thread commit anyway....

Ok, then I'm all for putting it in.  Especially as it should help
detangling the header mess.  IIRC after this patch there is no need
anymore to pull in xfs_trans.h before xfs_mount.h anymore.

Re: [PATCH 1/1] XFS: Replace custom AIL linked-list code with struct list_head

[David Chinner]

On Mon, Jan 21, 2008 at 06:54:42AM +0000, Christoph Hellwig wrote:
> On Mon, Jan 21, 2008 at 03:12:58PM +1100, David Chinner wrote:
> > Doesn't really make any difference to me - i've got to rebase the patch
> > on the xfsaild thread commit anyway....
> 
> Ok, then I'm all for putting it in.  Especially as it should help
> detangling the header mess.  IIRC after this patch there is no need
> anymore to pull in xfs_trans.h before xfs_mount.h anymore.

Ok, I'll look at that....

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group