<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">2013/12/12 Liu Yuan <span dir="ltr"><<a href="mailto:namei.unix@gmail.com" target="_blank">namei.unix@gmail.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Thu, Dec 12, 2013 at 02:26:19PM +0800, Robin Dong wrote:<br>
> 2013/12/12 Liu Yuan <<a href="mailto:namei.unix@gmail.com">namei.unix@gmail.com</a>><br>
><br>
> > On Wed, Dec 11, 2013 at 06:14:43PM +0800, Robin Dong wrote:<br>
> > > From: Robin Dong <<a href="mailto:sanbai@taobao.com">sanbai@taobao.com</a>><br>
> > ><br>
> > > Add locks to protect containers and objects when users create/delete<br>
> > > containers or create/delete objects in the same time.<br>
> > ><br>
> > > Signed-off-by: Robin Dong <<a href="mailto:sanbai@taobao.com">sanbai@taobao.com</a>><br>
> > > ---<br>
> > > sheep/http/kv.c | 252<br>
> > ++++++++++++++++++++++++++++++++---------------------<br>
> > > sheep/http/swift.c | 10 ++-<br>
> > > 2 files changed, 162 insertions(+), 100 deletions(-)<br>
> > ><br>
> > > diff --git a/sheep/http/kv.c b/sheep/http/kv.c<br>
> > > index fb66dfa..4d690cb 100644<br>
> > > --- a/sheep/http/kv.c<br>
> > > +++ b/sheep/http/kv.c<br>
> > > @@ -218,22 +218,22 @@ static int kv_get_account(const char *account,<br>
> > uint32_t *nr_buckets)<br>
> > ><br>
> > > ret = lookup_vdi(account, &account_vid);<br>
> > > if (ret != SD_RES_SUCCESS)<br>
> > > - return ret;<br>
> > > + goto out;<br>
> > ><br>
> > > /* read account vdi out */<br>
> > > oid = vid_to_vdi_oid(account_vid);<br>
> > > ret = sd_read_object(oid, (char *)&inode, sizeof(struct sd_inode),<br>
> > 0);<br>
> > > if (ret != SD_RES_SUCCESS) {<br>
> > > sd_err("Failed to read inode header %lx", oid);<br>
> > > - return ret;<br>
> > > + goto out;<br>
> > > }<br>
> > ><br>
> > > struct list_buckets_arg arg = {NULL, NULL, NULL, 0};<br>
> > > traverse_btree(sheep_bnode_reader, &inode, list_buckets_cb, &arg);<br>
> > > if (nr_buckets)<br>
> > > *nr_buckets = arg.bucket_counter;<br>
> > > -<br>
> > > - return SD_RES_SUCCESS;<br>
> > > +out:<br>
> > > + return ret;<br>
> > > }<br>
> > ><br>
> > > int kv_read_account(const char *account, uint32_t *nr_buckets)<br>
> > > @@ -528,21 +528,22 @@ out:<br>
> > > }<br>
> > ><br>
> > > static int kv_get_bucket(struct sd_inode *account_inode, const char<br>
> > *account,<br>
> > > - const char *bucket)<br>
> > > + const char *bucket, uint32_t *account_vid)<br>
> ><br>
> > account_inode already has the account_vid, no? So we can remove<br>
> > account_vdi as<br>
> > parameter.<br>
> ><br>
><br>
> No, the account_vid will be set by lookup_vdi() below, so we can't remove<br>
> it.<br>
><br>
><br>
> ><br>
> > > {<br>
> > > char vdi_name[SD_MAX_VDI_LEN];<br>
> > > uint64_t oid;<br>
> > > - uint32_t account_vid, bucket_vid;<br>
> > > + uint32_t bucket_vid;<br>
> > > int ret;<br>
> > ><br>
> > > - ret = lookup_vdi(account, &account_vid);<br>
> > > + ret = lookup_vdi(account, account_vid);<br>
> > > if (ret != SD_RES_SUCCESS) {<br>
> > > sd_err("Failed to find account %s", account);<br>
> > > return -1;<br>
> > > }<br>
> > ><br>
> > > + sys->cdrv->lock(*account_vid);<br>
> ><br>
> > Seems that you don't unlock after exit of this function?<br>
> ><br>
><br>
> We must lock the account_vid before read the accout_inode and unlock after<br>
> create(or delete) the container,<br>
> so we call lock() in kv_get_bucket() and call unlock() in the end<br>
> ofkv_create_bucket() and kv_delete_bucket().<br>
> If not so, the buckets will corrupt under parallely high pressure tests (<br>
> <a href="https://github.com/RobinDong/misc/blob/master/sheepdog_test/pswift.sh" target="_blank">https://github.com/RobinDong/misc/blob/master/sheepdog_test/pswift.sh</a>)<br>
><br>
<br>
</div></div>If we can't move the ->lock out of kv_get_bucket(), then I'd suggest that we<br>
rename it as kv_get_lock_bucket() to explicitly inform people it locks the<br>
bucket after calling it.<br></blockquote><div><br></div><div>Nice suggestion. I will change it on next version. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks<br>
<span class="HOEnZb"><font color="#888888">Yuan<br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br>--<br>Best Regard<br>Robin Dong
</div></div>