xfs
[Top] [All Lists]

Re: shutdown umount hangs

To: Utz Lehmann <leh@xxxxxxxxxx>, @thebarn.com, linux-xfs@xxxxxxxxxxx
Subject: Re: shutdown umount hangs
From: Russell Cattelan <cattelan@xxxxxxxxxxx>
Date: Thu, 05 Apr 2001 14:17:11 -0400
References: <leh@xxxxxxxxxx> <200104051412.f35ECMU25857@xxxxxxxxxxxxxxxxxxxx> <20010405172344.A1151@xxxxxxxxxx> <3ACC9D13.33DA7299@xxxxxxxxxxx>
Sender: owner-linux-xfs@xxxxxxxxxxx
Ok one more shot at it.

I'm not convinced this is entirely correct, but is closer to the irix semantics

notable exception; the pagebuf lock is not grabbed before checking the pin 
count.

I'm trying to think of a scenario where is this matters, it is much cheaper to

just check the count rather than grabbing the lock checking the count and then 
dropping the

lock.

If this still hangs please send the bt and the output of kdb's pb <addr>.

I've tested doing busy umounts:

mount /xfs2; cd /xfs2/dbench/; ./dbench 10 ; cd / ; umount /xfs2

seems to work?!

I'm going toss a lvm volume on top of this and see if that makes a difference.

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


--- /usr/tmp/TmpDir.25365-0/linux/fs/pagebuf/page_buf.c_1.72    Thu Apr  5 
13:08:24 2001
+++ linux/fs/pagebuf/page_buf.c Thu Apr  5 12:45:05 2001
@@ -2136,6 +2136,7 @@
        struct list_head *head, *curr;
        unsigned long save;
        pagebuf_marker_t *pb_marker_ptr;
+       int lr;
 
 
        pb_marker_ptr = kmalloc(sizeof(pagebuf_marker_t), GFP_KERNEL);
@@ -2164,7 +2165,8 @@
 
                if (pb->pb_target == target) {
                        if (pb->pb_flags & PBF_DELWRI) {
-
+                         
+                         /* This would be handled correctly below */
                          if ((flags & PBDF_PINCOUNT) && pagebuf_ispin(pb)){
                                /* just want a count of the number of pinned 
buffer */
                                (*pincount)++;
@@ -2172,11 +2174,6 @@
                                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);
 
@@ -2184,40 +2181,43 @@
                                                &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)){
+                                /* if (pb->pb_flags & _PBF_LOCKABLE){ */
+                               /* meta data pagebufs better be lockable 
pagebuf's */
+                                 assert(pb->pb_flags & _PBF_LOCKABLE); 
+       
+                                 if (!pagebuf_ispin(pb)){
+
+                                       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 */
+                                         pb->pb_flags &= ~PBF_DELWRI;
+                                         pb->pb_flags |= PBF_WRITE;
+                                         if (flags & PBDF_WAIT)
+                                               pb->pb_flags &= ~PBF_ASYNC;
                                          __pagebuf_iorequest(pb);
-                                       } else {
+                                       } else  /* didn't get the lock just 
bump pin count return val*/
                                          (*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);
+                                 } else { /* still pinned */
+                                       if (flags & PBDF_PINCOUNT) 
+                                         (*pincount)++;
+                                 }
+                               spin_lock_irqsave(&pb_daemon->pb_delwrite_lock, 
save);
                                /*
-                                * ok got the lock back; pick up the place
+                                * 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 {
-                               /* not doing anything with current...
-                                * move on to the next one */
-                               curr = curr->next;
-                       }
-               } else {
-                       /* not doing anything with current...
-                        * move on to the next one */
-                       curr = curr->next;
-               }
+                               continue;
+                       } 
+               } 
+               /* not doing anything with current...
+                * move on to the next one */
+               curr = curr->next;
        }
 
        spin_unlock_irqrestore(&pb_daemon->pb_delwrite_lock, save);
@@ -2270,14 +2270,13 @@
                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) {
+                       if (!(pb->pb_flags & PBF_ASYNC)) {
                                pagebuf_iowait(pb);
                                pb->pb_flags |= PBF_ASYNC;
                                pagebuf_delwri_dequeue(pb);
<Prev in Thread] Current Thread [Next in Thread>