tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Cleanup bread/breadn API



The functions bread() and breadn() have an ambiguous API.  They return
a buffer on error except when called from the pagedaemon.  This lead
to ugly code in some file systems where bread() had to be expanded
for operations that get called from the pagedaemon.  See PR kern/46282.

The attached diff changes bread() and breadn() to not return a buffer
in case of error.  Changing ~160 calls of bread() to not brelse() on
error is not included in the diff.

Comments or objections?

--
J. Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig 
(Germany)

Index: vfs_bio.c
===================================================================
RCS file: /cvsroot/src/sys/kern/vfs_bio.c,v
retrieving revision 1.239
diff -p -u -4 -r1.239 vfs_bio.c
--- vfs_bio.c   3 Jun 2012 16:23:44 -0000       1.239
+++ vfs_bio.c   9 Dec 2012 09:03:08 -0000
@@ -661,13 +661,13 @@ bio_doread(struct vnode *vp, daddr_t blk
        struct mount *mp;
 
        bp = getblk(vp, blkno, size, 0, 0);
 
-#ifdef DIAGNOSTIC
-       if (bp == NULL) {
-               panic("bio_doread: no such buf");
-       }
-#endif
+       /*
+        * getblk() may return NULL if we are the pagedaemon.
+        */
+       if (bp == NULL)
+               return NULL;
 
        /*
         * If buffer does not have data valid, start a read.
         * Note that if buffer is BC_INVAL, getblk() won't return it.
@@ -719,13 +719,20 @@ bread(struct vnode *vp, daddr_t blkno, i
        int error;
 
        /* Get buffer for block. */
        bp = *bpp = bio_doread(vp, blkno, size, cred, 0);
+       if (bp == NULL)
+               return ENOMEM;
 
        /* Wait for the read to complete, and return result. */
        error = biowait(bp);
-       if (error == 0 && (flags & B_MODIFY) != 0)
+       if (error == 0 && (flags & B_MODIFY) != 0) {
                error = fscow_run(bp, true);
+               if (error) {
+                       brelse(bp, 0)
+                       *bpp = NULL;
+               }
+       }
 
        return error;
 }
 
@@ -740,8 +747,10 @@ breadn(struct vnode *vp, daddr_t blkno, 
        buf_t *bp;
        int error, i;
 
        bp = *bpp = bio_doread(vp, blkno, size, cred, 0);
+       if (bp == NULL)
+               return ENOMEM;
 
        /*
         * For each of the read-ahead blocks, start a read, if necessary.
         */
@@ -759,10 +768,16 @@ breadn(struct vnode *vp, daddr_t blkno, 
        mutex_exit(&bufcache_lock);
 
        /* Otherwise, we had to start a read for it; wait until it's valid. */
        error = biowait(bp);
-       if (error == 0 && (flags & B_MODIFY) != 0)
+       if (error == 0 && (flags & B_MODIFY) != 0) {
                error = fscow_run(bp, true);
+               if (error) {
+                       brelse(bp, 0)
+                       *bpp = NULL;
+               }
+       }
+
        return error;
 }
 
 /*


Home | Main Index | Thread Index | Old Index