xfs
[Top] [All Lists]

Re: shutdown umount hangs

To: Utz Lehmann <leh@xxxxxxxxxx>
Subject: Re: shutdown umount hangs
From: Russell Cattelan <cattelan@xxxxxxxxxxx>
Date: Fri, 06 Apr 2001 12:52:34 -0400
Cc: linux-xfs@xxxxxxxxxxx
References: <leh@xxxxxxxxxx> <200104051412.f35ECMU25857@xxxxxxxxxxxxxxxxxxxx> <20010405172344.A1151@xxxxxxxxxx> <3ACC9D13.33DA7299@xxxxxxxxxxx> <3ACCB6A7.EA84241@xxxxxxxxxxx> <20010405221141.A1151@xxxxxxxxxx> <3ACD0535.B71075F7@xxxxxxxxxxx> <20010406115848.B1150@xxxxxxxxxx>
Sender: owner-linux-xfs@xxxxxxxxxxx
Utz Lehmann wrote:

>
> xfs_unmountfs_writesb pre  strat flags 0xa08017e  pb_iodonesema cnt(0,0)sl
> xfs_unmountfs_writesb prot strat flags 0xa08017e  pb_iodonesema cnt(0,0)sl

   yup the flags are wrong... ok I have another patch for you to try.
I think this will fix the problem.
I haven't tested it yet but you can give it a try if you want.


>
>
> (hangs)
>
> utz

--
Russell Cattelan
--
Digital Elves inc. -- Currently on loan to SGI
Linux XFS core developer.


===========================================================================
linux/fs/pagebuf/page_buf.c
===========================================================================

--- /usr/tmp/TmpDir.17129-0/linux/fs/pagebuf/page_buf.c_1.72    Fri Apr  6 
11:51:48 2001
+++ linux/fs/pagebuf/page_buf.c Fri Apr  6 11:25:53 2001
@@ -2136,6 +2136,7 @@
        struct list_head *head, *curr;
        unsigned long save;
        pagebuf_marker_t *pb_marker_ptr;
+       int lr=1; 
 
 
        pb_marker_ptr = kmalloc(sizeof(pagebuf_marker_t), GFP_KERNEL);
