This patch corrects two problems with the rate limiting code

that was introduced in revision 1.80. The problem manifested
itself with a `locking against myself' panic and could also
result in soft updates inconsistences associated with inodedeps.
The two problems are:

1) One of the background operations could manipulate the bitmap
while holding it locked with intent to create. This held lock
results in a `locking against myself' panic, when the background
processing that we have been coopted to do tries to lock the bitmap
which we are already holding locked. To understand how to fix this
problem, first, observe that we can do the background cleanups in
inodedep_lookup only when allocating inodedeps (DEPALLOC is set in
the call to inodedep_lookup). Second observe that calls to
inodedep_lookup with DEPALLOC set can only happen from the following
calls into the softdep code:

        softdep_setup_inomapdep
        softdep_setup_allocdirect
        softdep_setup_remove
        softdep_setup_freeblocks
        softdep_setup_directory_change
        softdep_setup_directory_add
        softdep_change_linkcnt

Only the first two of these can come from ffs_alloc.c while holding
a bitmap locked. Thus, inodedep_lookup must not go off to do
request_cleanups when being called from these functions. This change
adds a flag, NODELAY, that can be passed to inodedep_lookup to let
it know that it should not do background processing in those cases.

2) The return value from request_cleanup when helping out with the
cleanup was 0 instead of 1. This meant that despite the fact that
we may have slept while doing the cleanups, the code did not recheck
for the appearance of an inodedep (e.g., goto top in inodedep_lookup).
This lead to the softdep inconsistency in which we ended up with
two inodedep's for the same inode.

Reviewed by:	Peter Wemm <peter@yahoo-inc.com>,
		Matt Dillon <dillon@earth.backplane.com>
This commit is contained in:
mckusick 2001-02-20 11:14:38 +00:00
parent 6d1e67599e
commit d6b473bae1

View File

@ -825,6 +825,7 @@ softdep_flushfiles(oldmnt, flags, p)
* an existing entry is not found.
*/
#define DEPALLOC 0x0001 /* allocate structure if lookup fails */
#define NODELAY 0x0002 /* cannot do background work */
/*
* Structures and routines associated with pagedep caching.
@ -943,7 +944,7 @@ inodedep_lookup(fs, inum, flags, inodedeppp)
/*
* If we are over our limit, try to improve the situation.
*/
if (num_inodedep > max_softdeps && firsttry &&
if (num_inodedep > max_softdeps && firsttry && (flags & NODELAY) == 0 &&
request_cleanup(FLUSH_INODES, 1)) {
firsttry = 0;
goto top;
@ -1145,7 +1146,7 @@ softdep_setup_inomapdep(bp, ip, newinum)
* the cylinder group map from which it was allocated.
*/
ACQUIRE_LOCK(&lk);
if (inodedep_lookup(ip->i_fs, newinum, DEPALLOC, &inodedep) != 0)
if ((inodedep_lookup(ip->i_fs, newinum, DEPALLOC | NODELAY, &inodedep)))
panic("softdep_setup_inomapdep: found inode");
inodedep->id_buf = bp;
inodedep->id_state &= ~DEPCOMPLETE;
@ -1279,7 +1280,7 @@ softdep_setup_allocdirect(ip, lbn, newblkno, oldblkno, newsize, oldsize, bp)
panic("softdep_setup_allocdirect: lost block");
ACQUIRE_LOCK(&lk);
(void) inodedep_lookup(ip->i_fs, ip->i_number, DEPALLOC, &inodedep);
inodedep_lookup(ip->i_fs, ip->i_number, DEPALLOC | NODELAY, &inodedep);
adp->ad_inodedep = inodedep;
if (newblk->nb_state == DEPCOMPLETE) {
@ -4439,10 +4440,14 @@ request_cleanup(resource, islocked)
* inode as that could lead to deadlock.
*/
if (num_on_worklist > max_softdeps / 10) {
if (islocked)
FREE_LOCK(&lk);
process_worklist_item(NULL, LK_NOWAIT);
process_worklist_item(NULL, LK_NOWAIT);
stat_worklist_push += 2;
return(0);
if (islocked)
ACQUIRE_LOCK(&lk);
return(1);
}
/*
* Next, we attempt to speed up the syncer process. If that