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

[David Chinner <dgc@xxxxxxx>]

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@xxxxxxxxxxxxxx>
> > > ---
> > > 
> > > 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@xxxxxxx>
---
 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;
+                       }
                }
        }
 }