Avoid a race in lafs_get_cleanable.

If we find a cleanable segment that is actually clean, then we
drop the lock and try to add it.  If something else removed it at
just this time we end up with a refcount issue as we are meant to
take references to youth_db and usage0_db when adding things to the
table, and we don't have them any more.

So simply allow the 'add_clean' to fail as that is safe.

Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/segments.c b/segments.c
index 124c2121f79b6e1ea78f517ddf0ed3fc70f44ba4..ac83ca98d3facd5340253f5c6a1a7537be2e7e16 100644 (file)
--- a/segments.c
+++ b/segments.c
@@ -1366,11 +1366,12 @@ static int add_free(struct fs *fs, unsigned int dev, u32 seg, u16 *youthp)
        return 1;
 }
 
-static int add_clean(struct fs *fs, unsigned int dev, u32 seg)
+static int add_clean(struct fs *fs, unsigned int dev, u32 seg, bool if_present)
 {
        /* This dev/seg is now clean.  Make sure it is on the list.
         * Chances are that this is already present in the table as
-        * a recently cleaned segment.
+        * a recently cleaned segment.  If 'if_present', then insist
+        * on this.
         * Return TRUE if segment was added to the table (as this
         * implies a reference count).
         */
@@ -1402,6 +1403,10 @@ static int add_clean(struct fs *fs, unsigned int dev, u32 seg)
                spin_unlock(&fs->lock);
                return 0;
        }
+       if (if_present) {
+               spin_unlock(&fs->lock);
+               return 0;
+       }
 
        if (fs->segtrack->free.cnt + fs->segtrack->clean.cnt >=
            fs->segtrack->total / 2) {
@@ -1621,10 +1626,8 @@ retry:
        if (ss->usage == 0) {
                int rv;
                spin_unlock(&fs->lock);
-               rv = add_clean(fs, ss->dev, ss->segment);
-               /* FIXME we dropped the lock, so maybe this could bug?? */
-               /* I should make sure I hold the block references */
-               BUG_ON(rv);
+               rv = add_clean(fs, *dev, *seg, true);
+               BUG_ON(rv); /* passing 'true' makes this impossible */
                goto retry;
        }
        segdelete(fs->segtrack, ss);
@@ -1659,7 +1662,7 @@ static int add_cleanable(struct fs *fs, unsigned int dev, u32 seg,
        fs->total_free += segsize - usage /* - 1 */;
 
        if (usage == 0) {
-               int rv = add_clean(fs, dev, seg);
+               int rv = add_clean(fs, dev, seg, false);
                lafs_check_seg_cnt(fs->segtrack);
                return rv;
        }