diff mbox

[RFC,v2,08/22] block/pcache: implement pickup parts of the cache

Message ID 20160829171021.4902-9-pbutsykin@virtuozzo.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Butsykin Aug. 29, 2016, 5:10 p.m. UTC
Implementation of obtaining fragments of the cache belonging to one area
of request. This will allow to handle the case when a request is partially
hits the cache.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
---
 block/pcache.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

Comments

Kevin Wolf Sept. 2, 2016, 8:59 a.m. UTC | #1
Am 29.08.2016 um 19:10 hat Pavel Butsykin geschrieben:
> Implementation of obtaining fragments of the cache belonging to one area
> of request. This will allow to handle the case when a request is partially
> hits the cache.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>

> +static void pcache_pickup_parts_of_cache(PrefCacheAIOCB *acb, PCNode *node,
> +                                         uint64_t num, uint32_t size)
> +{
> +    uint32_t up_size;
> +
> +    do {
> +        if (num < node->cm.sector_num) {
> +            PCNode *new_node;
> +            RbNodeKey lc_key = {
> +                .num = num,
> +                .size = node->cm.sector_num - num,
> +            };
> +            up_size = lc_key.size;
> +
> +            if (!pcache_node_find_and_create(acb, &lc_key, &new_node)) {
> +                node = new_node;
> +                continue;
> +            }

We're creating additional nodes here; and we need them because they have
their own status. But once the read has completed, wouldn't it make
sense to merge all adjacent nodes in NODE_SUCCESS_STATUS?

Kevin
Pavel Butsykin Sept. 8, 2016, 12:29 p.m. UTC | #2
On 02.09.2016 11:59, Kevin Wolf wrote:
> Am 29.08.2016 um 19:10 hat Pavel Butsykin geschrieben:
>> Implementation of obtaining fragments of the cache belonging to one area
>> of request. This will allow to handle the case when a request is partially
>> hits the cache.
>>
>> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
>
>> +static void pcache_pickup_parts_of_cache(PrefCacheAIOCB *acb, PCNode *node,
>> +                                         uint64_t num, uint32_t size)
>> +{
>> +    uint32_t up_size;
>> +
>> +    do {
>> +        if (num < node->cm.sector_num) {
>> +            PCNode *new_node;
>> +            RbNodeKey lc_key = {
>> +                .num = num,
>> +                .size = node->cm.sector_num - num,
>> +            };
>> +            up_size = lc_key.size;
>> +
>> +            if (!pcache_node_find_and_create(acb, &lc_key, &new_node)) {
>> +                node = new_node;
>> +                continue;
>> +            }
>
> We're creating additional nodes here; and we need them because they have
> their own status. But once the read has completed, wouldn't it make
> sense to merge all adjacent nodes in NODE_SUCCESS_STATUS?

This can be done, but first we need to think, worth it or not. If we
merge nodes, the tree will have less nodes, the search will be faster,
that's good. But we will have to re-allocate memory, as well as the
merging could lead to the formation of larger nodes, and it's not very
good for the displacement, because it increases the chance of
displacement of unread areas of the cache memory.

I think we do not need to cache missing memory, because it is contrary
to the read-ahead policy. (read the area of the disk once will not be
read again) Also, considering the 22nd patch(
[PATCH RFC v2 22/22] block/pcache: drop used pcache node), it just
makes no sense to move the missing pieces of data in the cache memory.

> Kevin
>
diff mbox

Patch

diff --git a/block/pcache.c b/block/pcache.c
index 7504db8..28bd056 100644
--- a/block/pcache.c
+++ b/block/pcache.c
@@ -143,6 +143,24 @@  static int pcache_key_cmp(const RbNodeKey *key1, const RbNodeKey *key2)
     return 0;
 }
 
+static BlockNode *pcache_node_prev(BlockNode* node, RbNodeKey *key)
+{
+    while (node) {
+        struct RbNode *prev_rb_node = rb_prev(&node->rb_node);
+        BlockNode *prev_node;
+        if (prev_rb_node == NULL) {
+            break;
+        }
+        prev_node = container_of(prev_rb_node, BlockNode, rb_node);
+        if (prev_node->sector_num + prev_node->nb_sectors <= key->num) {
+            break;
+        }
+        node = prev_node;
+    }
+
+    return node;
+}
+
 static void *node_insert(struct RbRoot *root, BlockNode *node)
 {
     struct RbNode **new = &(root->rb_node), *parent = NULL;
@@ -152,7 +170,7 @@  static void *node_insert(struct RbRoot *root, BlockNode *node)
         BlockNode *this = container_of(*new, BlockNode, rb_node);
         int result = pcache_key_cmp(&node->key, &this->key);
         if (result == 0) {
-            return this;
+            return pcache_node_prev(this, &node->key);
         }
         parent = *new;
         new = result < 0 ? &((*new)->rb_left) : &((*new)->rb_right);
@@ -258,6 +276,45 @@  static inline void prefetch_init_key(PrefCacheAIOCB *acb, RbNodeKey* key)
     key->size = acb->nb_sectors;
 }
 
+static void pcache_pickup_parts_of_cache(PrefCacheAIOCB *acb, PCNode *node,
+                                         uint64_t num, uint32_t size)
+{
+    uint32_t up_size;
+
+    do {
+        if (num < node->cm.sector_num) {
+            PCNode *new_node;
+            RbNodeKey lc_key = {
+                .num = num,
+                .size = node->cm.sector_num - num,
+            };
+            up_size = lc_key.size;
+
+            if (!pcache_node_find_and_create(acb, &lc_key, &new_node)) {
+                node = new_node;
+                continue;
+            }
+            size -= up_size;
+            num += up_size;
+        }
+        /* XXX: node read */
+        up_size = MIN(node->cm.sector_num + node->cm.nb_sectors - num, size);
+
+        size -= up_size;
+        num += up_size;
+        if (size != 0) {
+            RbNodeKey lc_key = {
+                .num = num,
+                .size = size,
+            };
+            if (pcache_node_find_and_create(acb, &lc_key, &node)) {
+                size -= lc_key.size;
+                assert(size == 0);
+            }
+        }
+    } while (size);
+}
+
 enum {
     PREFETCH_NEW_NODE  = 0,
     PREFETCH_FULL_UP   = 1,
@@ -281,6 +338,7 @@  static int32_t pcache_prefetch(PrefCacheAIOCB *acb)
     {
         return PREFETCH_FULL_UP;
     }
+    pcache_pickup_parts_of_cache(acb, node, key.num, key.size);
 
     return PREFETCH_PART_UP;
 }