Block requests come with a completion method
authorBarret Rhoden <brho@cs.berkeley.edu>
Wed, 13 Oct 2010 20:07:35 +0000 (13:07 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:55 +0000 (17:35 -0700)
breq_submit_request() doesn't require you sleep on the breq, but since
it is common (to both callers!) we provide helpers for it.  This also
splits up the usage of lock_page() to both block on IO as well as block
on waiting for a page to be ready (blocking on someone else's IO).

kern/include/blockdev.h
kern/src/blockdev.c
kern/src/ext2fs.c
kern/src/page_alloc.c
kern/src/pagemap.c

index 7e63ed9..74a1ad2 100644 (file)
@@ -74,8 +74,11 @@ void bdev_put_buffer(struct buffer_head *bh);
  * another array of BH pointers if you want more.  The BHs do not need to be
  * linked or otherwise associated with a page mapping. */
 #define NR_INLINE_BH (PGSIZE >> SECTOR_SZ_LOG)
+struct block_request;
 struct block_request {
        unsigned int                            flags;
+       void                                            (*callback)(struct block_request *breq);
+       void                                            *data;
        struct buffer_head                      **bhs;                          /* BHs describing the IOs */
        unsigned int                            nr_bhs;
        struct buffer_head                      *local_bhs[NR_INLINE_BH];
@@ -89,7 +92,8 @@ struct kmem_cache *breq_kcache;       /* for the block requests */
 void block_init(void);
 struct block_device *get_bdev(char *path);
 void free_bhs(struct page *page);
-/* This function will probably be the one that blocks */
-int bdev_submit_request(struct block_device *bdev, struct block_request *req);
+int bdev_submit_request(struct block_device *bdev, struct block_request *breq);
+void generic_breq_done(struct block_request *breq);
+void sleep_on_breq(struct block_request *breq);
 
 #endif /* ROS_KERN_BLOCKDEV_H */
index 18ba977..b1d1956 100644 (file)
@@ -79,35 +79,54 @@ void free_bhs(struct page *page)
 
 /* This ultimately will handle the actual request processing, all the way down
  * to the driver, and will deal with blocking.  For now, we just fulfill the
- * request right away (RAM based block devs). */
-int bdev_submit_request(struct block_device *bdev, struct block_request *req)
+ * request right away (RAM based block devs).  Note this calls the callback
+ * before it returns.  This race is possible, so callers should be able to
+ * handle it. */
+int bdev_submit_request(struct block_device *bdev, struct block_request *breq)
 {
        void *src, *dst;
        unsigned long first_sector;
        unsigned int nr_sector;
 
-       for (int i = 0; i < req->nr_bhs; i++) {
-               first_sector = req->bhs[i]->bh_sector;
-               nr_sector = req->bhs[i]->bh_nr_sector;
+       for (int i = 0; i < breq->nr_bhs; i++) {
+               first_sector = breq->bhs[i]->bh_sector;
+               nr_sector = breq->bhs[i]->bh_nr_sector;
                /* Sectors are indexed starting with 0, for now. */
                if (first_sector + nr_sector > bdev->b_nr_sector) {
                        warn("Exceeding the num sectors!");
                        return -1;
                }
-               if (req->flags & BREQ_READ) {
-                       dst = req->bhs[i]->bh_buffer;
+               if (breq->flags & BREQ_READ) {
+                       dst = breq->bhs[i]->bh_buffer;
                        src = bdev->b_data + (first_sector << SECTOR_SZ_LOG);
-               } else if (req->flags & BREQ_WRITE) {
+               } else if (breq->flags & BREQ_WRITE) {
                        dst = bdev->b_data + (first_sector << SECTOR_SZ_LOG);
-                       src = req->bhs[i]->bh_buffer;
+                       src = breq->bhs[i]->bh_buffer;
                } else {
                        panic("Need a request type!\n");
                }
                memcpy(dst, src, nr_sector << SECTOR_SZ_LOG);
        }
+       if (breq->callback)
+               breq->callback(breq);
        return 0;
 }
 
+/* Helper method, unblocks someone blocked on sleep_on_breq(). */
+void generic_breq_done(struct block_request *breq)
+{
+       /* TODO: BLK - unblock the kthread sleeping on this request */
+       breq->data = (void*)1;
+}
+
+/* Helper, pairs with generic_breq_done() */
+void sleep_on_breq(struct block_request *breq)
+{
+       /* TODO: BLK Block til we are done: data gets toggled in the completion.
+        * This only works if the completion happened first (for now) */
+       assert(breq->data);
+}
+
 /* This just tells the page cache that it is 'up to date'.  Due to the nature of
  * the blocks in the page cache, we don't actually read the items in on
  * readpage, we read them in when a specific block is there */
@@ -199,12 +218,14 @@ found:
        breq = kmem_cache_alloc(breq_kcache, 0);
        assert(breq);
        breq->flags = BREQ_READ;
+       breq->callback = generic_breq_done;
+       breq->data = 0;
        breq->bhs = breq->local_bhs;
        breq->bhs[0] = bh;
        breq->nr_bhs = 1;
        error = bdev_submit_request(bdev, breq);
-       /* TODO: (BLK) this assumes we slept til the request was done */
        assert(!error);
+       sleep_on_breq(breq);
        kmem_cache_free(breq_kcache, breq);
        /* after the data is read, we mark it up to date and unlock the page. */
        bh->bh_flags |= BH_UPTODATE;
index 783f68e..f89eafb 100644 (file)
@@ -717,6 +717,8 @@ int ext2_readpage(struct page_map *pm, struct page *page)
                return -ENOMEM;
        }
        breq->flags = BREQ_READ;
+       breq->callback = generic_breq_done;
+       breq->data = 0;
        breq->bhs = breq->local_bhs;
        breq->nr_bhs = 0;
        /* Pack the BH pointers in the block request */
@@ -735,9 +737,10 @@ int ext2_readpage(struct page_map *pm, struct page *page)
                        bh->bh_page->pg_flags |= PG_DIRTY;
                }
        }
-       /* TODO: (BLK) this assumes we slept til the request was done */
        retval = bdev_submit_request(bdev, breq);
        assert(!retval);
+       sleep_on_breq(breq);
+       kmem_cache_free(breq_kcache, breq);
        /* zero out whatever is beyond the EOF.  we could do this by figuring out
         * where the BHs end and zeroing from there, but I'd rather zero from where
         * the file ends (which could be in the middle of an FS block */
@@ -747,10 +750,8 @@ int ext2_readpage(struct page_map *pm, struct page *page)
        /* at this point, eof_off is the offset into the page of the EOF, or 0 */
        if (eof_off)
                memset(eof_off + page2kva(page), 0, PGSIZE - eof_off);
-       /* after the data is read, we mark it up to date and unlock the page. */
+       /* Now the page is up to date */
        page->pg_flags |= PG_UPTODATE;
-       unlock_page(page);
-       kmem_cache_free(breq_kcache, breq);
        /* Useful debugging.  Put one higher up if the page is not getting mapped */
        //print_pageinfo(page);
        return 0;
index 8f614e7..dc83725 100644 (file)
@@ -300,7 +300,9 @@ void page_setref(page_t *page, size_t val)
 }
 
 /* Attempts to get a lock on the page for IO operations.  If it is already
- * locked, it will block the thread until it is unlocked. */
+ * locked, it will block the thread until it is unlocked.  Note that this is
+ * really a "sleep on some event", not necessarily the IO, but it is "the page
+ * is ready". */
 void lock_page(struct page *page)
 {
        /* TODO: (BLK) actually do something!  And this has a race!  Not a big deal
index 09a44a0..294022a 100644 (file)
@@ -137,11 +137,10 @@ int pm_load_page(struct page_map *pm, unsigned long index, struct page **pp)
        }
        /* if we're here, the page is locked by us, and it needs to be read in */
        assert(page->pg_mapping == pm);
+       /* Readpage will block internally, returning when it is done */
        error = pm->pm_op->readpage(pm, page);
        assert(!error);
-       /* Try to sleep on the IO.  The page will be unlocked when the IO is done */
-       /* TODO: (BLK) this assumes we never slept til we got here */
-       lock_page(page);
+       /* Unlock, since we're done with the page and it is up to date */
        unlock_page(page);
        assert(page->pg_flags & PG_UPTODATE);
        return 0;