@@ -2152,73 +2153,58 @@
        while (curr != head) {
                pb = list_entry(curr, page_buf_t, pb_list);
 
-               /*
-                * Skip other markers.
-                */
-               if (pb->pb_flags == 0 ) { 
-                       curr = curr->next;
-                       continue;
-               }
 
                PB_TRACE(pb, PB_TRACE_REC(walkq2), pagebuf_ispin(pb));
 
-               if (pb->pb_target == target) {
-                       if (pb->pb_flags & PBF_DELWRI) {
+                         
+               if ((pb->pb_flags == 0) || (pb->pb_target != target) ||
+                       !(pb->pb_flags & PBF_DELWRI)) {
+                 curr = curr->next;
+                 continue;
+               }
 
-                         if ((flags & PBDF_PINCOUNT) && pagebuf_ispin(pb)){
-                               /* just want a count of the number of pinned 
buffer */
-                               (*pincount)++;
-                               curr = curr->next;
-                               continue;
-                         }
-
-                               pb->pb_flags &= ~PBF_DELWRI;
-                               pb->pb_flags |= PBF_WRITE;
-                               if (flags & PBDF_WAIT)
-                                       pb->pb_flags &= ~PBF_ASYNC;
-
-                               /* insert a place holder */
-                               list_add(&pb_marker_ptr->pb_list, curr);
-
-                               spin_unlock_irqrestore(
-                                               &pb_daemon->pb_delwrite_lock,
-                                               save);
-
-                               if (pb->pb_flags & _PBF_LOCKABLE){
-                                 if (flags & PBDF_PINCOUNT){
-                                       /* if we are doing this pin count thing 
and we can't get the lock just skip it for now */
-                                       }if (!pagebuf_cond_lock(pb)){
-                                         __pagebuf_iorequest(pb);
-                                       } else {
-                                         (*pincount)++;
-                                         /* make it async again... don't want 
to wait on in below */
-                                         pb->pb_flags |= PBF_ASYNC;
-                                       }
-                                 }else {
-                                       pagebuf_lock(pb);
-                                       __pagebuf_iorequest(pb);
-                                 }
-
-                               spin_lock_irqsave(
-                                               &pb_daemon->pb_delwrite_lock,
-                                               save);
-                               /*
-                                * ok got the lock back; pick up the place
-                                * holder and continue on
-                                */
-                               curr = pb_marker_ptr->pb_list.next;
-                               list_del(&pb_marker_ptr->pb_list);
-                       } else {
-                               /* not doing anything with current...
-                                * move on to the next one */
-                               curr = curr->next;
+               if (!pagebuf_ispin(pb)){
+                 /* insert a place holder */
+                 list_add(&pb_marker_ptr->pb_list, curr);
+                 
+                 spin_unlock_irqrestore(&pb_daemon->pb_delwrite_lock, save);
+                 
+                 /* meta data pagebufs better be lockable pagebuf's */
+                 assert(pb->pb_flags & _PBF_LOCKABLE); 
+                 
+                 if ((flags & (PBDF_WAIT | PBDF_PINCOUNT)) == PBDF_WAIT)
+                       lr = pagebuf_lock(pb);
+                 else if (flags &  PBDF_PINCOUNT)
+                       lr = pagebuf_cond_lock(pb);
+                 
+                 if (lr == 0){ /* got the lock */
+                       /* check the pin count one more time just make sure */
+                       if (pagebuf_ispin(pb)){
+                         /* opps somebody bumped the pin count since the last 
time we checked it */
+                         pagebuf_unlock(pb);
+                         (*pincount)++;
+                         goto out;
                        }
-               } else {
-                       /* not doing anything with current...
-                        * move on to the next one */
-                       curr = curr->next;
+                         pb->pb_flags &= ~PBF_DELWRI;
+                         pb->pb_flags |= PBF_WRITE;
+                         if (flags & PBDF_WAIT)
+                               pb->pb_flags &= ~PBF_ASYNC;
+                         __pagebuf_iorequest(pb);
+                       } else  /* didn't get the lock just bump pin count 
return val*/
+                         (*pincount)++;
+       out:
+                 spin_lock_irqsave(&pb_daemon->pb_delwrite_lock, save);
+                 /*
+                  * ok got the list lock back; pick up the place
+                  * holder and continue on
+                  */
+                 curr = pb_marker_ptr->pb_list.next;
+                 list_del(&pb_marker_ptr->pb_list);
+               } else { /* still pinned */
+                 (*pincount)++;
+                 curr = curr->next;
                }
-       }
+       } /* while */
 
        spin_unlock_irqrestore(&pb_daemon->pb_delwrite_lock, save);
 
@@ -2238,63 +2224,45 @@
         * order to synchronize with it.
         */
 
-
        /* Now do that again, just waiting for the lock */
        spin_lock_irqsave(&pb_daemon->pb_delwrite_lock, flags);
 
        head = curr = &pb_daemon->pb_delwrite_l;
        curr = curr->next;
 
-
        while (curr != head) {
 
                pb = list_entry(curr, page_buf_t, pb_list);
 
                /*
                 * Skip other markers.
-                */
-               if (pb->pb_flags == 0 ) { 
-                       curr = curr->next;
-                       continue;
-               }
-
-               /*
                 * Skip pagebuf's that may have been added
                 * after the previous loop initiated I/O.
                 */
-               if (pb->pb_flags & PBF_DELWRI) {
+               if ((pb->pb_flags == 0) ||(pb->pb_flags & PBF_DELWRI)
+                       || !(pb->pb_flags & PBF_ASYNC)
+                       || !(pb->pb_target == target)) { 
                        curr = curr->next;
                        continue;
                }
 
                PB_TRACE(pb, PB_TRACE_REC(walkq3), pagebuf_ispin(pb));
 
-               if (pb->pb_target == target) {
-                       int     sync = (pb->pb_flags & PBF_ASYNC) == 0;
-                       list_add(&pb_marker_ptr->pb_list, curr);
-
-                       spin_unlock_irqrestore(
-                                       &pb_daemon->pb_delwrite_lock,
-                                       flags);
-
-                       if (sync) {
-                               pagebuf_iowait(pb);
-                               pb->pb_flags |= PBF_ASYNC;
-                               pagebuf_delwri_dequeue(pb);
-                               if (!pb->pb_relse)
-                                       pagebuf_unlock(pb);
-                               pagebuf_rele(pb);
-                       }
+               list_add(&pb_marker_ptr->pb_list, curr);
+               
+               spin_unlock_irqrestore(&pb_daemon->pb_delwrite_lock, flags);
+
+               pagebuf_iowait(pb);
+               pb->pb_flags |= PBF_ASYNC;
+               pagebuf_delwri_dequeue(pb);
+               if (!pb->pb_relse)
+                 pagebuf_unlock(pb);
+               pagebuf_rele(pb);
+               
+               spin_lock_irqsave( &pb_daemon->pb_delwrite_lock, flags);
 
-                       spin_lock_irqsave(
-                                       &pb_daemon->pb_delwrite_lock,
-                                       flags);
-
-                       curr = pb_marker_ptr->pb_list.next;
-                       list_del(&pb_marker_ptr->pb_list);
-               } else {
-                       curr = curr->next;
-               }
+               curr = pb_marker_ptr->pb_list.next;
+               list_del(&pb_marker_ptr->pb_list);
        }
 
        spin_unlock_irqrestore(&pb_daemon->pb_delwrite_lock, flags);

===========================================================================
linux/fs/xfs/xfs_buf.h
===========================================================================

--- /usr/tmp/TmpDir.17129-0/linux/fs/xfs/xfs_buf.h_1.70 Fri Apr  6 11:51:48 2001
+++ linux/fs/xfs/xfs_buf.h      Fri Apr  6 11:15:28 2001
@@ -317,7 +317,7 @@
 #define xfs_iowait(pb)              \
            pagebuf_iowait(pb)
 
-#define xfs_binval(buftarg) /* NOT used with pagebufs... do nothing */
+#define xfs_binval(buftarg) XFS_bflush(buftarg)
 
 extern void XFS_bflush(buftarg_t);
 

===========================================================================
linux/fs/xfs/xfs_mount.c
===========================================================================

--- /usr/tmp/TmpDir.17129-0/linux/fs/xfs/xfs_mount.c_1.249      Fri Apr  6 
11:51:48 2001
+++ linux/fs/xfs/xfs_mount.c    Fri Apr  6 11:20:20 2001
@@ -29,7 +29,7 @@
  * 
  * http://oss.sgi.com/projects/GenInfo/SGIGPLNoticeExplan/
  */
-
+#define _PAGE_BUF_INTERNAL_
 #include <xfs.h>
 
 
@@ -1075,12 +1075,13 @@
        xfs_buf_t       *sbp;
        xfs_sb_t        *sb;
        int             error = 0;
-
+       page_buf_private_t *pb;
        /*
         * skip superblock write if fs is read-only, or
         * if we are doing a forced umount.
         */
        sbp = xfs_getsb(mp, 0);
+       pb = (page_buf_private_t *)sbp;
        if (!(XFS_MTOVFS(mp)->vfs_flag & VFS_RDONLY ||
                XFS_FORCED_SHUTDOWN(mp))) {
                /*
@@ -1099,8 +1100,19 @@
                XFS_BUF_UNREAD(sbp);
                XFS_BUF_WRITE(sbp);
                xfs_bwait_unpin(sbp);
+               /* the next two lines added for linux port 
+                * make sure the superblock buffer won't simply
+                * get requeued. If delwri or async is set iowait will never see
+                * the iodone up(pb_iodonesema)
+                */
+               XFS_BUF_UNDELAYWRITE(sbp);
+               XFS_BUF_UNASYNC(sbp);
                ASSERT(XFS_BUF_TARGET(sbp) == mp->m_dev);
+               printk ("xfs_unmountfs_writesb pre  strat flags 0x%x  
pb_iodonesema cnt(%d,%d)sl\n",
+                               sbp->pb_flags,pb->pb_iodonesema.count.counter, 
pb->pb_iodonesema.sleepers);
                xfsbdstrat(mp, sbp);
+               printk ("xfs_unmountfs_writesb prot strat flags 0x%x  
pb_iodonesema cnt(%d,%d)sl\n",
+                               sbp->pb_flags,pb->pb_iodonesema.count.counter, 
pb->pb_iodonesema.sleepers);
                /* Nevermind errors we might get here. */
                error = xfs_iowait(sbp);
                if (error && mp->m_mk_sharedro)
<Prev in Thread] Current Thread [Next in Thread>