On Mon, May 7, 2012 at 5:11 PM, Christoph Hellwig <hch at infradead.org> wrote: > There is no need to grab a local copy of the vnode information in the object > cache, as the callers can pass it. Unfortunately we also need the local copy > hack for the async flush. It seems ugly to pass a vnode_info pointer, if we need to access other variables in the future, we should add another pointer? If we don't want to use RCU recently, passing a void* pointer to those functions will be more scalable. And I suggests that this void* pointer can point to *sys*, or a new struct, looks like: struct user_data { struct vnode_info *vnode_info; /* add other data in the future */ }; > > Signed-off-by: Christoph Hellwig <hch at lst.de> > --- > sheep/object_cache.c | 29 +++++++++++++++-------------- > sheep/ops.c | 8 ++++++-- > sheep/sheep_priv.h | 8 +++++--- > sheep/store.c | 9 +++++---- > 4 files changed, 31 insertions(+), 23 deletions(-) > > Index: sheepdog/sheep/object_cache.c > =================================================================== > --- sheepdog.orig/sheep/object_cache.c 2012-05-07 10:54:32.093461458 +0200 > +++ sheepdog/sheep/object_cache.c 2012-05-07 10:58:00.177466788 +0200 > @@ -376,14 +376,14 @@ out: > } > > /* Fetch the object, cache it in success */ > -int object_cache_pull(struct object_cache *oc, uint32_t idx) > +int object_cache_pull(struct vnode_info *vnode_info, struct object_cache *oc, > + uint32_t idx) > { > int i, fd, ret = SD_RES_NO_MEM; > unsigned wlen = 0, rlen, data_length, read_len; > uint64_t oid; > struct sd_obj_req hdr = { 0 }; > struct sd_obj_rsp *rsp = (struct sd_obj_rsp *)&hdr; > - struct vnode_info *vnodes = get_vnode_info(); > struct sd_vnode *v; > void *buf; > int nr_copies; > @@ -403,9 +403,9 @@ int object_cache_pull(struct object_cach > } > > /* Check if we can read locally */ > - nr_copies = get_nr_copies(vnodes); > + nr_copies = get_nr_copies(vnode_info); > for (i = 0; i < nr_copies; i++) { > - v = oid_to_vnode(vnodes, oid, i); > + v = oid_to_vnode(vnode_info, oid, i); > if (vnode_is_local(v)) { > struct siocb iocb = { 0 }; > iocb.epoch = sys->epoch; > @@ -430,7 +430,7 @@ int object_cache_pull(struct object_cach > pull_remote: > /* Okay, no luck, let's read remotely */ > for (i = 0; i < nr_copies; i++) { > - v = oid_to_vnode(vnodes, oid, i); > + v = oid_to_vnode(vnode_info, oid, i); > > if (vnode_is_local(v)) > continue; > @@ -464,7 +464,6 @@ out: > if (ret == SD_RES_SUCCESS) > ret = create_cache_object(oc, idx, buf, read_len); > free(buf); > - put_vnode_info(vnodes); > return ret; > } > > @@ -476,7 +475,8 @@ static uint64_t idx_to_oid(uint32_t vid, > return vid_to_data_oid(vid, idx); > } > > -static int push_cache_object(uint32_t vid, uint32_t idx, int create) > +static int push_cache_object(struct vnode_info *vnode_info, uint32_t vid, > + uint32_t idx, int create) > { > struct request fake_req; > struct sd_obj_req *hdr = (struct sd_obj_req *)&fake_req.rq; > @@ -512,21 +512,19 @@ static int push_cache_object(uint32_t vi > hdr->epoch = sys->epoch; > fake_req.data = buf; > fake_req.op = get_sd_op(hdr->opcode); > - fake_req.vnodes = get_vnode_info(); > + fake_req.vnodes = vnode_info; > > ret = forward_write_obj_req(&fake_req); > if (ret != SD_RES_SUCCESS) > eprintf("failed to push object %x\n", ret); > > - put_vnode_info(fake_req.vnodes); > - > out: > free(buf); > return ret; > } > > /* Push back all the dirty objects to sheep cluster storage */ > -int object_cache_push(struct object_cache *oc) > +int object_cache_push(struct vnode_info *vnode_info, struct object_cache *oc) > { > struct object_cache_entry *entry, *t; > struct rb_root *inactive_dirty_tree; > @@ -546,7 +544,8 @@ int object_cache_push(struct object_cach > * request is issued in one of gateway worker threads > * So we need not to protect inactive dirty tree and list */ > list_for_each_entry_safe(entry, t, inactive_dirty_list, list) { > - ret = push_cache_object(oc->vid, entry->idx, entry->create); > + ret = push_cache_object(vnode_info, oc->vid, entry->idx, > + entry->create); > if (ret != SD_RES_SUCCESS) > goto push_failed; > del_from_dirty_tree_and_list(entry, inactive_dirty_tree); > @@ -608,7 +607,8 @@ void object_cache_delete(uint32_t vid) > > } > > -int object_cache_flush_and_delete(struct object_cache *oc) > +int object_cache_flush_and_delete(struct vnode_info *vnode_info, > + struct object_cache *oc) > { > DIR *dir; > struct dirent *d; > @@ -635,7 +635,8 @@ int object_cache_flush_and_delete(struct > idx = strtoul(d->d_name, NULL, 16); > if (idx == ULLONG_MAX) > continue; > - if (push_cache_object(vid, idx, 1) != SD_RES_SUCCESS) { > + if (push_cache_object(vnode_info, vid, idx, 1) != > + SD_RES_SUCCESS) { > dprintf("failed to push %"PRIx64"\n", > idx_to_oid(vid, idx)); > ret = -1; > Index: sheepdog/sheep/ops.c > =================================================================== > --- sheepdog.orig/sheep/ops.c 2012-05-07 10:56:03.857463808 +0200 > +++ sheepdog/sheep/ops.c 2012-05-07 10:58:00.177466788 +0200 > @@ -63,6 +63,7 @@ struct sd_op_template { > > struct flush_work { > struct object_cache *cache; > + struct vnode_info vnode_info; > struct work work; > }; > > @@ -581,7 +582,7 @@ static void flush_vdi_fn(struct work *wo > struct flush_work *fw = container_of(work, struct flush_work, work); > > dprintf("flush vdi %"PRIx32"\n", fw->cache->vid); > - if (object_cache_push(fw->cache) != SD_RES_SUCCESS) > + if (object_cache_push(&fw->vnode_info, fw->cache) != SD_RES_SUCCESS) > eprintf("failed to flush vdi %"PRIx32"\n", fw->cache->vid); > } > > @@ -601,12 +602,15 @@ static int local_flush_vdi(struct reques > > if (cache) { > if (!sys->async_flush) > - return object_cache_push(cache); > + return object_cache_push(req->vnodes, cache); > else { > struct flush_work *fw = xmalloc(sizeof(*fw)); > + > fw->work.fn = flush_vdi_fn; > fw->work.done = flush_vdi_done; > fw->cache = cache; > + memcpy(&fw->vnode_info, req->vnodes, > + sizeof(fw->vnode_info)); > queue_work(sys->flush_wqueue, &fw->work); > } > } > Index: sheepdog/sheep/sheep_priv.h > =================================================================== > --- sheepdog.orig/sheep/sheep_priv.h 2012-05-07 10:56:03.857463808 +0200 > +++ sheepdog/sheep/sheep_priv.h 2012-05-07 10:58:00.181466786 +0200 > @@ -456,11 +456,13 @@ struct object_cache_entry { > struct object_cache *find_object_cache(uint32_t vid, int create); > int object_cache_lookup(struct object_cache *oc, uint32_t index, int create); > int object_cache_rw(struct object_cache *oc, uint32_t idx, struct request *); > -int object_cache_pull(struct object_cache *oc, uint32_t index); > -int object_cache_push(struct object_cache *oc); > +int object_cache_pull(struct vnode_info *vnode_info, struct object_cache *oc, > + uint32_t index); > +int object_cache_push(struct vnode_info *vnode_info, struct object_cache *oc); > int object_cache_init(const char *p); > int object_is_cached(uint64_t oid); > void object_cache_delete(uint32_t vid); > -int object_cache_flush_and_delete(struct object_cache *oc); > +int object_cache_flush_and_delete(struct vnode_info *vnode_info, > + struct object_cache *oc); > > #endif > Index: sheepdog/sheep/store.c > =================================================================== > --- sheepdog.orig/sheep/store.c 2012-05-07 10:54:39.669461654 +0200 > +++ sheepdog/sheep/store.c 2012-05-07 10:58:00.181466786 +0200 > @@ -363,15 +363,16 @@ static int handle_gateway_request(struct > create = 1; > > if (object_cache_lookup(cache, idx, create) < 0) { > - ret = object_cache_pull(cache, idx); > + ret = object_cache_pull(req->vnodes, cache, idx); > if (ret != SD_RES_SUCCESS) > return ret; > } > return object_cache_rw(cache, idx, req); > } > > -static int bypass_object_cache(struct sd_obj_req *hdr) > +static int bypass_object_cache(struct request *req) > { > + struct sd_obj_req *hdr = (struct sd_obj_req *)&req->rq; > uint64_t oid = hdr->oid; > > if (!(hdr->flags & SD_FLAG_CMD_CACHE)) { > @@ -382,7 +383,7 @@ static int bypass_object_cache(struct sd > if (!cache) > return 1; > if (hdr->flags & SD_FLAG_CMD_WRITE) { > - object_cache_flush_and_delete(cache); > + object_cache_flush_and_delete(req->vnodes, cache); > return 1; > } else { > /* For read requet, we can read cache if any */ > @@ -424,7 +425,7 @@ void do_io_request(struct work *work) > if (hdr->flags & SD_FLAG_CMD_IO_LOCAL) { > ret = do_local_io(req, epoch); > } else { > - if (bypass_object_cache(hdr)) { > + if (bypass_object_cache(req)) { > /* fix object consistency when we read the object for the first time */ > if (req->check_consistency) { > ret = fix_object_consistency(req); > > -- > sheepdog mailing list > sheepdog at lists.wpkg.org > http://lists.wpkg.org/mailman/listinfo/sheepdog -- Yunkai Zhang Work at Taobao |