From owner-freebsd-arch Wed Mar 27 13:52:39 2002 Delivered-To: freebsd-arch@freebsd.org Received: from albatross.prod.itd.earthlink.net (albatross.mail.pas.earthlink.net [207.217.120.120]) by hub.freebsd.org (Postfix) with ESMTP id B110D37B43B for ; Wed, 27 Mar 2002 13:51:35 -0800 (PST) Received: from pool0142.cvx21-bradley.dialup.earthlink.net ([209.179.192.142] helo=mindspring.com) by albatross.prod.itd.earthlink.net with esmtp (Exim 3.33 #1) id 16qLKD-0004NO-00; Wed, 27 Mar 2002 13:51:22 -0800 Message-ID: <3CA23EC4.EB6E8508@mindspring.com> Date: Wed, 27 Mar 2002 13:51:00 -0800 From: Terry Lambert X-Mailer: Mozilla 4.7 [en]C-CCK-MCD {Sony} (Win98; U) X-Accept-Language: en MIME-Version: 1.0 To: Kirk McKusick Cc: Ollivier Robert , arch@FreeBSD.ORG Subject: Re: [PATCH] MFC the full filesystem softupdates fix ? References: <200203272134.g2RLYTD97258@beastie.mckusick.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG I think Kirk meant "-stable", not "-current"; in any case, I'd like to see it handled and brought back into -stable anyway. I look at -stable as a pruned -current. Some of the growth in -current should/will never see the light of day in a realease... -- Terry Kirk McKusick wrote: > > If it was easy as your patch, there would be no problem in > putting it in. The problem is that -current does not have > the code to maintain the fs_pendingblocks variable. At a > minimum you would have to add that code as well. It is a > good deal more extensive, though at this point well enough > tested that I am less fearful of adding it. > > Kirk McKusick > > =-=-=-=-=-= > > Date: Wed, 27 Mar 2002 11:19:47 +0100 > From: Ollivier Robert > To: mckusick@mckusick.com > Cc: arch@FreeBSD.ORG > Subject: [PATCH] MFC the full filesystem softupdates fix ? > > Following some discussions in the French fr.comp.os.bsd newsgroup about > softupdates and the full filesystem problem (which is fixed in CURRENT > thanks to you), here a proposed diff to merge the fix into STABLE. > > Can you (and -arch) review it ? It would be nice to have this fixed in > STABLE as well. > > Thanks. > > Index: ffs_alloc.c > =================================================================== > RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_alloc.c,v > retrieving revision 1.64.2.2 > diff -u -2 -r1.64.2.2 ffs_alloc.c > --- ffs_alloc.c 21 Sep 2001 19:15:21 -0000 1.64.2.2 > +++ ffs_alloc.c 27 Mar 2002 10:08:29 -0000 > @@ -107,5 +107,5 @@ > register struct fs *fs; > ufs_daddr_t bno; > - int cg; > + int cg, reclaimed; > #ifdef QUOTA > int error; > @@ -124,4 +124,6 @@ > panic("ffs_alloc: missing credential"); > #endif /* DIAGNOSTIC */ > + reclaimed = 0; > +retry: > if (size == fs->fs_bsize && fs->fs_cstotal.cs_nbfree == 0) > goto nospace; > @@ -155,4 +157,9 @@ > #endif > nospace: > + if (fs->fs_pendingblocks > 0 && reclaimed == 0) { > + reclaimed = 1; > + softdep_request_cleanup(fs, ITOV(ip)); > + goto retry; > + } > ffs_fserr(fs, cred->cr_uid, "file system full"); > uprintf("\n%s: write failed, file system is full\n", fs->fs_fsmnt); > @@ -177,7 +184,7 @@ > struct buf **bpp; > { > - register struct fs *fs; > + struct fs *fs; > struct buf *bp; > - int cg, request, error; > + int cg, request, error, reclaimed; > ufs_daddr_t bprev, bno; > > @@ -198,4 +205,6 @@ > if (cred->cr_uid != 0 && > freespace(fs, fs->fs_minfree) - numfrags(fs, nsize - osize) < 0) > + reclaimed = 0; > +retry: > goto nospace; > if ((bprev = ip->i_db[lbprev]) == 0) { > @@ -208,5 +217,5 @@ > * Allocate the extra space in the buffer. > */ > - error = bread(ITOV(ip), lbprev, osize, NOCRED, &bp); > + error = bread(vp, lbprev, osize, NOCRED, &bp); > if (error) { > brelse(bp); > @@ -295,5 +304,5 @@ > if (bno > 0) { > bp->b_blkno = fsbtodb(fs, bno); > - if (!DOINGSOFTDEP(ITOV(ip))) > + if (!DOINGSOFTDEP(vp)) > ffs_blkfree(ip, bprev, (long)osize); > if (nsize < request) > @@ -319,4 +328,9 @@ > * no space available > */ > + if (fs->fs_pendingblocks > 0 && reclaimed == 0) { > + reclaimed = 1; > + softdep_request_cleanup(fs, vp); > + goto retry; > + } > ffs_fserr(fs, cred->cr_uid, "file system full"); > uprintf("\n%s: write failed, file system is full\n", fs->fs_fsmnt); > Index: ffs_extern.h > =================================================================== > RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_extern.h,v > retrieving revision 1.30 > diff -u -2 -r1.30 ffs_extern.h > --- ffs_extern.h 9 Jan 2000 22:40:02 -0000 1.30 > +++ ffs_extern.h 27 Mar 2002 10:05:03 -0000 > @@ -115,4 +115,5 @@ > void softdep_load_inodeblock __P((struct inode *)); > void softdep_freefile __P((struct vnode *, ino_t, int)); > +int softdep_request_cleanup __P((struct fs *, struct vnode *)); > void softdep_setup_freeblocks __P((struct inode *, off_t)); > void softdep_setup_inomapdep __P((struct buf *, struct inode *, ino_t)); > Index: ffs_softdep.c > =================================================================== > RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_softdep.c,v > retrieving revision 1.57.2.11 > diff -u -2 -r1.57.2.11 ffs_softdep.c > --- ffs_softdep.c 5 Feb 2002 18:46:53 -0000 1.57.2.11 > +++ ffs_softdep.c 27 Mar 2002 10:12:48 -0000 > @@ -508,7 +508,8 @@ > static struct proc *filesys_syncer; /* proc of filesystem syncer process */ > static int req_clear_inodedeps; /* syncer process flush some inodedeps */ > -#define FLUSH_INODES 1 > +#define FLUSH_INODES 1 > static int req_clear_remove; /* syncer process flush some freeblks */ > -#define FLUSH_REMOVE 2 > +#define FLUSH_REMOVE 2 > +#define FLUSH_REMOVE_WAIT 3 > /* > * runtime statistics > @@ -703,5 +704,5 @@ > if (wk == 0) { > FREE_LOCK(&lk); > - return (0); > + return (-1); > } > WORKLIST_REMOVE(wk); > @@ -4561,4 +4562,39 @@ > > /* > + * Called by the allocation routines when they are about to fail > + * in the hope that we can free up some disk space. > + * > + * First check to see if the work list has anything on it. If it has, > + * clean up entries until we successfully free some space. Because this > + * process holds inodes locked, we cannot handle any remove requests > + * that might block on a locked inode as that could lead to deadlock. > + * If the worklist yields no free space, encourage the syncer daemon > + * to help us. In no event will we try for longer than tickdelay seconds. > + */ > +int > +softdep_request_cleanup(fs, vp) > + struct fs *fs; > + struct vnode *vp; > +{ > + long starttime, needed; > + > + needed = fs->fs_cstotal.cs_nbfree + fs->fs_contigsumsize; > + starttime = time_second + tickdelay; > + if (UFS_UPDATE(vp, 1) != 0) > + return (0); > + while (fs->fs_pendingblocks > 0 && fs->fs_cstotal.cs_nbfree <= needed) { > + if (time_second > starttime) > + return (0); > + if (num_on_worklist > 0 && > + process_worklist_item(NULL, LK_NOWAIT) != -1) { > + stat_worklist_push += 1; > + continue; > + } > + request_cleanup(FLUSH_REMOVE_WAIT, 0); > + } > + return (1); > +} > + > +/* > * If memory utilization has gotten too high, deliberately slow things > * down and speed up the I/O processing. > @@ -4595,4 +4631,10 @@ > > /* > + * Next, we attempt to speed up the syncer process. If that > + * is successful, then we allow the process to continue. > + */ > + if (speedup_syncer() && resource != FLUSH_REMOVE_WAIT) > + return(0); > + /* > * If we are resource constrained on inode dependencies, try > * flushing some dirty inodes. Otherwise, we are constrained > @@ -4613,4 +4655,5 @@ > > case FLUSH_REMOVE: > + case FLUSH_REMOVE_WAIT: > stat_blk_limit_push += 1; > req_clear_remove += 1; > > -- > Ollivier ROBERT -=- Eurocontrol EEC/ITM -=- Ollivier.Robert@eurocontrol.fr > FreeBSD caerdonn.eurocontrol.fr 5.0-CURRENT #46: Wed Jan 3 15:52:00 CET 2001 > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-arch" in the body of the message > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-arch" in the body of the message To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message