* [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid
@ 2011-06-06 17:16 Aneesh Kumar K.V
2011-06-06 17:16 ` [Qemu-devel] [PATCH 2/6] hw/9pfs: Don't free fid in clunk Aneesh Kumar K.V
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Aneesh Kumar K.V @ 2011-06-06 17:16 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Aneesh Kumar K.V
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
hw/9pfs/virtio-9p.c | 205 +++++++++++++++++++++++++++++++++++----------------
hw/9pfs/virtio-9p.h | 7 ++
2 files changed, 147 insertions(+), 65 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index e2aa863..03d8664 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -232,12 +232,13 @@ static size_t v9fs_string_size(V9fsString *str)
return str->size;
}
-static V9fsFidState *lookup_fid(V9fsState *s, int32_t fid)
+static V9fsFidState *get_fid(V9fsState *s, int32_t fid)
{
V9fsFidState *f;
for (f = s->fid_list; f; f = f->next) {
if (f->fid == fid) {
+ f->ref++;
return f;
}
}
@@ -249,16 +250,16 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
{
V9fsFidState *f;
- f = lookup_fid(s, fid);
+ f = get_fid(s, fid);
if (f) {
+ f->ref--;
return NULL;
}
f = qemu_mallocz(sizeof(V9fsFidState));
-
f->fid = fid;
f->fid_type = P9_FID_NONE;
-
+ f->ref = 1;
f->next = s->fid_list;
s->fid_list = f;
@@ -1014,19 +1015,22 @@ static void v9fs_attach(void *opaque)
fidp = alloc_fid(s, fid);
if (fidp == NULL) {
err = -EINVAL;
- goto out;
+ goto out_nofid;
}
fidp->uid = n_uname;
v9fs_string_sprintf(&fidp->path, "%s", "/");
err = fid_to_qid(s, fidp, &qid);
if (err < 0) {
err = -EINVAL;
+ put_fid(fidp);
free_fid(s, fid);
- goto out;
+ goto out_nofid;
}
offset += pdu_marshal(pdu, offset, "Q", &qid);
err = offset;
-out:
+ put_fid(fidp);
+
+out_nofid:
complete_pdu(s, pdu, err);
v9fs_string_free(&uname);
v9fs_string_free(&aname);
@@ -1045,10 +1049,10 @@ static void v9fs_stat(void *opaque)
pdu_unmarshal(pdu, offset, "d", &fid);
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
err = v9fs_co_lstat(s, &fidp->path, &stbuf);
if (err < 0) {
@@ -1062,6 +1066,8 @@ static void v9fs_stat(void *opaque)
err = offset;
v9fs_stat_free(&v9stat);
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(s, pdu, err);
}
@@ -1079,10 +1085,10 @@ static void v9fs_getattr(void *opaque)
pdu_unmarshal(pdu, offset, "dq", &fid, &request_mask);
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
retval = -ENOENT;
- goto out;
+ goto out_nofid;
}
/*
* Currently we only support BASIC fields in stat, so there is no
@@ -1096,6 +1102,8 @@ static void v9fs_getattr(void *opaque)
retval = offset;
retval += pdu_marshal(pdu, offset, "A", &v9stat_dotl);
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(s, pdu, retval);
}
@@ -1123,10 +1131,10 @@ static void v9fs_setattr(void *opaque)
pdu_unmarshal(pdu, offset, "dI", &fid, &v9iattr);
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
err = -EINVAL;
- goto out;
+ goto out_nofid;
}
if (v9iattr.valid & ATTR_MODE) {
err = v9fs_co_chmod(s, &fidp->path, v9iattr.mode);
@@ -1188,6 +1196,8 @@ static void v9fs_setattr(void *opaque)
}
err = offset;
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(s, pdu, err);
}
@@ -1214,7 +1224,7 @@ static void v9fs_walk(void *opaque)
int32_t fid, newfid;
V9fsString *wnames = NULL;
V9fsFidState *fidp;
- V9fsFidState *newfidp;
+ V9fsFidState *newfidp = NULL;;
V9fsPDU *pdu = opaque;
V9fsState *s = pdu->s;
@@ -1231,12 +1241,12 @@ static void v9fs_walk(void *opaque)
} else if (nwnames > P9_MAXWELEM) {
err = -EINVAL;
- goto out;
+ goto out_nofid;
}
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
if (fid == newfid) {
BUG_ON(fidp->fid_type != P9_FID_NONE);
@@ -1269,7 +1279,9 @@ static void v9fs_walk(void *opaque)
v9fs_string_copy(&newfidp->path, &path);
err = v9fs_co_lstat(s, &newfidp->path, &stbuf);
if (err < 0) {
+ put_fid(newfidp);
free_fid(s, newfidp->fid);
+ newfidp = NULL;
v9fs_string_free(&path);
goto out;
}
@@ -1279,6 +1291,11 @@ static void v9fs_walk(void *opaque)
}
err = v9fs_walk_marshal(pdu, nwnames, qids);
out:
+ put_fid(fidp);
+ if (newfidp) {
+ put_fid(newfidp);
+ }
+out_nofid:
complete_pdu(s, pdu, err);
if (nwnames && nwnames <= P9_MAXWELEM) {
for (name_idx = 0; name_idx < nwnames; name_idx++) {
@@ -1327,10 +1344,10 @@ static void v9fs_open(void *opaque)
} else {
pdu_unmarshal(pdu, offset, "db", &fid, &mode);
}
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
BUG_ON(fidp->fid_type != P9_FID_NONE);
@@ -1366,6 +1383,8 @@ static void v9fs_open(void *opaque)
err = offset;
}
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(s, pdu, err);
}
@@ -1388,10 +1407,10 @@ static void v9fs_lcreate(void *opaque)
pdu_unmarshal(pdu, offset, "dsddd", &dfid, &name, &flags,
&mode, &gid);
- fidp = lookup_fid(pdu->s, dfid);
+ fidp = get_fid(pdu->s, dfid);
if (fidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
v9fs_string_sprintf(&fullname, "%s/%s", fidp->path.data, name.data);
@@ -1418,6 +1437,8 @@ static void v9fs_lcreate(void *opaque)
offset += pdu_marshal(pdu, offset, "Qd", &qid, iounit);
err = offset;
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(pdu->s, pdu, err);
v9fs_string_free(&name);
v9fs_string_free(&fullname);
@@ -1434,7 +1455,7 @@ static void v9fs_fsync(void *opaque)
V9fsState *s = pdu->s;
pdu_unmarshal(pdu, offset, "dd", &fid, &datasync);
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
err = -ENOENT;
goto out;
@@ -1444,6 +1465,7 @@ static void v9fs_fsync(void *opaque)
err = offset;
}
out:
+ put_fid(fidp);
complete_pdu(s, pdu, err);
}
@@ -1561,10 +1583,10 @@ static void v9fs_read(void *opaque)
pdu_unmarshal(pdu, offset, "dqd", &fid, &off, &max_count);
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
err = -EINVAL;
- goto out;
+ goto out_nofid;
}
if (fidp->fid_type == P9_FID_DIR) {
@@ -1616,6 +1638,8 @@ static void v9fs_read(void *opaque)
err = -EINVAL;
}
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(s, pdu, err);
}
@@ -1700,8 +1724,12 @@ static void v9fs_readdir(void *opaque)
pdu_unmarshal(pdu, offset, "dqd", &fid, &initial_offset, &max_count);
- fidp = lookup_fid(s, fid);
- if (fidp == NULL || !fidp->fs.dir) {
+ fidp = get_fid(s, fid);
+ if (fidp == NULL) {
+ retval = -EINVAL;
+ goto out_nofid;
+ }
+ if (!fidp->fs.dir) {
retval = -EINVAL;
goto out;
}
@@ -1719,6 +1747,8 @@ static void v9fs_readdir(void *opaque)
retval += pdu_marshal(pdu, offset, "d", count);
retval += count;
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(s, pdu, retval);
}
@@ -1784,10 +1814,10 @@ static void v9fs_write(void *opaque)
pdu_unmarshal(pdu, offset, "dqdv", &fid, &off, &count, sg, &cnt);
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
err = -EINVAL;
- goto out;
+ goto out_nofid;
}
if (fidp->fid_type == P9_FID_FILE) {
if (fidp->fs.fd == -1) {
@@ -1827,6 +1857,8 @@ static void v9fs_write(void *opaque)
offset += pdu_marshal(pdu, offset, "d", total);
err = offset;
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(s, pdu, err);
}
@@ -1851,10 +1883,10 @@ static void v9fs_create(void *opaque)
pdu_unmarshal(pdu, offset, "dsdbs", &fid, &name,
&perm, &mode, &extension);
- fidp = lookup_fid(pdu->s, fid);
+ fidp = get_fid(pdu->s, fid);
if (fidp == NULL) {
err = -EINVAL;
- goto out;
+ goto out_nofid;
}
v9fs_string_sprintf(&fullname, "%s/%s", fidp->path.data, name.data);
@@ -1884,15 +1916,17 @@ static void v9fs_create(void *opaque)
}
} else if (perm & P9_STAT_MODE_LINK) {
int32_t nfid = atoi(extension.data);
- V9fsFidState *nfidp = lookup_fid(pdu->s, nfid);
+ V9fsFidState *nfidp = get_fid(pdu->s, nfid);
if (nfidp == NULL) {
err = -EINVAL;
goto out;
}
err = v9fs_co_link(pdu->s, &nfidp->path, &fullname);
if (err < 0) {
+ put_fid(nfidp);
goto out;
}
+ put_fid(nfidp);
} else if (perm & P9_STAT_MODE_DEVICE) {
char ctype;
uint32_t major, minor;
@@ -1956,6 +1990,8 @@ static void v9fs_create(void *opaque)
err = offset;
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(pdu->s, pdu, err);
v9fs_string_free(&name);
v9fs_string_free(&extension);
@@ -1980,10 +2016,10 @@ static void v9fs_symlink(void *opaque)
pdu_unmarshal(pdu, offset, "dssd", &dfid, &name, &symname, &gid);
- dfidp = lookup_fid(pdu->s, dfid);
+ dfidp = get_fid(pdu->s, dfid);
if (dfidp == NULL) {
err = -EINVAL;
- goto out;
+ goto out_nofid;
}
v9fs_string_sprintf(&fullname, "%s/%s", dfidp->path.data, name.data);
@@ -1999,6 +2035,8 @@ static void v9fs_symlink(void *opaque)
offset += pdu_marshal(pdu, offset, "Q", &qid);
err = offset;
out:
+ put_fid(dfidp);
+out_nofid:
complete_pdu(pdu->s, pdu, err);
v9fs_string_free(&name);
v9fs_string_free(&symname);
@@ -2028,13 +2066,13 @@ static void v9fs_link(void *opaque)
pdu_unmarshal(pdu, offset, "dds", &dfid, &oldfid, &name);
- dfidp = lookup_fid(s, dfid);
+ dfidp = get_fid(s, dfid);
if (dfidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
- oldfidp = lookup_fid(s, oldfid);
+ oldfidp = get_fid(s, oldfid);
if (oldfidp == NULL) {
err = -ENOENT;
goto out;
@@ -2048,6 +2086,8 @@ static void v9fs_link(void *opaque)
v9fs_string_free(&fullname);
out:
+ put_fid(dfidp);
+out_nofid:
v9fs_string_free(&name);
complete_pdu(s, pdu, err);
}
@@ -2062,10 +2102,10 @@ static void v9fs_remove(void *opaque)
pdu_unmarshal(pdu, offset, "d", &fid);
- fidp = lookup_fid(pdu->s, fid);
+ fidp = get_fid(pdu->s, fid);
if (fidp == NULL) {
err = -EINVAL;
- goto out;
+ goto out_nofid;
}
err = v9fs_co_remove(pdu->s, &fidp->path);
if (!err) {
@@ -2073,8 +2113,9 @@ static void v9fs_remove(void *opaque)
}
/* For TREMOVE we need to clunk the fid even on failed remove */
+ put_fid(fidp);
free_fid(pdu->s, fidp->fid);
-out:
+out_nofid:
complete_pdu(pdu->s, pdu, err);
}
@@ -2083,14 +2124,14 @@ static int v9fs_complete_rename(V9fsState *s, V9fsFidState *fidp,
{
char *end;
int err = 0;
+ V9fsFidState *dirfidp = NULL;
char *old_name, *new_name;
if (newdirfid != -1) {
- V9fsFidState *dirfidp;
- dirfidp = lookup_fid(s, newdirfid);
+ dirfidp = get_fid(s, newdirfid);
if (dirfidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
BUG_ON(dirfidp->fid_type != P9_FID_NONE);
@@ -2143,6 +2184,10 @@ static int v9fs_complete_rename(V9fsState *s, V9fsFidState *fidp,
v9fs_string_copy(&fidp->path, name);
}
out:
+ if (dirfidp) {
+ put_fid(dirfidp);
+ }
+out_nofid:
return err;
}
@@ -2159,10 +2204,10 @@ static void v9fs_rename(void *opaque)
pdu_unmarshal(pdu, offset, "dds", &fid, &newdirfid, &name);
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
BUG_ON(fidp->fid_type != P9_FID_NONE);
@@ -2171,6 +2216,8 @@ static void v9fs_rename(void *opaque)
err = offset;
}
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(s, pdu, err);
v9fs_string_free(&name);
}
@@ -2189,10 +2236,10 @@ static void v9fs_wstat(void *opaque)
pdu_unmarshal(pdu, offset, "dwS", &fid, &unused, &v9stat);
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
err = -EINVAL;
- goto out;
+ goto out_nofid;
}
/* do we need to sync the file? */
if (donttouch_stat(&v9stat)) {
@@ -2258,6 +2305,8 @@ static void v9fs_wstat(void *opaque)
}
err = offset;
out:
+ put_fid(fidp);
+out_nofid:
v9fs_stat_free(&v9stat);
complete_pdu(s, pdu, err);
}
@@ -2318,10 +2367,10 @@ static void v9fs_statfs(void *opaque)
V9fsState *s = pdu->s;
pdu_unmarshal(pdu, offset, "d", &fid);
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
retval = -ENOENT;
- goto out;
+ goto out_nofid;
}
retval = v9fs_co_statfs(s, &fidp->path, &stbuf);
if (retval < 0) {
@@ -2330,6 +2379,8 @@ static void v9fs_statfs(void *opaque)
retval = offset;
retval += v9fs_fill_statfs(s, pdu, &stbuf);
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(s, pdu, retval);
return;
}
@@ -2355,10 +2406,10 @@ static void v9fs_mknod(void *opaque)
pdu_unmarshal(pdu, offset, "dsdddd", &fid, &name, &mode,
&major, &minor, &gid);
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
v9fs_string_sprintf(&fullname, "%s/%s", fidp->path.data, name.data);
err = v9fs_co_mknod(s, &fullname, fidp->uid, gid,
@@ -2374,6 +2425,8 @@ static void v9fs_mknod(void *opaque)
err = offset;
err += pdu_marshal(pdu, offset, "Q", &qid);
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(s, pdu, err);
v9fs_string_free(&fullname);
v9fs_string_free(&name);
@@ -2407,12 +2460,12 @@ static void v9fs_lock(void *opaque)
/* We support only block flag now (that too ignored currently) */
if (flock->flags & ~P9_LOCK_FLAGS_BLOCK) {
err = -EINVAL;
- goto out;
+ goto out_nofid;
}
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
err = v9fs_co_fstat(s, fidp->fs.fd, &stbuf);
if (err < 0) {
@@ -2420,6 +2473,8 @@ static void v9fs_lock(void *opaque)
}
status = P9_LOCK_SUCCESS;
out:
+ put_fid(fidp);
+out_nofid:
err = offset;
err += pdu_marshal(pdu, offset, "b", status);
complete_pdu(s, pdu, err);
@@ -2445,10 +2500,10 @@ static void v9fs_getlock(void *opaque)
&glock->start, &glock->length, &glock->proc_id,
&glock->client_id);
- fidp = lookup_fid(s, fid);
+ fidp = get_fid(s, fid);
if (fidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
err = v9fs_co_fstat(s, fidp->fs.fd, &stbuf);
if (err < 0) {
@@ -2460,6 +2515,8 @@ static void v9fs_getlock(void *opaque)
&glock->client_id);
err = offset;
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(s, pdu, err);
qemu_free(glock);
}
@@ -2480,10 +2537,10 @@ static void v9fs_mkdir(void *opaque)
v9fs_string_init(&fullname);
pdu_unmarshal(pdu, offset, "dsdd", &fid, &name, &mode, &gid);
- fidp = lookup_fid(pdu->s, fid);
+ fidp = get_fid(pdu->s, fid);
if (fidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
v9fs_string_sprintf(&fullname, "%s/%s", fidp->path.data, name.data);
err = v9fs_co_mkdir(pdu->s, fullname.data, mode, fidp->uid, gid);
@@ -2498,6 +2555,8 @@ static void v9fs_mkdir(void *opaque)
offset += pdu_marshal(pdu, offset, "Q", &qid);
err = offset;
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(pdu->s, pdu, err);
v9fs_string_free(&fullname);
v9fs_string_free(&name);
@@ -2511,15 +2570,15 @@ static void v9fs_xattrwalk(void *opaque)
size_t offset = 7;
int32_t fid, newfid;
V9fsFidState *file_fidp;
- V9fsFidState *xattr_fidp;
+ V9fsFidState *xattr_fidp = NULL;
V9fsPDU *pdu = opaque;
V9fsState *s = pdu->s;
pdu_unmarshal(pdu, offset, "dds", &fid, &newfid, &name);
- file_fidp = lookup_fid(s, fid);
+ file_fidp = get_fid(s, fid);
if (file_fidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
xattr_fidp = alloc_fid(s, newfid);
if (xattr_fidp == NULL) {
@@ -2534,7 +2593,9 @@ static void v9fs_xattrwalk(void *opaque)
size = v9fs_co_llistxattr(s, &xattr_fidp->path, NULL, 0);
if (size < 0) {
err = size;
+ put_fid(xattr_fidp);
free_fid(s, xattr_fidp->fid);
+ xattr_fidp = NULL;
goto out;
}
/*
@@ -2549,7 +2610,9 @@ static void v9fs_xattrwalk(void *opaque)
xattr_fidp->fs.xattr.value,
xattr_fidp->fs.xattr.len);
if (err < 0) {
+ put_fid(xattr_fidp);
free_fid(s, xattr_fidp->fid);
+ xattr_fidp = NULL;
goto out;
}
}
@@ -2564,7 +2627,9 @@ static void v9fs_xattrwalk(void *opaque)
&name, NULL, 0);
if (size < 0) {
err = size;
+ put_fid(xattr_fidp);
free_fid(s, xattr_fidp->fid);
+ xattr_fidp = NULL;
goto out;
}
/*
@@ -2579,7 +2644,9 @@ static void v9fs_xattrwalk(void *opaque)
&name, xattr_fidp->fs.xattr.value,
xattr_fidp->fs.xattr.len);
if (err < 0) {
+ put_fid(xattr_fidp);
free_fid(s, xattr_fidp->fid);
+ xattr_fidp = NULL;
goto out;
}
}
@@ -2587,6 +2654,11 @@ static void v9fs_xattrwalk(void *opaque)
err = offset;
}
out:
+ put_fid(file_fidp);
+ if (xattr_fidp) {
+ put_fid(xattr_fidp);
+ }
+out_nofid:
complete_pdu(s, pdu, err);
v9fs_string_free(&name);
}
@@ -2607,10 +2679,10 @@ static void v9fs_xattrcreate(void *opaque)
pdu_unmarshal(pdu, offset, "dsqd",
&fid, &name, &size, &flags);
- file_fidp = lookup_fid(s, fid);
+ file_fidp = get_fid(s, fid);
if (file_fidp == NULL) {
err = -EINVAL;
- goto out;
+ goto out_nofid;
}
/* Make the file fid point to xattr */
xattr_fidp = file_fidp;
@@ -2626,7 +2698,8 @@ static void v9fs_xattrcreate(void *opaque)
xattr_fidp->fs.xattr.value = NULL;
}
err = offset;
-out:
+ put_fid(file_fidp);
+out_nofid:
complete_pdu(s, pdu, err);
v9fs_string_free(&name);
}
@@ -2641,10 +2714,10 @@ static void v9fs_readlink(void *opaque)
V9fsFidState *fidp;
pdu_unmarshal(pdu, offset, "d", &fid);
- fidp = lookup_fid(pdu->s, fid);
+ fidp = get_fid(pdu->s, fid);
if (fidp == NULL) {
err = -ENOENT;
- goto out;
+ goto out_nofid;
}
v9fs_string_init(&target);
@@ -2656,6 +2729,8 @@ static void v9fs_readlink(void *opaque)
err = offset;
v9fs_string_free(&target);
out:
+ put_fid(fidp);
+out_nofid:
complete_pdu(pdu->s, pdu, err);
}
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 1d8c1b1..ce93c20 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -203,6 +203,7 @@ struct V9fsFidState
V9fsXattr xattr;
} fs;
uid_t uid;
+ int ref;
V9fsFidState *next;
};
@@ -361,5 +362,11 @@ static inline size_t do_pdu_unpack(void *dst, struct iovec *sg, int sg_count,
return pdu_packunpack(dst, sg, sg_count, offset, size, 0);
}
+static inline void put_fid(V9fsFidState *fidp)
+{
+ BUG_ON(!fidp->ref);
+ fidp->ref--;
+}
+
extern void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq);
#endif
--
1.7.4.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 2/6] hw/9pfs: Don't free fid in clunk
2011-06-06 17:16 [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid Aneesh Kumar K.V
@ 2011-06-06 17:16 ` Aneesh Kumar K.V
2011-06-10 0:27 ` Venkateswararao Jujjuri
2011-06-06 17:16 ` [Qemu-devel] [PATCH 3/6] hw/9pfs: Add file descriptor reclaim support Aneesh Kumar K.V
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Aneesh Kumar K.V @ 2011-06-06 17:16 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Aneesh Kumar K.V
On interrupted syscall on client we can end up having fid
being acted upon by glib pool but still get a clunk request on that
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
hw/9pfs/virtio-9p.c | 139 +++++++++++++++++++++++++++-----------------------
hw/9pfs/virtio-9p.h | 7 +--
2 files changed, 76 insertions(+), 70 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 03d8664..f3127a5 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -237,12 +237,11 @@ static V9fsFidState *get_fid(V9fsState *s, int32_t fid)
V9fsFidState *f;
for (f = s->fid_list; f; f = f->next) {
- if (f->fid == fid) {
+ if (f->fid == fid && !f->clunked) {
f->ref++;
return f;
}
}
-
return NULL;
}
@@ -250,12 +249,12 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
{
V9fsFidState *f;
- f = get_fid(s, fid);
- if (f) {
- f->ref--;
- return NULL;
+ for (f = s->fid_list; f; f = f->next) {
+ /* If fid is already there return NULL */
+ if (f->fid == fid && !f->clunked) {
+ return NULL;
+ }
}
-
f = qemu_mallocz(sizeof(V9fsFidState));
f->fid = fid;
f->fid_type = P9_FID_NONE;
@@ -300,9 +299,33 @@ free_value:
return retval;
}
-static int free_fid(V9fsState *s, int32_t fid)
+static int release_fid(V9fsState *s, V9fsFidState *fidp)
{
int retval = 0;
+
+ if (fidp->fid_type == P9_FID_FILE) {
+ retval = v9fs_co_close(s, fidp);
+ } else if (fidp->fid_type == P9_FID_DIR) {
+ retval = v9fs_co_closedir(s, fidp);
+ } else if (fidp->fid_type == P9_FID_XATTR) {
+ retval = v9fs_xattr_fid_clunk(s, fidp);
+ }
+ v9fs_string_free(&fidp->path);
+ qemu_free(fidp);
+ return retval;
+}
+
+static void put_fid(V9fsState *s, V9fsFidState *fidp)
+{
+ BUG_ON(!fidp->ref);
+ fidp->ref--;
+ if (!fidp->ref && fidp->clunked) {
+ release_fid(s, fidp);
+ }
+}
+
+static int free_fid(V9fsState *s, int32_t fid)
+{
V9fsFidState **fidpp, *fidp;
for (fidpp = &s->fid_list; *fidpp; fidpp = &(*fidpp)->next) {
@@ -314,20 +337,10 @@ static int free_fid(V9fsState *s, int32_t fid)
if (*fidpp == NULL) {
return -ENOENT;
}
-
fidp = *fidpp;
*fidpp = fidp->next;
-
- if (fidp->fid_type == P9_FID_FILE) {
- retval = v9fs_co_close(s, fidp);
- } else if (fidp->fid_type == P9_FID_DIR) {
- retval = v9fs_co_closedir(s, fidp);
- } else if (fidp->fid_type == P9_FID_XATTR) {
- retval = v9fs_xattr_fid_clunk(s, fidp);
- }
- v9fs_string_free(&fidp->path);
- qemu_free(fidp);
- return retval;
+ fidp->clunked = 1;
+ return 0;
}
#define P9_QID_TYPE_DIR 0x80
@@ -1022,14 +1035,13 @@ static void v9fs_attach(void *opaque)
err = fid_to_qid(s, fidp, &qid);
if (err < 0) {
err = -EINVAL;
- put_fid(fidp);
free_fid(s, fid);
- goto out_nofid;
+ goto out;
}
offset += pdu_marshal(pdu, offset, "Q", &qid);
err = offset;
- put_fid(fidp);
-
+out:
+ put_fid(s, fidp);
out_nofid:
complete_pdu(s, pdu, err);
v9fs_string_free(&uname);
@@ -1066,7 +1078,7 @@ static void v9fs_stat(void *opaque)
err = offset;
v9fs_stat_free(&v9stat);
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
complete_pdu(s, pdu, err);
}
@@ -1102,7 +1114,7 @@ static void v9fs_getattr(void *opaque)
retval = offset;
retval += pdu_marshal(pdu, offset, "A", &v9stat_dotl);
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
complete_pdu(s, pdu, retval);
}
@@ -1196,7 +1208,7 @@ static void v9fs_setattr(void *opaque)
}
err = offset;
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
complete_pdu(s, pdu, err);
}
@@ -1279,9 +1291,7 @@ static void v9fs_walk(void *opaque)
v9fs_string_copy(&newfidp->path, &path);
err = v9fs_co_lstat(s, &newfidp->path, &stbuf);
if (err < 0) {
- put_fid(newfidp);
free_fid(s, newfidp->fid);
- newfidp = NULL;
v9fs_string_free(&path);
goto out;
}
@@ -1291,9 +1301,9 @@ static void v9fs_walk(void *opaque)
}
err = v9fs_walk_marshal(pdu, nwnames, qids);
out:
- put_fid(fidp);
+ put_fid(s, fidp);
if (newfidp) {
- put_fid(newfidp);
+ put_fid(s, newfidp);
}
out_nofid:
complete_pdu(s, pdu, err);
@@ -1383,7 +1393,7 @@ static void v9fs_open(void *opaque)
err = offset;
}
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
complete_pdu(s, pdu, err);
}
@@ -1437,7 +1447,7 @@ static void v9fs_lcreate(void *opaque)
offset += pdu_marshal(pdu, offset, "Qd", &qid, iounit);
err = offset;
out:
- put_fid(fidp);
+ put_fid(pdu->s, fidp);
out_nofid:
complete_pdu(pdu->s, pdu, err);
v9fs_string_free(&name);
@@ -1465,7 +1475,7 @@ static void v9fs_fsync(void *opaque)
err = offset;
}
out:
- put_fid(fidp);
+ put_fid(s, fidp);
complete_pdu(s, pdu, err);
}
@@ -1474,16 +1484,25 @@ static void v9fs_clunk(void *opaque)
int err;
int32_t fid;
size_t offset = 7;
+ V9fsFidState *fidp;
V9fsPDU *pdu = opaque;
V9fsState *s = pdu->s;
pdu_unmarshal(pdu, offset, "d", &fid);
- err = free_fid(s, fid);
+
+ fidp = get_fid(s, fid);
+ if (fidp == NULL) {
+ err = -ENOENT;
+ goto out_nofid;
+ }
+ err = free_fid(s, fidp->fid);
if (err < 0) {
goto out;
}
err = offset;
out:
+ put_fid(s, fidp);
+out_nofid:
complete_pdu(s, pdu, err);
}
@@ -1638,7 +1657,7 @@ static void v9fs_read(void *opaque)
err = -EINVAL;
}
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
complete_pdu(s, pdu, err);
}
@@ -1747,7 +1766,7 @@ static void v9fs_readdir(void *opaque)
retval += pdu_marshal(pdu, offset, "d", count);
retval += count;
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
complete_pdu(s, pdu, retval);
}
@@ -1857,7 +1876,7 @@ static void v9fs_write(void *opaque)
offset += pdu_marshal(pdu, offset, "d", total);
err = offset;
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
complete_pdu(s, pdu, err);
}
@@ -1923,10 +1942,10 @@ static void v9fs_create(void *opaque)
}
err = v9fs_co_link(pdu->s, &nfidp->path, &fullname);
if (err < 0) {
- put_fid(nfidp);
+ put_fid(pdu->s, nfidp);
goto out;
}
- put_fid(nfidp);
+ put_fid(pdu->s, nfidp);
} else if (perm & P9_STAT_MODE_DEVICE) {
char ctype;
uint32_t major, minor;
@@ -1990,7 +2009,7 @@ static void v9fs_create(void *opaque)
err = offset;
out:
- put_fid(fidp);
+ put_fid(pdu->s, fidp);
out_nofid:
complete_pdu(pdu->s, pdu, err);
v9fs_string_free(&name);
@@ -2035,7 +2054,7 @@ static void v9fs_symlink(void *opaque)
offset += pdu_marshal(pdu, offset, "Q", &qid);
err = offset;
out:
- put_fid(dfidp);
+ put_fid(pdu->s, dfidp);
out_nofid:
complete_pdu(pdu->s, pdu, err);
v9fs_string_free(&name);
@@ -2086,7 +2105,7 @@ static void v9fs_link(void *opaque)
v9fs_string_free(&fullname);
out:
- put_fid(dfidp);
+ put_fid(s, dfidp);
out_nofid:
v9fs_string_free(&name);
complete_pdu(s, pdu, err);
@@ -2113,8 +2132,8 @@ static void v9fs_remove(void *opaque)
}
/* For TREMOVE we need to clunk the fid even on failed remove */
- put_fid(fidp);
free_fid(pdu->s, fidp->fid);
+ put_fid(pdu->s, fidp);
out_nofid:
complete_pdu(pdu->s, pdu, err);
}
@@ -2185,7 +2204,7 @@ static int v9fs_complete_rename(V9fsState *s, V9fsFidState *fidp,
}
out:
if (dirfidp) {
- put_fid(dirfidp);
+ put_fid(s, dirfidp);
}
out_nofid:
return err;
@@ -2216,7 +2235,7 @@ static void v9fs_rename(void *opaque)
err = offset;
}
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
complete_pdu(s, pdu, err);
v9fs_string_free(&name);
@@ -2305,7 +2324,7 @@ static void v9fs_wstat(void *opaque)
}
err = offset;
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
v9fs_stat_free(&v9stat);
complete_pdu(s, pdu, err);
@@ -2379,7 +2398,7 @@ static void v9fs_statfs(void *opaque)
retval = offset;
retval += v9fs_fill_statfs(s, pdu, &stbuf);
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
complete_pdu(s, pdu, retval);
return;
@@ -2425,7 +2444,7 @@ static void v9fs_mknod(void *opaque)
err = offset;
err += pdu_marshal(pdu, offset, "Q", &qid);
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
complete_pdu(s, pdu, err);
v9fs_string_free(&fullname);
@@ -2473,7 +2492,7 @@ static void v9fs_lock(void *opaque)
}
status = P9_LOCK_SUCCESS;
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
err = offset;
err += pdu_marshal(pdu, offset, "b", status);
@@ -2515,7 +2534,7 @@ static void v9fs_getlock(void *opaque)
&glock->client_id);
err = offset;
out:
- put_fid(fidp);
+ put_fid(s, fidp);
out_nofid:
complete_pdu(s, pdu, err);
qemu_free(glock);
@@ -2555,7 +2574,7 @@ static void v9fs_mkdir(void *opaque)
offset += pdu_marshal(pdu, offset, "Q", &qid);
err = offset;
out:
- put_fid(fidp);
+ put_fid(pdu->s, fidp);
out_nofid:
complete_pdu(pdu->s, pdu, err);
v9fs_string_free(&fullname);
@@ -2593,9 +2612,7 @@ static void v9fs_xattrwalk(void *opaque)
size = v9fs_co_llistxattr(s, &xattr_fidp->path, NULL, 0);
if (size < 0) {
err = size;
- put_fid(xattr_fidp);
free_fid(s, xattr_fidp->fid);
- xattr_fidp = NULL;
goto out;
}
/*
@@ -2610,9 +2627,7 @@ static void v9fs_xattrwalk(void *opaque)
xattr_fidp->fs.xattr.value,
xattr_fidp->fs.xattr.len);
if (err < 0) {
- put_fid(xattr_fidp);
free_fid(s, xattr_fidp->fid);
- xattr_fidp = NULL;
goto out;
}
}
@@ -2627,9 +2642,7 @@ static void v9fs_xattrwalk(void *opaque)
&name, NULL, 0);
if (size < 0) {
err = size;
- put_fid(xattr_fidp);
free_fid(s, xattr_fidp->fid);
- xattr_fidp = NULL;
goto out;
}
/*
@@ -2644,9 +2657,7 @@ static void v9fs_xattrwalk(void *opaque)
&name, xattr_fidp->fs.xattr.value,
xattr_fidp->fs.xattr.len);
if (err < 0) {
- put_fid(xattr_fidp);
free_fid(s, xattr_fidp->fid);
- xattr_fidp = NULL;
goto out;
}
}
@@ -2654,9 +2665,9 @@ static void v9fs_xattrwalk(void *opaque)
err = offset;
}
out:
- put_fid(file_fidp);
+ put_fid(s, file_fidp);
if (xattr_fidp) {
- put_fid(xattr_fidp);
+ put_fid(s, xattr_fidp);
}
out_nofid:
complete_pdu(s, pdu, err);
@@ -2698,7 +2709,7 @@ static void v9fs_xattrcreate(void *opaque)
xattr_fidp->fs.xattr.value = NULL;
}
err = offset;
- put_fid(file_fidp);
+ put_fid(s, file_fidp);
out_nofid:
complete_pdu(s, pdu, err);
v9fs_string_free(&name);
@@ -2729,7 +2740,7 @@ static void v9fs_readlink(void *opaque)
err = offset;
v9fs_string_free(&target);
out:
- put_fid(fidp);
+ put_fid(pdu->s, fidp);
out_nofid:
complete_pdu(pdu->s, pdu, err);
}
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index ce93c20..e16e5f4 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -204,6 +204,7 @@ struct V9fsFidState
} fs;
uid_t uid;
int ref;
+ int clunked;
V9fsFidState *next;
};
@@ -362,11 +363,5 @@ static inline size_t do_pdu_unpack(void *dst, struct iovec *sg, int sg_count,
return pdu_packunpack(dst, sg, sg_count, offset, size, 0);
}
-static inline void put_fid(V9fsFidState *fidp)
-{
- BUG_ON(!fidp->ref);
- fidp->ref--;
-}
-
extern void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq);
#endif
--
1.7.4.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH 2/6] hw/9pfs: Don't free fid in clunk
2011-06-06 17:16 ` [Qemu-devel] [PATCH 2/6] hw/9pfs: Don't free fid in clunk Aneesh Kumar K.V
@ 2011-06-10 0:27 ` Venkateswararao Jujjuri
2011-06-10 6:19 ` Aneesh Kumar K.V
0 siblings, 1 reply; 13+ messages in thread
From: Venkateswararao Jujjuri @ 2011-06-10 0:27 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: aliguori, qemu-devel
On 06/06/2011 10:16 AM, Aneesh Kumar K.V wrote:
> On interrupted syscall on client we can end up having fid
> being acted upon by glib pool but still get a clunk request on that
Couple of comments below.
- JV
> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
> ---
> hw/9pfs/virtio-9p.c | 139 +++++++++++++++++++++++++++-----------------------
> hw/9pfs/virtio-9p.h | 7 +--
> 2 files changed, 76 insertions(+), 70 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index 03d8664..f3127a5 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -237,12 +237,11 @@ static V9fsFidState *get_fid(V9fsState *s, int32_t fid)
> V9fsFidState *f;
>
> for (f = s->fid_list; f; f = f->next) {
> - if (f->fid == fid) {
> + if (f->fid == fid&& !f->clunked) {
> f->ref++;
> return f;
> }
> }
> -
> return NULL;
> }
>
> @@ -250,12 +249,12 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
> {
> V9fsFidState *f;
>
> - f = get_fid(s, fid);
> - if (f) {
> - f->ref--;
> - return NULL;
> + for (f = s->fid_list; f; f = f->next) {
> + /* If fid is already there return NULL */
> + if (f->fid == fid&& !f->clunked) {
> + return NULL;
> + }
> }
> -
> f = qemu_mallocz(sizeof(V9fsFidState));
Memory leak if we find a cluncked fid here?
More than that how do you handle this?
> f->fid = fid;
> f->fid_type = P9_FID_NONE;
> @@ -300,9 +299,33 @@ free_value:
> return retval;
> }
>
> -static int free_fid(V9fsState *s, int32_t fid)
> +static int release_fid(V9fsState *s, V9fsFidState *fidp)
> {
> int retval = 0;
> +
> + if (fidp->fid_type == P9_FID_FILE) {
> + retval = v9fs_co_close(s, fidp);
> + } else if (fidp->fid_type == P9_FID_DIR) {
> + retval = v9fs_co_closedir(s, fidp);
> + } else if (fidp->fid_type == P9_FID_XATTR) {
> + retval = v9fs_xattr_fid_clunk(s, fidp);
> + }
> + v9fs_string_free(&fidp->path);
> + qemu_free(fidp);
> + return retval;
> +}
> +
> +static void put_fid(V9fsState *s, V9fsFidState *fidp)
> +{
> + BUG_ON(!fidp->ref);
> + fidp->ref--;
> + if (!fidp->ref&& fidp->clunked) {
> + release_fid(s, fidp);
> + }
> +}
> +
> +static int free_fid(V9fsState *s, int32_t fid)
With the changed semantics; I would suggest you to swap the names of
release_fid() and free_fid()
Or even better
free_fid() -> clunk_fid()
release_fid() -> free_fid()
> +{
> V9fsFidState **fidpp, *fidp;
>
> for (fidpp =&s->fid_list; *fidpp; fidpp =&(*fidpp)->next) {
> @@ -314,20 +337,10 @@ static int free_fid(V9fsState *s, int32_t fid)
> if (*fidpp == NULL) {
> return -ENOENT;
> }
> -
> fidp = *fidpp;
> *fidpp = fidp->next;
> -
> - if (fidp->fid_type == P9_FID_FILE) {
> - retval = v9fs_co_close(s, fidp);
> - } else if (fidp->fid_type == P9_FID_DIR) {
> - retval = v9fs_co_closedir(s, fidp);
> - } else if (fidp->fid_type == P9_FID_XATTR) {
> - retval = v9fs_xattr_fid_clunk(s, fidp);
> - }
> - v9fs_string_free(&fidp->path);
> - qemu_free(fidp);
> - return retval;
> + fidp->clunked = 1;
> + return 0;
> }
>
> #define P9_QID_TYPE_DIR 0x80
> @@ -1022,14 +1035,13 @@ static void v9fs_attach(void *opaque)
> err = fid_to_qid(s, fidp,&qid);
> if (err< 0) {
> err = -EINVAL;
> - put_fid(fidp);
> free_fid(s, fid);
> - goto out_nofid;
> + goto out;
> }
> offset += pdu_marshal(pdu, offset, "Q",&qid);
> err = offset;
> - put_fid(fidp);
> -
> +out:
> + put_fid(s, fidp);
> out_nofid:
> complete_pdu(s, pdu, err);
> v9fs_string_free(&uname);
> @@ -1066,7 +1078,7 @@ static void v9fs_stat(void *opaque)
> err = offset;
> v9fs_stat_free(&v9stat);
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> complete_pdu(s, pdu, err);
> }
> @@ -1102,7 +1114,7 @@ static void v9fs_getattr(void *opaque)
> retval = offset;
> retval += pdu_marshal(pdu, offset, "A",&v9stat_dotl);
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> complete_pdu(s, pdu, retval);
> }
> @@ -1196,7 +1208,7 @@ static void v9fs_setattr(void *opaque)
> }
> err = offset;
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> complete_pdu(s, pdu, err);
> }
> @@ -1279,9 +1291,7 @@ static void v9fs_walk(void *opaque)
> v9fs_string_copy(&newfidp->path,&path);
> err = v9fs_co_lstat(s,&newfidp->path,&stbuf);
> if (err< 0) {
> - put_fid(newfidp);
> free_fid(s, newfidp->fid);
> - newfidp = NULL;
> v9fs_string_free(&path);
> goto out;
> }
> @@ -1291,9 +1301,9 @@ static void v9fs_walk(void *opaque)
> }
> err = v9fs_walk_marshal(pdu, nwnames, qids);
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> if (newfidp) {
> - put_fid(newfidp);
> + put_fid(s, newfidp);
> }
> out_nofid:
> complete_pdu(s, pdu, err);
> @@ -1383,7 +1393,7 @@ static void v9fs_open(void *opaque)
> err = offset;
> }
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> complete_pdu(s, pdu, err);
> }
> @@ -1437,7 +1447,7 @@ static void v9fs_lcreate(void *opaque)
> offset += pdu_marshal(pdu, offset, "Qd",&qid, iounit);
> err = offset;
> out:
> - put_fid(fidp);
> + put_fid(pdu->s, fidp);
> out_nofid:
> complete_pdu(pdu->s, pdu, err);
> v9fs_string_free(&name);
> @@ -1465,7 +1475,7 @@ static void v9fs_fsync(void *opaque)
> err = offset;
> }
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> complete_pdu(s, pdu, err);
> }
>
> @@ -1474,16 +1484,25 @@ static void v9fs_clunk(void *opaque)
> int err;
> int32_t fid;
> size_t offset = 7;
> + V9fsFidState *fidp;
> V9fsPDU *pdu = opaque;
> V9fsState *s = pdu->s;
>
> pdu_unmarshal(pdu, offset, "d",&fid);
> - err = free_fid(s, fid);
> +
> + fidp = get_fid(s, fid);
> + if (fidp == NULL) {
> + err = -ENOENT;
> + goto out_nofid;
> + }
> + err = free_fid(s, fidp->fid);
> if (err< 0) {
> goto out;
> }
> err = offset;
> out:
> + put_fid(s, fidp);
> +out_nofid:
> complete_pdu(s, pdu, err);
> }
>
> @@ -1638,7 +1657,7 @@ static void v9fs_read(void *opaque)
> err = -EINVAL;
> }
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> complete_pdu(s, pdu, err);
> }
> @@ -1747,7 +1766,7 @@ static void v9fs_readdir(void *opaque)
> retval += pdu_marshal(pdu, offset, "d", count);
> retval += count;
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> complete_pdu(s, pdu, retval);
> }
> @@ -1857,7 +1876,7 @@ static void v9fs_write(void *opaque)
> offset += pdu_marshal(pdu, offset, "d", total);
> err = offset;
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> complete_pdu(s, pdu, err);
> }
> @@ -1923,10 +1942,10 @@ static void v9fs_create(void *opaque)
> }
> err = v9fs_co_link(pdu->s,&nfidp->path,&fullname);
> if (err< 0) {
> - put_fid(nfidp);
> + put_fid(pdu->s, nfidp);
> goto out;
> }
> - put_fid(nfidp);
> + put_fid(pdu->s, nfidp);
> } else if (perm& P9_STAT_MODE_DEVICE) {
> char ctype;
> uint32_t major, minor;
> @@ -1990,7 +2009,7 @@ static void v9fs_create(void *opaque)
> err = offset;
>
> out:
> - put_fid(fidp);
> + put_fid(pdu->s, fidp);
> out_nofid:
> complete_pdu(pdu->s, pdu, err);
> v9fs_string_free(&name);
> @@ -2035,7 +2054,7 @@ static void v9fs_symlink(void *opaque)
> offset += pdu_marshal(pdu, offset, "Q",&qid);
> err = offset;
> out:
> - put_fid(dfidp);
> + put_fid(pdu->s, dfidp);
> out_nofid:
> complete_pdu(pdu->s, pdu, err);
> v9fs_string_free(&name);
> @@ -2086,7 +2105,7 @@ static void v9fs_link(void *opaque)
> v9fs_string_free(&fullname);
>
> out:
> - put_fid(dfidp);
> + put_fid(s, dfidp);
> out_nofid:
> v9fs_string_free(&name);
> complete_pdu(s, pdu, err);
> @@ -2113,8 +2132,8 @@ static void v9fs_remove(void *opaque)
> }
>
> /* For TREMOVE we need to clunk the fid even on failed remove */
> - put_fid(fidp);
> free_fid(pdu->s, fidp->fid);
> + put_fid(pdu->s, fidp);
> out_nofid:
> complete_pdu(pdu->s, pdu, err);
> }
> @@ -2185,7 +2204,7 @@ static int v9fs_complete_rename(V9fsState *s, V9fsFidState *fidp,
> }
> out:
> if (dirfidp) {
> - put_fid(dirfidp);
> + put_fid(s, dirfidp);
> }
> out_nofid:
> return err;
> @@ -2216,7 +2235,7 @@ static void v9fs_rename(void *opaque)
> err = offset;
> }
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> complete_pdu(s, pdu, err);
> v9fs_string_free(&name);
> @@ -2305,7 +2324,7 @@ static void v9fs_wstat(void *opaque)
> }
> err = offset;
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> v9fs_stat_free(&v9stat);
> complete_pdu(s, pdu, err);
> @@ -2379,7 +2398,7 @@ static void v9fs_statfs(void *opaque)
> retval = offset;
> retval += v9fs_fill_statfs(s, pdu,&stbuf);
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> complete_pdu(s, pdu, retval);
> return;
> @@ -2425,7 +2444,7 @@ static void v9fs_mknod(void *opaque)
> err = offset;
> err += pdu_marshal(pdu, offset, "Q",&qid);
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> complete_pdu(s, pdu, err);
> v9fs_string_free(&fullname);
> @@ -2473,7 +2492,7 @@ static void v9fs_lock(void *opaque)
> }
> status = P9_LOCK_SUCCESS;
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> err = offset;
> err += pdu_marshal(pdu, offset, "b", status);
> @@ -2515,7 +2534,7 @@ static void v9fs_getlock(void *opaque)
> &glock->client_id);
> err = offset;
> out:
> - put_fid(fidp);
> + put_fid(s, fidp);
> out_nofid:
> complete_pdu(s, pdu, err);
> qemu_free(glock);
> @@ -2555,7 +2574,7 @@ static void v9fs_mkdir(void *opaque)
> offset += pdu_marshal(pdu, offset, "Q",&qid);
> err = offset;
> out:
> - put_fid(fidp);
> + put_fid(pdu->s, fidp);
> out_nofid:
> complete_pdu(pdu->s, pdu, err);
> v9fs_string_free(&fullname);
> @@ -2593,9 +2612,7 @@ static void v9fs_xattrwalk(void *opaque)
> size = v9fs_co_llistxattr(s,&xattr_fidp->path, NULL, 0);
> if (size< 0) {
> err = size;
> - put_fid(xattr_fidp);
> free_fid(s, xattr_fidp->fid);
> - xattr_fidp = NULL;
> goto out;
> }
> /*
> @@ -2610,9 +2627,7 @@ static void v9fs_xattrwalk(void *opaque)
> xattr_fidp->fs.xattr.value,
> xattr_fidp->fs.xattr.len);
> if (err< 0) {
> - put_fid(xattr_fidp);
> free_fid(s, xattr_fidp->fid);
> - xattr_fidp = NULL;
> goto out;
> }
> }
> @@ -2627,9 +2642,7 @@ static void v9fs_xattrwalk(void *opaque)
> &name, NULL, 0);
> if (size< 0) {
> err = size;
> - put_fid(xattr_fidp);
> free_fid(s, xattr_fidp->fid);
> - xattr_fidp = NULL;
> goto out;
> }
> /*
> @@ -2644,9 +2657,7 @@ static void v9fs_xattrwalk(void *opaque)
> &name, xattr_fidp->fs.xattr.value,
> xattr_fidp->fs.xattr.len);
> if (err< 0) {
> - put_fid(xattr_fidp);
> free_fid(s, xattr_fidp->fid);
> - xattr_fidp = NULL;
> goto out;
> }
> }
> @@ -2654,9 +2665,9 @@ static void v9fs_xattrwalk(void *opaque)
> err = offset;
> }
> out:
> - put_fid(file_fidp);
> + put_fid(s, file_fidp);
> if (xattr_fidp) {
> - put_fid(xattr_fidp);
> + put_fid(s, xattr_fidp);
> }
> out_nofid:
> complete_pdu(s, pdu, err);
> @@ -2698,7 +2709,7 @@ static void v9fs_xattrcreate(void *opaque)
> xattr_fidp->fs.xattr.value = NULL;
> }
> err = offset;
> - put_fid(file_fidp);
> + put_fid(s, file_fidp);
> out_nofid:
> complete_pdu(s, pdu, err);
> v9fs_string_free(&name);
> @@ -2729,7 +2740,7 @@ static void v9fs_readlink(void *opaque)
> err = offset;
> v9fs_string_free(&target);
> out:
> - put_fid(fidp);
> + put_fid(pdu->s, fidp);
> out_nofid:
> complete_pdu(pdu->s, pdu, err);
> }
> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> index ce93c20..e16e5f4 100644
> --- a/hw/9pfs/virtio-9p.h
> +++ b/hw/9pfs/virtio-9p.h
> @@ -204,6 +204,7 @@ struct V9fsFidState
> } fs;
> uid_t uid;
> int ref;
> + int clunked;
> V9fsFidState *next;
> };
>
> @@ -362,11 +363,5 @@ static inline size_t do_pdu_unpack(void *dst, struct iovec *sg, int sg_count,
> return pdu_packunpack(dst, sg, sg_count, offset, size, 0);
> }
>
> -static inline void put_fid(V9fsFidState *fidp)
> -{
> - BUG_ON(!fidp->ref);
> - fidp->ref--;
> -}
> -
> extern void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq);
> #endif
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH 2/6] hw/9pfs: Don't free fid in clunk
2011-06-10 0:27 ` Venkateswararao Jujjuri
@ 2011-06-10 6:19 ` Aneesh Kumar K.V
0 siblings, 0 replies; 13+ messages in thread
From: Aneesh Kumar K.V @ 2011-06-10 6:19 UTC (permalink / raw)
To: Venkateswararao Jujjuri; +Cc: aliguori, qemu-devel
On Thu, 09 Jun 2011 17:27:00 -0700, Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com> wrote:
> On 06/06/2011 10:16 AM, Aneesh Kumar K.V wrote:
> > On interrupted syscall on client we can end up having fid
> > being acted upon by glib pool but still get a clunk request on that
>
> Couple of comments below.
>
> - JV
>
> > Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
> > ---
> > hw/9pfs/virtio-9p.c | 139 +++++++++++++++++++++++++++-----------------------
> > hw/9pfs/virtio-9p.h | 7 +--
> > 2 files changed, 76 insertions(+), 70 deletions(-)
> >
> > diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> > index 03d8664..f3127a5 100644
> > --- a/hw/9pfs/virtio-9p.c
> > +++ b/hw/9pfs/virtio-9p.c
> > @@ -237,12 +237,11 @@ static V9fsFidState *get_fid(V9fsState *s, int32_t fid)
> > V9fsFidState *f;
> >
> > for (f = s->fid_list; f; f = f->next) {
> > - if (f->fid == fid) {
> > + if (f->fid == fid&& !f->clunked) {
> > f->ref++;
> > return f;
> > }
> > }
> > -
> > return NULL;
> > }
> >
> > @@ -250,12 +249,12 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
> > {
> > V9fsFidState *f;
> >
> > - f = get_fid(s, fid);
> > - if (f) {
> > - f->ref--;
> > - return NULL;
> > + for (f = s->fid_list; f; f = f->next) {
> > + /* If fid is already there return NULL */
> > + if (f->fid == fid&& !f->clunked) {
> > + return NULL;
> > + }
> > }
> > -
> > f = qemu_mallocz(sizeof(V9fsFidState));
> Memory leak if we find a cluncked fid here?
> More than that how do you handle this?
clunk request happening parallel to alloc request ? is that
possible. Only when the alloc succeed the client will find the
fid initialized and only on initialized fid we send clunk request.
> > f->fid = fid;
> > f->fid_type = P9_FID_NONE;
> > @@ -300,9 +299,33 @@ free_value:
> > return retval;
> > }
> >
> > -static int free_fid(V9fsState *s, int32_t fid)
> > +static int release_fid(V9fsState *s, V9fsFidState *fidp)
> > {
> > int retval = 0;
> > +
> > + if (fidp->fid_type == P9_FID_FILE) {
> > + retval = v9fs_co_close(s, fidp);
> > + } else if (fidp->fid_type == P9_FID_DIR) {
> > + retval = v9fs_co_closedir(s, fidp);
> > + } else if (fidp->fid_type == P9_FID_XATTR) {
> > + retval = v9fs_xattr_fid_clunk(s, fidp);
> > + }
> > + v9fs_string_free(&fidp->path);
> > + qemu_free(fidp);
> > + return retval;
> > +}
> > +
> > +static void put_fid(V9fsState *s, V9fsFidState *fidp)
> > +{
> > + BUG_ON(!fidp->ref);
> > + fidp->ref--;
> > + if (!fidp->ref&& fidp->clunked) {
> > + release_fid(s, fidp);
> > + }
> > +}
> > +
> > +static int free_fid(V9fsState *s, int32_t fid)
> With the changed semantics; I would suggest you to swap the names of
> release_fid() and free_fid()
>
> Or even better
> free_fid() -> clunk_fid()
> release_fid() -> free_fid()
ok
-aneesh
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 3/6] hw/9pfs: Add file descriptor reclaim support
2011-06-06 17:16 [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid Aneesh Kumar K.V
2011-06-06 17:16 ` [Qemu-devel] [PATCH 2/6] hw/9pfs: Don't free fid in clunk Aneesh Kumar K.V
@ 2011-06-06 17:16 ` Aneesh Kumar K.V
2011-06-10 1:27 ` Venkateswararao Jujjuri
2011-06-06 17:16 ` [Qemu-devel] [PATCH 4/6] hw/9pfs: init fid list properly Aneesh Kumar K.V
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Aneesh Kumar K.V @ 2011-06-06 17:16 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Aneesh Kumar K.V
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
hw/9pfs/codir.c | 4 +-
hw/9pfs/cofile.c | 19 ++++-
hw/9pfs/virtio-9p-coth.h | 4 +-
hw/9pfs/virtio-9p-device.c | 1 +
hw/9pfs/virtio-9p.c | 193 +++++++++++++++++++++++++++++++++++++++++++-
hw/9pfs/virtio-9p.h | 23 +++++-
6 files changed, 229 insertions(+), 15 deletions(-)
diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
index 783d279..3347038 100644
--- a/hw/9pfs/codir.c
+++ b/hw/9pfs/codir.c
@@ -101,12 +101,10 @@ int v9fs_co_opendir(V9fsState *s, V9fsFidState *fidp)
return err;
}
-int v9fs_co_closedir(V9fsState *s, V9fsFidState *fidp)
+int v9fs_co_closedir(V9fsState *s, DIR *dir)
{
int err;
- DIR *dir;
- dir = fidp->fs.dir;
v9fs_co_run_in_worker(
{
err = s->ops->closedir(&s->ctx, dir);
diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index e388146..0caf1e3 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -58,6 +58,12 @@ int v9fs_co_open(V9fsState *s, V9fsFidState *fidp, int flags)
err = 0;
}
});
+ if (!err) {
+ total_open_fd++;
+ if (total_open_fd > open_fd_hw) {
+ v9fs_reclaim_fd(s);
+ }
+ }
return err;
}
@@ -79,15 +85,19 @@ int v9fs_co_open2(V9fsState *s, V9fsFidState *fidp, char *fullname, gid_t gid,
err = -errno;
}
});
+ if (!err) {
+ total_open_fd++;
+ if (total_open_fd > open_fd_hw) {
+ v9fs_reclaim_fd(s);
+ }
+ }
return err;
}
-int v9fs_co_close(V9fsState *s, V9fsFidState *fidp)
+int v9fs_co_close(V9fsState *s, int fd)
{
- int fd;
int err;
- fd = fidp->fs.fd;
v9fs_co_run_in_worker(
{
err = s->ops->close(&s->ctx, fd);
@@ -95,6 +105,9 @@ int v9fs_co_close(V9fsState *s, V9fsFidState *fidp)
err = -errno;
}
});
+ if (!err) {
+ total_open_fd--;
+ }
return err;
}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index 48defb7..b7be9b5 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -83,8 +83,8 @@ extern int v9fs_co_open2(V9fsState *, V9fsFidState *, char *, gid_t, int, int);
extern int v9fs_co_lsetxattr(V9fsState *, V9fsString *, V9fsString *,
void *, size_t, int);
extern int v9fs_co_lremovexattr(V9fsState *, V9fsString *, V9fsString *);
-extern int v9fs_co_closedir(V9fsState *, V9fsFidState *);
-extern int v9fs_co_close(V9fsState *, V9fsFidState *);
+extern int v9fs_co_closedir(V9fsState *, DIR *);
+extern int v9fs_co_close(V9fsState *, int);
extern int v9fs_co_fsync(V9fsState *, V9fsFidState *, int);
extern int v9fs_co_symlink(V9fsState *, V9fsFidState *, const char *,
const char *, gid_t);
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 7811103..70629b2 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -171,6 +171,7 @@ static PCIDeviceInfo virtio_9p_info = {
static void virtio_9p_register_devices(void)
{
pci_qdev_register(&virtio_9p_info);
+ virtio_9p_set_fd_limit();
}
device_init(virtio_9p_register_devices)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index f3127a5..21e07fb 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -22,6 +22,9 @@
#include "virtio-9p-coth.h"
int debug_9p_pdu;
+int open_fd_hw;
+int total_open_fd;
+static int open_fd_rc;
enum {
Oread = 0x00,
@@ -234,11 +237,40 @@ static size_t v9fs_string_size(V9fsString *str)
static V9fsFidState *get_fid(V9fsState *s, int32_t fid)
{
+ int err;
V9fsFidState *f;
for (f = s->fid_list; f; f = f->next) {
if (f->fid == fid && !f->clunked) {
+ /*
+ * Update the fid ref upfront so that
+ * we don't get reclaimed when we yield
+ * in open later.
+ */
f->ref++;
+
+ /*
+ * check whether we need to reopen the
+ * file. We might have closed the fd
+ * while trying to free up some file
+ * descriptors.
+ */
+ if (f->fid_type == P9_FID_FILE) {
+ if (f->fs.fd == -1) {
+ do {
+ err = v9fs_co_open(s, f, f->open_flags);
+ } while (err == -EINTR);
+ if (err < 0) {
+ f->ref--;
+ return NULL;
+ }
+ }
+ }
+ /*
+ * Mark the fid as referenced so that the LRU
+ * reclaim won't close the file descriptor
+ */
+ f->flags |= FID_REFERENCED;
return f;
}
}
@@ -259,6 +291,11 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
f->fid = fid;
f->fid_type = P9_FID_NONE;
f->ref = 1;
+ /*
+ * Mark the fid as referenced so that the LRU
+ * reclaim won't close the file descriptor
+ */
+ f->flags |= FID_REFERENCED;
f->next = s->fid_list;
s->fid_list = f;
@@ -304,9 +341,12 @@ static int release_fid(V9fsState *s, V9fsFidState *fidp)
int retval = 0;
if (fidp->fid_type == P9_FID_FILE) {
- retval = v9fs_co_close(s, fidp);
+ /* If we reclaimed the fd no need to close */
+ if (fidp->fs.fd != -1) {
+ retval = v9fs_co_close(s, fidp->fs.fd);
+ }
} else if (fidp->fid_type == P9_FID_DIR) {
- retval = v9fs_co_closedir(s, fidp);
+ retval = v9fs_co_closedir(s, fidp->fs.dir);
} else if (fidp->fid_type == P9_FID_XATTR) {
retval = v9fs_xattr_fid_clunk(s, fidp);
}
@@ -319,7 +359,10 @@ static void put_fid(V9fsState *s, V9fsFidState *fidp)
{
BUG_ON(!fidp->ref);
fidp->ref--;
- if (!fidp->ref && fidp->clunked) {
+ /*
+ * Don't free the fid if it is in reclaim list
+ */
+ if (!fidp->ref && fidp->clunked && !fidp->rclm_lst) {
release_fid(s, fidp);
}
}
@@ -343,6 +386,105 @@ static int free_fid(V9fsState *s, int32_t fid)
return 0;
}
+void v9fs_reclaim_fd(V9fsState *s)
+{
+ int reclaim_count = 0;
+ V9fsFidState *f, head_fid, *reclaim_list = NULL;
+
+ /*
+ * We don't want multiple coroutine to do reclaim
+ */
+ if (s->reclaim_in_prgs) {
+ return;
+ }
+ s->reclaim_in_prgs = 1;
+
+ head_fid.next = s->fid_list;
+ for (f = s->fid_list; f; f = f->next) {
+ /*
+ * Unlink fids cannot be reclaimed. Check
+ * for them and skip them. Also skip fids
+ * currently being operated on.
+ */
+ if (f->ref || f->flags & FID_NON_RECLAIMABLE) {
+ continue;
+ }
+ /*
+ * if it is a recently referenced fid
+ * we leave the fid untouched and clear the
+ * reference bit. We come back to it later
+ * in the next iteration. (a simple LRU without
+ * moving list elements around)
+ */
+ if (f->flags & FID_REFERENCED) {
+ f->flags &= ~FID_REFERENCED;
+ continue;
+ }
+ /*
+ * Add fids to reclaim list.
+ */
+ if (f->fid_type == P9_FID_FILE) {
+ if (f->fs.fd != -1) {
+ f->rclm_lst = reclaim_list;
+ reclaim_list = f;
+ f->fs_reclaim.fd = f->fs.fd;
+ f->fs.fd = -1;
+ reclaim_count++;
+ }
+ }
+ if (reclaim_count >= open_fd_rc) {
+ break;
+ }
+ }
+ /*
+ * Now close the fid in reclaim list. Free them if they
+ * are already clunked.
+ */
+ while (reclaim_list) {
+ f = reclaim_list;
+ reclaim_list = f->rclm_lst;
+ if (f->clunked) {
+ release_fid(s, f);
+ } else {
+ if (f->fid_type == P9_FID_FILE) {
+ v9fs_co_close(s, f->fs_reclaim.fd);
+ }
+ f->rclm_lst = NULL;
+ }
+ }
+ s->reclaim_in_prgs = 0;
+}
+
+static int v9fs_mark_fids_unreclaim(V9fsState *s, V9fsString *str)
+{
+ int err;
+ V9fsFidState *fidp, head_fid;
+
+ head_fid.next = s->fid_list;
+ for (fidp = s->fid_list; fidp; fidp = fidp->next) {
+ if (!strcmp(fidp->path.data, str->data)) {
+ /* Mark the fid non reclaimable. */
+ fidp->flags |= FID_NON_RECLAIMABLE;
+ /* reopen the file if already closed */
+ if (fidp->fs.fd == -1) {
+ do {
+ err = v9fs_co_open(s, fidp, fidp->open_flags);
+ } while (err == -EINTR);
+ if (err < 0) {
+ return -1;
+ }
+ /*
+ * Go back to head of fid list because
+ * the list could have got updated when
+ * switched to the worker thread
+ */
+ fidp = &head_fid;
+ }
+ }
+ }
+ return 0;
+}
+
#define P9_QID_TYPE_DIR 0x80
#define P9_QID_TYPE_SYMLINK 0x02
@@ -1388,6 +1530,14 @@ static void v9fs_open(void *opaque)
goto out;
}
fidp->fid_type = P9_FID_FILE;
+ fidp->open_flags = flags;
+ if (flags & O_EXCL) {
+ /*
+ * We let the host file system do O_EXCL check
+ * We should not reclaim such fd
+ */
+ fidp->flags |= FID_NON_RECLAIMABLE;
+ }
iounit = get_iounit(s, &fidp->path);
offset += pdu_marshal(pdu, offset, "Qd", &qid, iounit);
err = offset;
@@ -1432,6 +1582,14 @@ static void v9fs_lcreate(void *opaque)
goto out;
}
fidp->fid_type = P9_FID_FILE;
+ fidp->open_flags = flags;
+ if (flags & O_EXCL) {
+ /*
+ * We let the host file system do O_EXCL check
+ * We should not reclaim such fd
+ */
+ fidp->flags |= FID_NON_RECLAIMABLE;
+ }
iounit = get_iounit(pdu->s, &fullname);
err = v9fs_co_lstat(pdu->s, &fullname, &stbuf);
@@ -1993,6 +2151,14 @@ static void v9fs_create(void *opaque)
goto out;
}
fidp->fid_type = P9_FID_FILE;
+ fidp->open_flags = omode_to_uflags(mode);
+ if (fidp->open_flags & O_EXCL) {
+ /*
+ * We let the host file system do O_EXCL check
+ * We should not reclaim such fd
+ */
+ fidp->flags |= FID_NON_RECLAIMABLE;
+ }
}
err = v9fs_co_lstat(pdu->s, &fullname, &stbuf);
if (err < 0) {
@@ -2126,11 +2292,19 @@ static void v9fs_remove(void *opaque)
err = -EINVAL;
goto out_nofid;
}
+ /*
+ * IF the file is unlinked, we cannot reopen
+ * the file later. So don't reclaim fd
+ */
+ err = v9fs_mark_fids_unreclaim(pdu->s, &fidp->path);
+ if (err < 0) {
+ goto out_err;
+ }
err = v9fs_co_remove(pdu->s, &fidp->path);
if (!err) {
err = offset;
}
-
+out_err:
/* For TREMOVE we need to clunk the fid even on failed remove */
free_fid(pdu->s, fidp->fid);
put_fid(pdu->s, fidp);
@@ -2826,3 +3000,14 @@ void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
}
free_pdu(s, pdu);
}
+
+void virtio_9p_set_fd_limit(void)
+{
+ struct rlimit rlim;
+ if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) {
+ fprintf(stderr, "Failed to get the resource limit\n");
+ exit(1);
+ }
+ open_fd_hw = rlim.rlim_cur - MIN(400, rlim.rlim_cur/3);
+ open_fd_rc = rlim.rlim_cur/2;
+}
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index e16e5f4..36aa4a5 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -5,6 +5,7 @@
#include <dirent.h>
#include <sys/time.h>
#include <utime.h>
+#include <sys/resource.h>
#include "hw/virtio.h"
#include "fsdev/file-op-9p.h"
@@ -101,6 +102,9 @@ enum p9_proto_version {
#define P9_NOTAG (u16)(~0)
#define P9_NOFID (u32)(~0)
#define P9_MAXWELEM 16
+
+#define FID_REFERENCED 0x1
+#define FID_NON_RECLAIMABLE 0x2
static inline const char *rpath(FsContext *ctx, const char *path, char *buffer)
{
snprintf(buffer, PATH_MAX, "%s/%s", ctx->fs_root, path);
@@ -198,14 +202,21 @@ struct V9fsFidState
int32_t fid;
V9fsString path;
union {
- int fd;
- DIR *dir;
- V9fsXattr xattr;
+ int fd;
+ DIR *dir;
+ V9fsXattr xattr;
} fs;
+ union {
+ int fd;
+ DIR *dir;
+ } fs_reclaim;
+ int flags;
+ int open_flags;
uid_t uid;
int ref;
int clunked;
V9fsFidState *next;
+ V9fsFidState *rclm_lst;
};
typedef struct V9fsState
@@ -222,6 +233,7 @@ typedef struct V9fsState
size_t config_size;
enum p9_proto_version proto_version;
int32_t msize;
+ int reclaim_in_prgs;
} V9fsState;
typedef struct V9fsStatState {
@@ -354,6 +366,9 @@ typedef struct V9fsGetlock
V9fsString client_id;
} V9fsGetlock;
+extern int open_fd_hw;
+extern int total_open_fd;
+
size_t pdu_packunpack(void *addr, struct iovec *sg, int sg_count,
size_t offset, size_t size, int pack);
@@ -364,4 +379,6 @@ static inline size_t do_pdu_unpack(void *dst, struct iovec *sg, int sg_count,
}
extern void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq);
+extern void virtio_9p_set_fd_limit(void);
+extern void v9fs_reclaim_fd(V9fsState *s);
#endif
--
1.7.4.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH 3/6] hw/9pfs: Add file descriptor reclaim support
2011-06-06 17:16 ` [Qemu-devel] [PATCH 3/6] hw/9pfs: Add file descriptor reclaim support Aneesh Kumar K.V
@ 2011-06-10 1:27 ` Venkateswararao Jujjuri
0 siblings, 0 replies; 13+ messages in thread
From: Venkateswararao Jujjuri @ 2011-06-10 1:27 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: aliguori, qemu-devel
On 06/06/2011 10:16 AM, Aneesh Kumar K.V wrote:
> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
Minor comments below; I think this looks good.
Reviewed-by : Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
> hw/9pfs/codir.c | 4 +-
> hw/9pfs/cofile.c | 19 ++++-
> hw/9pfs/virtio-9p-coth.h | 4 +-
> hw/9pfs/virtio-9p-device.c | 1 +
> hw/9pfs/virtio-9p.c | 193 +++++++++++++++++++++++++++++++++++++++++++-
> hw/9pfs/virtio-9p.h | 23 +++++-
> 6 files changed, 229 insertions(+), 15 deletions(-)
>
> diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
> index 783d279..3347038 100644
> --- a/hw/9pfs/codir.c
> +++ b/hw/9pfs/codir.c
> @@ -101,12 +101,10 @@ int v9fs_co_opendir(V9fsState *s, V9fsFidState *fidp)
> return err;
> }
>
> -int v9fs_co_closedir(V9fsState *s, V9fsFidState *fidp)
> +int v9fs_co_closedir(V9fsState *s, DIR *dir)
> {
> int err;
> - DIR *dir;
>
> - dir = fidp->fs.dir;
> v9fs_co_run_in_worker(
> {
> err = s->ops->closedir(&s->ctx, dir);
> diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
> index e388146..0caf1e3 100644
> --- a/hw/9pfs/cofile.c
> +++ b/hw/9pfs/cofile.c
> @@ -58,6 +58,12 @@ int v9fs_co_open(V9fsState *s, V9fsFidState *fidp, int flags)
> err = 0;
> }
> });
> + if (!err) {
> + total_open_fd++;
> + if (total_open_fd> open_fd_hw) {
> + v9fs_reclaim_fd(s);
> + }
> + }
> return err;
> }
>
> @@ -79,15 +85,19 @@ int v9fs_co_open2(V9fsState *s, V9fsFidState *fidp, char *fullname, gid_t gid,
> err = -errno;
> }
> });
> + if (!err) {
> + total_open_fd++;
> + if (total_open_fd> open_fd_hw) {
> + v9fs_reclaim_fd(s);
> + }
> + }
> return err;
> }
>
> -int v9fs_co_close(V9fsState *s, V9fsFidState *fidp)
> +int v9fs_co_close(V9fsState *s, int fd)
> {
> - int fd;
> int err;
>
> - fd = fidp->fs.fd;
> v9fs_co_run_in_worker(
> {
> err = s->ops->close(&s->ctx, fd);
> @@ -95,6 +105,9 @@ int v9fs_co_close(V9fsState *s, V9fsFidState *fidp)
> err = -errno;
> }
> });
> + if (!err) {
> + total_open_fd--;
> + }
> return err;
> }
>
> diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
> index 48defb7..b7be9b5 100644
> --- a/hw/9pfs/virtio-9p-coth.h
> +++ b/hw/9pfs/virtio-9p-coth.h
> @@ -83,8 +83,8 @@ extern int v9fs_co_open2(V9fsState *, V9fsFidState *, char *, gid_t, int, int);
> extern int v9fs_co_lsetxattr(V9fsState *, V9fsString *, V9fsString *,
> void *, size_t, int);
> extern int v9fs_co_lremovexattr(V9fsState *, V9fsString *, V9fsString *);
> -extern int v9fs_co_closedir(V9fsState *, V9fsFidState *);
> -extern int v9fs_co_close(V9fsState *, V9fsFidState *);
> +extern int v9fs_co_closedir(V9fsState *, DIR *);
> +extern int v9fs_co_close(V9fsState *, int);
> extern int v9fs_co_fsync(V9fsState *, V9fsFidState *, int);
> extern int v9fs_co_symlink(V9fsState *, V9fsFidState *, const char *,
> const char *, gid_t);
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 7811103..70629b2 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -171,6 +171,7 @@ static PCIDeviceInfo virtio_9p_info = {
> static void virtio_9p_register_devices(void)
> {
> pci_qdev_register(&virtio_9p_info);
> + virtio_9p_set_fd_limit();
> }
>
> device_init(virtio_9p_register_devices)
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index f3127a5..21e07fb 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -22,6 +22,9 @@
> #include "virtio-9p-coth.h"
>
> int debug_9p_pdu;
> +int open_fd_hw;
> +int total_open_fd;
> +static int open_fd_rc;
>
> enum {
> Oread = 0x00,
> @@ -234,11 +237,40 @@ static size_t v9fs_string_size(V9fsString *str)
>
> static V9fsFidState *get_fid(V9fsState *s, int32_t fid)
> {
> + int err;
> V9fsFidState *f;
>
> for (f = s->fid_list; f; f = f->next) {
> if (f->fid == fid&& !f->clunked) {
> + /*
> + * Update the fid ref upfront so that
> + * we don't get reclaimed when we yield
> + * in open later.
> + */
> f->ref++;
> +
> + /*
> + * check whether we need to reopen the
> + * file. We might have closed the fd
> + * while trying to free up some file
> + * descriptors.
> + */
> + if (f->fid_type == P9_FID_FILE) {
> + if (f->fs.fd == -1) {
> + do {
> + err = v9fs_co_open(s, f, f->open_flags);
> + } while (err == -EINTR);
> + if (err< 0) {
> + f->ref--;
> + return NULL;
> + }
> + }
> + }
> + /*
> + * Mark the fid as referenced so that the LRU
> + * reclaim won't close the file descriptor
> + */
> + f->flags |= FID_REFERENCED;
> return f;
> }
> }
> @@ -259,6 +291,11 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
> f->fid = fid;
> f->fid_type = P9_FID_NONE;
> f->ref = 1;
> + /*
> + * Mark the fid as referenced so that the LRU
> + * reclaim won't close the file descriptor
> + */
> + f->flags |= FID_REFERENCED;
> f->next = s->fid_list;
> s->fid_list = f;
>
> @@ -304,9 +341,12 @@ static int release_fid(V9fsState *s, V9fsFidState *fidp)
> int retval = 0;
>
> if (fidp->fid_type == P9_FID_FILE) {
> - retval = v9fs_co_close(s, fidp);
> + /* If we reclaimed the fd no need to close */
> + if (fidp->fs.fd != -1) {
> + retval = v9fs_co_close(s, fidp->fs.fd);
> + }
> } else if (fidp->fid_type == P9_FID_DIR) {
> - retval = v9fs_co_closedir(s, fidp);
> + retval = v9fs_co_closedir(s, fidp->fs.dir);
> } else if (fidp->fid_type == P9_FID_XATTR) {
> retval = v9fs_xattr_fid_clunk(s, fidp);
> }
> @@ -319,7 +359,10 @@ static void put_fid(V9fsState *s, V9fsFidState *fidp)
> {
> BUG_ON(!fidp->ref);
> fidp->ref--;
> - if (!fidp->ref&& fidp->clunked) {
> + /*
> + * Don't free the fid if it is in reclaim list
> + */
> + if (!fidp->ref&& fidp->clunked&& !fidp->rclm_lst) {
> release_fid(s, fidp);
> }
> }
> @@ -343,6 +386,105 @@ static int free_fid(V9fsState *s, int32_t fid)
> return 0;
> }
>
> +void v9fs_reclaim_fd(V9fsState *s)
> +{
> + int reclaim_count = 0;
> + V9fsFidState *f, head_fid, *reclaim_list = NULL;
> +
> + /*
> + * We don't want multiple coroutine to do reclaim
> + */
> + if (s->reclaim_in_prgs) {
> + return;
> + }
> + s->reclaim_in_prgs = 1;
> +
> + head_fid.next = s->fid_list;
> + for (f = s->fid_list; f; f = f->next) {
> + /*
> + * Unlink fids cannot be reclaimed. Check
> + * for them and skip them. Also skip fids
> + * currently being operated on.
> + */
> + if (f->ref || f->flags& FID_NON_RECLAIMABLE) {
> + continue;
> + }
> + /*
> + * if it is a recently referenced fid
> + * we leave the fid untouched and clear the
> + * reference bit. We come back to it later
> + * in the next iteration. (a simple LRU without
> + * moving list elements around)
> + */
> + if (f->flags& FID_REFERENCED) {
> + f->flags&= ~FID_REFERENCED;
> + continue;
> + }
> + /*
> + * Add fids to reclaim list.
> + */
> + if (f->fid_type == P9_FID_FILE) {
> + if (f->fs.fd != -1) {
> + f->rclm_lst = reclaim_list;
> + reclaim_list = f;
> + f->fs_reclaim.fd = f->fs.fd;
> + f->fs.fd = -1;
> + reclaim_count++;
> + }
> + }
> + if (reclaim_count>= open_fd_rc) {
> + break;
> + }
> + }
> + /*
> + * Now close the fid in reclaim list. Free them if they
> + * are already clunked.
> + */
> + while (reclaim_list) {
> + f = reclaim_list;
> + reclaim_list = f->rclm_lst;
> + if (f->clunked) {
> + release_fid(s, f);
> + } else {
> + if (f->fid_type == P9_FID_FILE) {
> + v9fs_co_close(s, f->fs_reclaim.fd);
> + }
> + f->rclm_lst = NULL;
> + }
> + }
> + s->reclaim_in_prgs = 0;
> +}
> +
> +static int v9fs_mark_fids_unreclaim(V9fsState *s, V9fsString *str)
> +{
> + int err;
> + V9fsFidState *fidp, head_fid;
> +
> + head_fid.next = s->fid_list;
> + for (fidp = s->fid_list; fidp; fidp = fidp->next) {
> + if (!strcmp(fidp->path.data, str->data)) {
&& !( fidp->flags && FID_NON_RECLAIMABLE) To avoid unnecessarily
resetting the flag.
> + /* Mark the fid non reclaimable. */
> + fidp->flags |= FID_NON_RECLAIMABLE;
> + /* reopen the file if already closed */
> + if (fidp->fs.fd == -1) {
> + do {
> + err = v9fs_co_open(s, fidp, fidp->open_flags);
> + } while (err == -EINTR);
> + if (err< 0) {
> + return -1;
> + }
> + /*
> + * Go back to head of fid list because
> + * the list could have got updated when
> + * switched to the worker thread
> + */
> + fidp =&head_fid;
> + }
> + }
> + }
> + return 0;
> +}
> +
> #define P9_QID_TYPE_DIR 0x80
> #define P9_QID_TYPE_SYMLINK 0x02
>
> @@ -1388,6 +1530,14 @@ static void v9fs_open(void *opaque)
> goto out;
> }
> fidp->fid_type = P9_FID_FILE;
> + fidp->open_flags = flags;
> + if (flags& O_EXCL) {
> + /*
> + * We let the host file system do O_EXCL check
> + * We should not reclaim such fd
> + */
> + fidp->flags |= FID_NON_RECLAIMABLE;
> + }
> iounit = get_iounit(s,&fidp->path);
> offset += pdu_marshal(pdu, offset, "Qd",&qid, iounit);
> err = offset;
> @@ -1432,6 +1582,14 @@ static void v9fs_lcreate(void *opaque)
> goto out;
> }
> fidp->fid_type = P9_FID_FILE;
> + fidp->open_flags = flags;
> + if (flags& O_EXCL) {
> + /*
> + * We let the host file system do O_EXCL check
> + * We should not reclaim such fd
> + */
> + fidp->flags |= FID_NON_RECLAIMABLE;
> + }
> iounit = get_iounit(pdu->s,&fullname);
>
> err = v9fs_co_lstat(pdu->s,&fullname,&stbuf);
> @@ -1993,6 +2151,14 @@ static void v9fs_create(void *opaque)
> goto out;
> }
> fidp->fid_type = P9_FID_FILE;
> + fidp->open_flags = omode_to_uflags(mode);
> + if (fidp->open_flags& O_EXCL) {
> + /*
> + * We let the host file system do O_EXCL check
> + * We should not reclaim such fd
> + */
> + fidp->flags |= FID_NON_RECLAIMABLE;
> + }
> }
> err = v9fs_co_lstat(pdu->s,&fullname,&stbuf);
> if (err< 0) {
> @@ -2126,11 +2292,19 @@ static void v9fs_remove(void *opaque)
> err = -EINVAL;
> goto out_nofid;
> }
> + /*
> + * IF the file is unlinked, we cannot reopen
> + * the file later. So don't reclaim fd
> + */
> + err = v9fs_mark_fids_unreclaim(pdu->s,&fidp->path);
> + if (err< 0) {
> + goto out_err;
> + }
> err = v9fs_co_remove(pdu->s,&fidp->path);
> if (!err) {
> err = offset;
> }
> -
> +out_err:
> /* For TREMOVE we need to clunk the fid even on failed remove */
> free_fid(pdu->s, fidp->fid);
> put_fid(pdu->s, fidp);
> @@ -2826,3 +3000,14 @@ void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
> }
> free_pdu(s, pdu);
> }
> +
> +void virtio_9p_set_fd_limit(void)
> +{
> + struct rlimit rlim;
> + if (getrlimit(RLIMIT_NOFILE,&rlim)< 0) {
> + fprintf(stderr, "Failed to get the resource limit\n");
> + exit(1);
> + }
> + open_fd_hw = rlim.rlim_cur - MIN(400, rlim.rlim_cur/3);
> + open_fd_rc = rlim.rlim_cur/2;
Why not call it LW?
> +}
> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> index e16e5f4..36aa4a5 100644
> --- a/hw/9pfs/virtio-9p.h
> +++ b/hw/9pfs/virtio-9p.h
> @@ -5,6 +5,7 @@
> #include<dirent.h>
> #include<sys/time.h>
> #include<utime.h>
> +#include<sys/resource.h>
> #include "hw/virtio.h"
> #include "fsdev/file-op-9p.h"
>
> @@ -101,6 +102,9 @@ enum p9_proto_version {
> #define P9_NOTAG (u16)(~0)
> #define P9_NOFID (u32)(~0)
> #define P9_MAXWELEM 16
> +
> +#define FID_REFERENCED 0x1
> +#define FID_NON_RECLAIMABLE 0x2
> static inline const char *rpath(FsContext *ctx, const char *path, char *buffer)
> {
> snprintf(buffer, PATH_MAX, "%s/%s", ctx->fs_root, path);
> @@ -198,14 +202,21 @@ struct V9fsFidState
> int32_t fid;
> V9fsString path;
> union {
> - int fd;
> - DIR *dir;
> - V9fsXattr xattr;
> + int fd;
> + DIR *dir;
> + V9fsXattr xattr;
> } fs;
> + union {
> + int fd;
> + DIR *dir;
> + } fs_reclaim;
> + int flags;
> + int open_flags;
> uid_t uid;
> int ref;
> int clunked;
> V9fsFidState *next;
> + V9fsFidState *rclm_lst;
> };
>
> typedef struct V9fsState
> @@ -222,6 +233,7 @@ typedef struct V9fsState
> size_t config_size;
> enum p9_proto_version proto_version;
> int32_t msize;
> + int reclaim_in_prgs;
> } V9fsState;
>
> typedef struct V9fsStatState {
> @@ -354,6 +366,9 @@ typedef struct V9fsGetlock
> V9fsString client_id;
> } V9fsGetlock;
>
> +extern int open_fd_hw;
> +extern int total_open_fd;
> +
> size_t pdu_packunpack(void *addr, struct iovec *sg, int sg_count,
> size_t offset, size_t size, int pack);
>
> @@ -364,4 +379,6 @@ static inline size_t do_pdu_unpack(void *dst, struct iovec *sg, int sg_count,
> }
>
> extern void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq);
> +extern void virtio_9p_set_fd_limit(void);
> +extern void v9fs_reclaim_fd(V9fsState *s);
> #endif
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 4/6] hw/9pfs: init fid list properly
2011-06-06 17:16 [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid Aneesh Kumar K.V
2011-06-06 17:16 ` [Qemu-devel] [PATCH 2/6] hw/9pfs: Don't free fid in clunk Aneesh Kumar K.V
2011-06-06 17:16 ` [Qemu-devel] [PATCH 3/6] hw/9pfs: Add file descriptor reclaim support Aneesh Kumar K.V
@ 2011-06-06 17:16 ` Aneesh Kumar K.V
2011-06-06 17:16 ` [Qemu-devel] [PATCH 5/6] hw/9pfs: Use v9fs_do_close instead of close Aneesh Kumar K.V
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Aneesh Kumar K.V @ 2011-06-06 17:16 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Aneesh Kumar K.V
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
hw/9pfs/virtio-9p-device.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 70629b2..4bec8bf 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -130,6 +130,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
s->config_size = sizeof(struct virtio_9p_config) +
s->tag_len;
s->vdev.get_config = virtio_9p_get_config;
+ s->fid_list = NULL;
if (v9fs_init_worker_threads() < 0) {
fprintf(stderr, "worker thread initialization failed\n");
--
1.7.4.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 5/6] hw/9pfs: Use v9fs_do_close instead of close
2011-06-06 17:16 [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid Aneesh Kumar K.V
` (2 preceding siblings ...)
2011-06-06 17:16 ` [Qemu-devel] [PATCH 4/6] hw/9pfs: init fid list properly Aneesh Kumar K.V
@ 2011-06-06 17:16 ` Aneesh Kumar K.V
2011-06-10 1:32 ` Venkateswararao Jujjuri
2011-06-06 17:16 ` [Qemu-devel] [PATCH 6/6] hw/9pfs: Add directory reclaim support Aneesh Kumar K.V
2011-06-09 23:10 ` [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid Venkateswararao Jujjuri
5 siblings, 1 reply; 13+ messages in thread
From: Aneesh Kumar K.V @ 2011-06-06 17:16 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Aneesh Kumar K.V
we should use the local abstraction instead of
directly calling close.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
hw/9pfs/virtio-9p.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 21e07fb..d322814 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1596,7 +1596,7 @@ static void v9fs_lcreate(void *opaque)
if (err < 0) {
fidp->fid_type = P9_FID_NONE;
if (fidp->fs.fd > 0) {
- close(fidp->fs.fd);
+ v9fs_co_close(pdu->s, fidp->fs.fd);
}
goto out;
}
@@ -2164,7 +2164,7 @@ static void v9fs_create(void *opaque)
if (err < 0) {
fidp->fid_type = P9_FID_NONE;
if (fidp->fs.fd) {
- close(fidp->fs.fd);
+ v9fs_co_close(pdu->s, fidp->fs.fd);
}
goto out;
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH 5/6] hw/9pfs: Use v9fs_do_close instead of close
2011-06-06 17:16 ` [Qemu-devel] [PATCH 5/6] hw/9pfs: Use v9fs_do_close instead of close Aneesh Kumar K.V
@ 2011-06-10 1:32 ` Venkateswararao Jujjuri
0 siblings, 0 replies; 13+ messages in thread
From: Venkateswararao Jujjuri @ 2011-06-10 1:32 UTC (permalink / raw)
To: qemu-devel
On 06/06/2011 10:16 AM, Aneesh Kumar K.V wrote:
> we should use the local abstraction instead of
> directly calling close.
>
Let us fold this also into our coroutine patches.
- JV
> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
> ---
> hw/9pfs/virtio-9p.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index 21e07fb..d322814 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -1596,7 +1596,7 @@ static void v9fs_lcreate(void *opaque)
> if (err< 0) {
> fidp->fid_type = P9_FID_NONE;
> if (fidp->fs.fd> 0) {
> - close(fidp->fs.fd);
> + v9fs_co_close(pdu->s, fidp->fs.fd);
> }
> goto out;
> }
> @@ -2164,7 +2164,7 @@ static void v9fs_create(void *opaque)
> if (err< 0) {
> fidp->fid_type = P9_FID_NONE;
> if (fidp->fs.fd) {
> - close(fidp->fs.fd);
> + v9fs_co_close(pdu->s, fidp->fs.fd);
> }
> goto out;
> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 6/6] hw/9pfs: Add directory reclaim support
2011-06-06 17:16 [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid Aneesh Kumar K.V
` (3 preceding siblings ...)
2011-06-06 17:16 ` [Qemu-devel] [PATCH 5/6] hw/9pfs: Use v9fs_do_close instead of close Aneesh Kumar K.V
@ 2011-06-06 17:16 ` Aneesh Kumar K.V
2011-06-10 1:36 ` Venkateswararao Jujjuri
2011-06-09 23:10 ` [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid Venkateswararao Jujjuri
5 siblings, 1 reply; 13+ messages in thread
From: Aneesh Kumar K.V @ 2011-06-06 17:16 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Aneesh Kumar K.V
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
hw/9pfs/codir.c | 9 +++++++++
hw/9pfs/virtio-9p.c | 26 ++++++++++++++++++++++++--
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
index 3347038..2c26f72 100644
--- a/hw/9pfs/codir.c
+++ b/hw/9pfs/codir.c
@@ -98,6 +98,12 @@ int v9fs_co_opendir(V9fsState *s, V9fsFidState *fidp)
err = 0;
}
});
+ if (!err) {
+ total_open_fd++;
+ if (total_open_fd > open_fd_hw) {
+ v9fs_reclaim_fd(s);
+ }
+ }
return err;
}
@@ -112,5 +118,8 @@ int v9fs_co_closedir(V9fsState *s, DIR *dir)
err = -errno;
}
});
+ if (!err) {
+ total_open_fd--;
+ }
return err;
}
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index d322814..55aca93 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -265,7 +265,17 @@ static V9fsFidState *get_fid(V9fsState *s, int32_t fid)
return NULL;
}
}
- }
+ } else if (f->fid_type == P9_FID_DIR) {
+ if (f->fs.dir == NULL) {
+ do {
+ err = v9fs_co_opendir(s, f);
+ } while (err == -EINTR);
+ if (err < 0) {
+ f->ref--;
+ return NULL;
+ }
+ }
+ }
/*
* Mark the fid as referenced so that the LRU
* reclaim won't close the file descriptor
@@ -346,7 +356,9 @@ static int release_fid(V9fsState *s, V9fsFidState *fidp)
retval = v9fs_co_close(s, fidp->fs.fd);
}
} else if (fidp->fid_type == P9_FID_DIR) {
- retval = v9fs_co_closedir(s, fidp->fs.dir);
+ if (fidp->fs.dir != NULL) {
+ retval = v9fs_co_closedir(s, fidp->fs.dir);
+ }
} else if (fidp->fid_type == P9_FID_XATTR) {
retval = v9fs_xattr_fid_clunk(s, fidp);
}
@@ -431,6 +443,14 @@ void v9fs_reclaim_fd(V9fsState *s)
f->fs.fd = -1;
reclaim_count++;
}
+ } else if (f->fid_type == P9_FID_DIR) {
+ if (f->fs.dir != NULL) {
+ f->rclm_lst = reclaim_list;
+ reclaim_list = f;
+ f->fs_reclaim.dir = f->fs.dir;
+ f->fs.dir = NULL;
+ reclaim_count++;
+ }
}
if (reclaim_count >= open_fd_rc) {
break;
@@ -448,6 +468,8 @@ void v9fs_reclaim_fd(V9fsState *s)
} else {
if (f->fid_type == P9_FID_FILE) {
v9fs_co_close(s, f->fs_reclaim.fd);
+ } else if (f->fid_type == P9_FID_DIR) {
+ v9fs_co_closedir(s, f->fs_reclaim.dir);
}
f->rclm_lst = NULL;
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH 6/6] hw/9pfs: Add directory reclaim support
2011-06-06 17:16 ` [Qemu-devel] [PATCH 6/6] hw/9pfs: Add directory reclaim support Aneesh Kumar K.V
@ 2011-06-10 1:36 ` Venkateswararao Jujjuri
0 siblings, 0 replies; 13+ messages in thread
From: Venkateswararao Jujjuri @ 2011-06-10 1:36 UTC (permalink / raw)
To: qemu-devel
On 06/06/2011 10:16 AM, Aneesh Kumar K.V wrote:
> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
Reviewed-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
> hw/9pfs/codir.c | 9 +++++++++
> hw/9pfs/virtio-9p.c | 26 ++++++++++++++++++++++++--
> 2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
> index 3347038..2c26f72 100644
> --- a/hw/9pfs/codir.c
> +++ b/hw/9pfs/codir.c
> @@ -98,6 +98,12 @@ int v9fs_co_opendir(V9fsState *s, V9fsFidState *fidp)
> err = 0;
> }
> });
> + if (!err) {
> + total_open_fd++;
> + if (total_open_fd> open_fd_hw) {
> + v9fs_reclaim_fd(s);
> + }
> + }
> return err;
> }
>
> @@ -112,5 +118,8 @@ int v9fs_co_closedir(V9fsState *s, DIR *dir)
> err = -errno;
> }
> });
> + if (!err) {
> + total_open_fd--;
> + }
> return err;
> }
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index d322814..55aca93 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -265,7 +265,17 @@ static V9fsFidState *get_fid(V9fsState *s, int32_t fid)
> return NULL;
> }
> }
> - }
> + } else if (f->fid_type == P9_FID_DIR) {
> + if (f->fs.dir == NULL) {
> + do {
> + err = v9fs_co_opendir(s, f);
> + } while (err == -EINTR);
> + if (err< 0) {
> + f->ref--;
> + return NULL;
> + }
> + }
> + }
> /*
> * Mark the fid as referenced so that the LRU
> * reclaim won't close the file descriptor
> @@ -346,7 +356,9 @@ static int release_fid(V9fsState *s, V9fsFidState *fidp)
> retval = v9fs_co_close(s, fidp->fs.fd);
> }
> } else if (fidp->fid_type == P9_FID_DIR) {
> - retval = v9fs_co_closedir(s, fidp->fs.dir);
> + if (fidp->fs.dir != NULL) {
> + retval = v9fs_co_closedir(s, fidp->fs.dir);
> + }
> } else if (fidp->fid_type == P9_FID_XATTR) {
> retval = v9fs_xattr_fid_clunk(s, fidp);
> }
> @@ -431,6 +443,14 @@ void v9fs_reclaim_fd(V9fsState *s)
> f->fs.fd = -1;
> reclaim_count++;
> }
> + } else if (f->fid_type == P9_FID_DIR) {
> + if (f->fs.dir != NULL) {
> + f->rclm_lst = reclaim_list;
> + reclaim_list = f;
> + f->fs_reclaim.dir = f->fs.dir;
> + f->fs.dir = NULL;
> + reclaim_count++;
> + }
> }
> if (reclaim_count>= open_fd_rc) {
> break;
> @@ -448,6 +468,8 @@ void v9fs_reclaim_fd(V9fsState *s)
> } else {
> if (f->fid_type == P9_FID_FILE) {
> v9fs_co_close(s, f->fs_reclaim.fd);
> + } else if (f->fid_type == P9_FID_DIR) {
> + v9fs_co_closedir(s, f->fs_reclaim.dir);
> }
> f->rclm_lst = NULL;
> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid
2011-06-06 17:16 [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid Aneesh Kumar K.V
` (4 preceding siblings ...)
2011-06-06 17:16 ` [Qemu-devel] [PATCH 6/6] hw/9pfs: Add directory reclaim support Aneesh Kumar K.V
@ 2011-06-09 23:10 ` Venkateswararao Jujjuri
2011-06-10 6:27 ` Aneesh Kumar K.V
5 siblings, 1 reply; 13+ messages in thread
From: Venkateswararao Jujjuri @ 2011-06-09 23:10 UTC (permalink / raw)
To: qemu-devel
On 06/06/2011 10:16 AM, Aneesh Kumar K.V wrote:
> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
Just one minor issue below; otherwise
Reviewed-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
> ---
> hw/9pfs/virtio-9p.c | 205 +++++++++++++++++++++++++++++++++++----------------
> hw/9pfs/virtio-9p.h | 7 ++
> 2 files changed, 147 insertions(+), 65 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index e2aa863..03d8664 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -232,12 +232,13 @@ static size_t v9fs_string_size(V9fsString *str)
> return str->size;
> }
>
> -static V9fsFidState *lookup_fid(V9fsState *s, int32_t fid)
> +static V9fsFidState *get_fid(V9fsState *s, int32_t fid)
> {
> V9fsFidState *f;
>
> for (f = s->fid_list; f; f = f->next) {
> if (f->fid == fid) {
> + f->ref++;
> return f;
> }
> }
> @@ -249,16 +250,16 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
> {
> V9fsFidState *f;
>
> - f = lookup_fid(s, fid);
> + f = get_fid(s, fid);
> if (f) {
> + f->ref--;
> return NULL;
> }
>
> f = qemu_mallocz(sizeof(V9fsFidState));
> -
> f->fid = fid;
> f->fid_type = P9_FID_NONE;
> -
> + f->ref = 1;
> f->next = s->fid_list;
> s->fid_list = f;
>
> @@ -1014,19 +1015,22 @@ static void v9fs_attach(void *opaque)
> fidp = alloc_fid(s, fid);
> if (fidp == NULL) {
> err = -EINVAL;
> - goto out;
> + goto out_nofid;
> }
> fidp->uid = n_uname;
> v9fs_string_sprintf(&fidp->path, "%s", "/");
> err = fid_to_qid(s, fidp,&qid);
> if (err< 0) {
> err = -EINVAL;
> + put_fid(fidp);
> free_fid(s, fid);
> - goto out;
> + goto out_nofid;
> }
> offset += pdu_marshal(pdu, offset, "Q",&qid);
> err = offset;
> -out:
> + put_fid(fidp);
> +
> +out_nofid:
> complete_pdu(s, pdu, err);
> v9fs_string_free(&uname);
> v9fs_string_free(&aname);
> @@ -1045,10 +1049,10 @@ static void v9fs_stat(void *opaque)
>
> pdu_unmarshal(pdu, offset, "d",&fid);
>
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> err = v9fs_co_lstat(s,&fidp->path,&stbuf);
> if (err< 0) {
> @@ -1062,6 +1066,8 @@ static void v9fs_stat(void *opaque)
> err = offset;
> v9fs_stat_free(&v9stat);
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(s, pdu, err);
> }
>
> @@ -1079,10 +1085,10 @@ static void v9fs_getattr(void *opaque)
>
> pdu_unmarshal(pdu, offset, "dq",&fid,&request_mask);
>
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> retval = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> /*
> * Currently we only support BASIC fields in stat, so there is no
> @@ -1096,6 +1102,8 @@ static void v9fs_getattr(void *opaque)
> retval = offset;
> retval += pdu_marshal(pdu, offset, "A",&v9stat_dotl);
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(s, pdu, retval);
> }
>
> @@ -1123,10 +1131,10 @@ static void v9fs_setattr(void *opaque)
>
> pdu_unmarshal(pdu, offset, "dI",&fid,&v9iattr);
>
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> err = -EINVAL;
> - goto out;
> + goto out_nofid;
> }
> if (v9iattr.valid& ATTR_MODE) {
> err = v9fs_co_chmod(s,&fidp->path, v9iattr.mode);
> @@ -1188,6 +1196,8 @@ static void v9fs_setattr(void *opaque)
> }
> err = offset;
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(s, pdu, err);
> }
>
> @@ -1214,7 +1224,7 @@ static void v9fs_walk(void *opaque)
> int32_t fid, newfid;
> V9fsString *wnames = NULL;
> V9fsFidState *fidp;
> - V9fsFidState *newfidp;
> + V9fsFidState *newfidp = NULL;;
> V9fsPDU *pdu = opaque;
> V9fsState *s = pdu->s;
>
> @@ -1231,12 +1241,12 @@ static void v9fs_walk(void *opaque)
>
> } else if (nwnames> P9_MAXWELEM) {
> err = -EINVAL;
> - goto out;
> + goto out_nofid;
> }
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> if (fid == newfid) {
> BUG_ON(fidp->fid_type != P9_FID_NONE);
> @@ -1269,7 +1279,9 @@ static void v9fs_walk(void *opaque)
> v9fs_string_copy(&newfidp->path,&path);
> err = v9fs_co_lstat(s,&newfidp->path,&stbuf);
> if (err< 0) {
> + put_fid(newfidp);
> free_fid(s, newfidp->fid);
> + newfidp = NULL;
> v9fs_string_free(&path);
> goto out;
> }
> @@ -1279,6 +1291,11 @@ static void v9fs_walk(void *opaque)
> }
> err = v9fs_walk_marshal(pdu, nwnames, qids);
> out:
> + put_fid(fidp);
> + if (newfidp) {
> + put_fid(newfidp);
> + }
> +out_nofid:
> complete_pdu(s, pdu, err);
> if (nwnames&& nwnames<= P9_MAXWELEM) {
> for (name_idx = 0; name_idx< nwnames; name_idx++) {
> @@ -1327,10 +1344,10 @@ static void v9fs_open(void *opaque)
> } else {
> pdu_unmarshal(pdu, offset, "db",&fid,&mode);
> }
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> BUG_ON(fidp->fid_type != P9_FID_NONE);
>
> @@ -1366,6 +1383,8 @@ static void v9fs_open(void *opaque)
> err = offset;
> }
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(s, pdu, err);
> }
>
> @@ -1388,10 +1407,10 @@ static void v9fs_lcreate(void *opaque)
> pdu_unmarshal(pdu, offset, "dsddd",&dfid,&name,&flags,
> &mode,&gid);
>
> - fidp = lookup_fid(pdu->s, dfid);
> + fidp = get_fid(pdu->s, dfid);
> if (fidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> v9fs_string_sprintf(&fullname, "%s/%s", fidp->path.data, name.data);
>
> @@ -1418,6 +1437,8 @@ static void v9fs_lcreate(void *opaque)
> offset += pdu_marshal(pdu, offset, "Qd",&qid, iounit);
> err = offset;
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(pdu->s, pdu, err);
> v9fs_string_free(&name);
> v9fs_string_free(&fullname);
> @@ -1434,7 +1455,7 @@ static void v9fs_fsync(void *opaque)
> V9fsState *s = pdu->s;
>
> pdu_unmarshal(pdu, offset, "dd",&fid,&datasync);
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> err = -ENOENT;
> goto out;
> @@ -1444,6 +1465,7 @@ static void v9fs_fsync(void *opaque)
> err = offset;
> }
> out:
> + put_fid(fidp);
It should be
put_fid(fidp);
out_nofid:
> complete_pdu(s, pdu, err);
> }
>
> @@ -1561,10 +1583,10 @@ static void v9fs_read(void *opaque)
>
> pdu_unmarshal(pdu, offset, "dqd",&fid,&off,&max_count);
>
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> err = -EINVAL;
> - goto out;
> + goto out_nofid;
> }
> if (fidp->fid_type == P9_FID_DIR) {
>
> @@ -1616,6 +1638,8 @@ static void v9fs_read(void *opaque)
> err = -EINVAL;
> }
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(s, pdu, err);
> }
>
> @@ -1700,8 +1724,12 @@ static void v9fs_readdir(void *opaque)
>
> pdu_unmarshal(pdu, offset, "dqd",&fid,&initial_offset,&max_count);
>
> - fidp = lookup_fid(s, fid);
> - if (fidp == NULL || !fidp->fs.dir) {
> + fidp = get_fid(s, fid);
> + if (fidp == NULL) {
> + retval = -EINVAL;
> + goto out_nofid;
> + }
> + if (!fidp->fs.dir) {
> retval = -EINVAL;
> goto out;
> }
> @@ -1719,6 +1747,8 @@ static void v9fs_readdir(void *opaque)
> retval += pdu_marshal(pdu, offset, "d", count);
> retval += count;
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(s, pdu, retval);
> }
>
> @@ -1784,10 +1814,10 @@ static void v9fs_write(void *opaque)
>
> pdu_unmarshal(pdu, offset, "dqdv",&fid,&off,&count, sg,&cnt);
>
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> err = -EINVAL;
> - goto out;
> + goto out_nofid;
> }
> if (fidp->fid_type == P9_FID_FILE) {
> if (fidp->fs.fd == -1) {
> @@ -1827,6 +1857,8 @@ static void v9fs_write(void *opaque)
> offset += pdu_marshal(pdu, offset, "d", total);
> err = offset;
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(s, pdu, err);
> }
>
> @@ -1851,10 +1883,10 @@ static void v9fs_create(void *opaque)
> pdu_unmarshal(pdu, offset, "dsdbs",&fid,&name,
> &perm,&mode,&extension);
>
> - fidp = lookup_fid(pdu->s, fid);
> + fidp = get_fid(pdu->s, fid);
> if (fidp == NULL) {
> err = -EINVAL;
> - goto out;
> + goto out_nofid;
> }
>
> v9fs_string_sprintf(&fullname, "%s/%s", fidp->path.data, name.data);
> @@ -1884,15 +1916,17 @@ static void v9fs_create(void *opaque)
> }
> } else if (perm& P9_STAT_MODE_LINK) {
> int32_t nfid = atoi(extension.data);
> - V9fsFidState *nfidp = lookup_fid(pdu->s, nfid);
> + V9fsFidState *nfidp = get_fid(pdu->s, nfid);
> if (nfidp == NULL) {
> err = -EINVAL;
> goto out;
> }
> err = v9fs_co_link(pdu->s,&nfidp->path,&fullname);
> if (err< 0) {
> + put_fid(nfidp);
> goto out;
> }
> + put_fid(nfidp);
> } else if (perm& P9_STAT_MODE_DEVICE) {
> char ctype;
> uint32_t major, minor;
> @@ -1956,6 +1990,8 @@ static void v9fs_create(void *opaque)
> err = offset;
>
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(pdu->s, pdu, err);
> v9fs_string_free(&name);
> v9fs_string_free(&extension);
> @@ -1980,10 +2016,10 @@ static void v9fs_symlink(void *opaque)
>
> pdu_unmarshal(pdu, offset, "dssd",&dfid,&name,&symname,&gid);
>
> - dfidp = lookup_fid(pdu->s, dfid);
> + dfidp = get_fid(pdu->s, dfid);
> if (dfidp == NULL) {
> err = -EINVAL;
> - goto out;
> + goto out_nofid;
> }
>
> v9fs_string_sprintf(&fullname, "%s/%s", dfidp->path.data, name.data);
> @@ -1999,6 +2035,8 @@ static void v9fs_symlink(void *opaque)
> offset += pdu_marshal(pdu, offset, "Q",&qid);
> err = offset;
> out:
> + put_fid(dfidp);
> +out_nofid:
> complete_pdu(pdu->s, pdu, err);
> v9fs_string_free(&name);
> v9fs_string_free(&symname);
> @@ -2028,13 +2066,13 @@ static void v9fs_link(void *opaque)
>
> pdu_unmarshal(pdu, offset, "dds",&dfid,&oldfid,&name);
>
> - dfidp = lookup_fid(s, dfid);
> + dfidp = get_fid(s, dfid);
> if (dfidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
>
> - oldfidp = lookup_fid(s, oldfid);
> + oldfidp = get_fid(s, oldfid);
> if (oldfidp == NULL) {
> err = -ENOENT;
> goto out;
> @@ -2048,6 +2086,8 @@ static void v9fs_link(void *opaque)
> v9fs_string_free(&fullname);
>
> out:
> + put_fid(dfidp);
> +out_nofid:
> v9fs_string_free(&name);
> complete_pdu(s, pdu, err);
> }
> @@ -2062,10 +2102,10 @@ static void v9fs_remove(void *opaque)
>
> pdu_unmarshal(pdu, offset, "d",&fid);
>
> - fidp = lookup_fid(pdu->s, fid);
> + fidp = get_fid(pdu->s, fid);
> if (fidp == NULL) {
> err = -EINVAL;
> - goto out;
> + goto out_nofid;
> }
> err = v9fs_co_remove(pdu->s,&fidp->path);
> if (!err) {
> @@ -2073,8 +2113,9 @@ static void v9fs_remove(void *opaque)
> }
>
> /* For TREMOVE we need to clunk the fid even on failed remove */
> + put_fid(fidp);
> free_fid(pdu->s, fidp->fid);
> -out:
> +out_nofid:
> complete_pdu(pdu->s, pdu, err);
> }
>
> @@ -2083,14 +2124,14 @@ static int v9fs_complete_rename(V9fsState *s, V9fsFidState *fidp,
> {
> char *end;
> int err = 0;
> + V9fsFidState *dirfidp = NULL;
> char *old_name, *new_name;
>
> if (newdirfid != -1) {
> - V9fsFidState *dirfidp;
> - dirfidp = lookup_fid(s, newdirfid);
> + dirfidp = get_fid(s, newdirfid);
> if (dirfidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> BUG_ON(dirfidp->fid_type != P9_FID_NONE);
>
> @@ -2143,6 +2184,10 @@ static int v9fs_complete_rename(V9fsState *s, V9fsFidState *fidp,
> v9fs_string_copy(&fidp->path, name);
> }
> out:
> + if (dirfidp) {
> + put_fid(dirfidp);
> + }
> +out_nofid:
> return err;
> }
>
> @@ -2159,10 +2204,10 @@ static void v9fs_rename(void *opaque)
>
> pdu_unmarshal(pdu, offset, "dds",&fid,&newdirfid,&name);
>
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> BUG_ON(fidp->fid_type != P9_FID_NONE);
>
> @@ -2171,6 +2216,8 @@ static void v9fs_rename(void *opaque)
> err = offset;
> }
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(s, pdu, err);
> v9fs_string_free(&name);
> }
> @@ -2189,10 +2236,10 @@ static void v9fs_wstat(void *opaque)
>
> pdu_unmarshal(pdu, offset, "dwS",&fid,&unused,&v9stat);
>
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> err = -EINVAL;
> - goto out;
> + goto out_nofid;
> }
> /* do we need to sync the file? */
> if (donttouch_stat(&v9stat)) {
> @@ -2258,6 +2305,8 @@ static void v9fs_wstat(void *opaque)
> }
> err = offset;
> out:
> + put_fid(fidp);
> +out_nofid:
> v9fs_stat_free(&v9stat);
> complete_pdu(s, pdu, err);
> }
> @@ -2318,10 +2367,10 @@ static void v9fs_statfs(void *opaque)
> V9fsState *s = pdu->s;
>
> pdu_unmarshal(pdu, offset, "d",&fid);
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> retval = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> retval = v9fs_co_statfs(s,&fidp->path,&stbuf);
> if (retval< 0) {
> @@ -2330,6 +2379,8 @@ static void v9fs_statfs(void *opaque)
> retval = offset;
> retval += v9fs_fill_statfs(s, pdu,&stbuf);
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(s, pdu, retval);
> return;
> }
> @@ -2355,10 +2406,10 @@ static void v9fs_mknod(void *opaque)
> pdu_unmarshal(pdu, offset, "dsdddd",&fid,&name,&mode,
> &major,&minor,&gid);
>
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> v9fs_string_sprintf(&fullname, "%s/%s", fidp->path.data, name.data);
> err = v9fs_co_mknod(s,&fullname, fidp->uid, gid,
> @@ -2374,6 +2425,8 @@ static void v9fs_mknod(void *opaque)
> err = offset;
> err += pdu_marshal(pdu, offset, "Q",&qid);
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(s, pdu, err);
> v9fs_string_free(&fullname);
> v9fs_string_free(&name);
> @@ -2407,12 +2460,12 @@ static void v9fs_lock(void *opaque)
> /* We support only block flag now (that too ignored currently) */
> if (flock->flags& ~P9_LOCK_FLAGS_BLOCK) {
> err = -EINVAL;
> - goto out;
> + goto out_nofid;
> }
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> err = v9fs_co_fstat(s, fidp->fs.fd,&stbuf);
> if (err< 0) {
> @@ -2420,6 +2473,8 @@ static void v9fs_lock(void *opaque)
> }
> status = P9_LOCK_SUCCESS;
> out:
> + put_fid(fidp);
> +out_nofid:
> err = offset;
> err += pdu_marshal(pdu, offset, "b", status);
> complete_pdu(s, pdu, err);
> @@ -2445,10 +2500,10 @@ static void v9fs_getlock(void *opaque)
> &glock->start,&glock->length,&glock->proc_id,
> &glock->client_id);
>
> - fidp = lookup_fid(s, fid);
> + fidp = get_fid(s, fid);
> if (fidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> err = v9fs_co_fstat(s, fidp->fs.fd,&stbuf);
> if (err< 0) {
> @@ -2460,6 +2515,8 @@ static void v9fs_getlock(void *opaque)
> &glock->client_id);
> err = offset;
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(s, pdu, err);
> qemu_free(glock);
> }
> @@ -2480,10 +2537,10 @@ static void v9fs_mkdir(void *opaque)
> v9fs_string_init(&fullname);
> pdu_unmarshal(pdu, offset, "dsdd",&fid,&name,&mode,&gid);
>
> - fidp = lookup_fid(pdu->s, fid);
> + fidp = get_fid(pdu->s, fid);
> if (fidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> v9fs_string_sprintf(&fullname, "%s/%s", fidp->path.data, name.data);
> err = v9fs_co_mkdir(pdu->s, fullname.data, mode, fidp->uid, gid);
> @@ -2498,6 +2555,8 @@ static void v9fs_mkdir(void *opaque)
> offset += pdu_marshal(pdu, offset, "Q",&qid);
> err = offset;
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(pdu->s, pdu, err);
> v9fs_string_free(&fullname);
> v9fs_string_free(&name);
> @@ -2511,15 +2570,15 @@ static void v9fs_xattrwalk(void *opaque)
> size_t offset = 7;
> int32_t fid, newfid;
> V9fsFidState *file_fidp;
> - V9fsFidState *xattr_fidp;
> + V9fsFidState *xattr_fidp = NULL;
> V9fsPDU *pdu = opaque;
> V9fsState *s = pdu->s;
>
> pdu_unmarshal(pdu, offset, "dds",&fid,&newfid,&name);
> - file_fidp = lookup_fid(s, fid);
> + file_fidp = get_fid(s, fid);
> if (file_fidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
> xattr_fidp = alloc_fid(s, newfid);
> if (xattr_fidp == NULL) {
> @@ -2534,7 +2593,9 @@ static void v9fs_xattrwalk(void *opaque)
> size = v9fs_co_llistxattr(s,&xattr_fidp->path, NULL, 0);
> if (size< 0) {
> err = size;
> + put_fid(xattr_fidp);
> free_fid(s, xattr_fidp->fid);
> + xattr_fidp = NULL;
> goto out;
> }
> /*
> @@ -2549,7 +2610,9 @@ static void v9fs_xattrwalk(void *opaque)
> xattr_fidp->fs.xattr.value,
> xattr_fidp->fs.xattr.len);
> if (err< 0) {
> + put_fid(xattr_fidp);
> free_fid(s, xattr_fidp->fid);
> + xattr_fidp = NULL;
> goto out;
> }
> }
> @@ -2564,7 +2627,9 @@ static void v9fs_xattrwalk(void *opaque)
> &name, NULL, 0);
> if (size< 0) {
> err = size;
> + put_fid(xattr_fidp);
> free_fid(s, xattr_fidp->fid);
> + xattr_fidp = NULL;
> goto out;
> }
> /*
> @@ -2579,7 +2644,9 @@ static void v9fs_xattrwalk(void *opaque)
> &name, xattr_fidp->fs.xattr.value,
> xattr_fidp->fs.xattr.len);
> if (err< 0) {
> + put_fid(xattr_fidp);
> free_fid(s, xattr_fidp->fid);
> + xattr_fidp = NULL;
> goto out;
> }
> }
> @@ -2587,6 +2654,11 @@ static void v9fs_xattrwalk(void *opaque)
> err = offset;
> }
> out:
> + put_fid(file_fidp);
> + if (xattr_fidp) {
> + put_fid(xattr_fidp);
> + }
> +out_nofid:
> complete_pdu(s, pdu, err);
> v9fs_string_free(&name);
> }
> @@ -2607,10 +2679,10 @@ static void v9fs_xattrcreate(void *opaque)
> pdu_unmarshal(pdu, offset, "dsqd",
> &fid,&name,&size,&flags);
>
> - file_fidp = lookup_fid(s, fid);
> + file_fidp = get_fid(s, fid);
> if (file_fidp == NULL) {
> err = -EINVAL;
> - goto out;
> + goto out_nofid;
> }
> /* Make the file fid point to xattr */
> xattr_fidp = file_fidp;
> @@ -2626,7 +2698,8 @@ static void v9fs_xattrcreate(void *opaque)
> xattr_fidp->fs.xattr.value = NULL;
> }
> err = offset;
> -out:
> + put_fid(file_fidp);
> +out_nofid:
> complete_pdu(s, pdu, err);
> v9fs_string_free(&name);
> }
> @@ -2641,10 +2714,10 @@ static void v9fs_readlink(void *opaque)
> V9fsFidState *fidp;
>
> pdu_unmarshal(pdu, offset, "d",&fid);
> - fidp = lookup_fid(pdu->s, fid);
> + fidp = get_fid(pdu->s, fid);
> if (fidp == NULL) {
> err = -ENOENT;
> - goto out;
> + goto out_nofid;
> }
>
> v9fs_string_init(&target);
> @@ -2656,6 +2729,8 @@ static void v9fs_readlink(void *opaque)
> err = offset;
> v9fs_string_free(&target);
> out:
> + put_fid(fidp);
> +out_nofid:
> complete_pdu(pdu->s, pdu, err);
> }
>
> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> index 1d8c1b1..ce93c20 100644
> --- a/hw/9pfs/virtio-9p.h
> +++ b/hw/9pfs/virtio-9p.h
> @@ -203,6 +203,7 @@ struct V9fsFidState
> V9fsXattr xattr;
> } fs;
> uid_t uid;
> + int ref;
> V9fsFidState *next;
> };
>
> @@ -361,5 +362,11 @@ static inline size_t do_pdu_unpack(void *dst, struct iovec *sg, int sg_count,
> return pdu_packunpack(dst, sg, sg_count, offset, size, 0);
> }
>
> +static inline void put_fid(V9fsFidState *fidp)
> +{
> + BUG_ON(!fidp->ref);
> + fidp->ref--;
> +}
> +
> extern void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq);
> #endif
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid
2011-06-09 23:10 ` [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid Venkateswararao Jujjuri
@ 2011-06-10 6:27 ` Aneesh Kumar K.V
0 siblings, 0 replies; 13+ messages in thread
From: Aneesh Kumar K.V @ 2011-06-10 6:27 UTC (permalink / raw)
To: Venkateswararao Jujjuri, qemu-devel
On Thu, 09 Jun 2011 16:10:39 -0700, Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com> wrote:
> On 06/06/2011 10:16 AM, Aneesh Kumar K.V wrote:
> > Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
>
> Just one minor issue below; otherwise
>
> Reviewed-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
>
> > ---
....
> > hw/9pfs/virtio-9p.c | 205 +++++++++++++++++++++++++++++++++++----------------
> >
> > pdu_unmarshal(pdu, offset, "dd",&fid,&datasync);
> > - fidp = lookup_fid(s, fid);
> > + fidp = get_fid(s, fid);
> > if (fidp == NULL) {
> > err = -ENOENT;
> > goto out;
> > @@ -1444,6 +1465,7 @@ static void v9fs_fsync(void *opaque)
> > err = offset;
> > }
> > out:
> > + put_fid(fidp);
> It should be
>
> put_fid(fidp);
>
> out_nofid:
>
ok
> > complete_pdu(s, pdu, err);
> > }
-aneesh
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-06-10 6:28 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-06 17:16 [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid Aneesh Kumar K.V
2011-06-06 17:16 ` [Qemu-devel] [PATCH 2/6] hw/9pfs: Don't free fid in clunk Aneesh Kumar K.V
2011-06-10 0:27 ` Venkateswararao Jujjuri
2011-06-10 6:19 ` Aneesh Kumar K.V
2011-06-06 17:16 ` [Qemu-devel] [PATCH 3/6] hw/9pfs: Add file descriptor reclaim support Aneesh Kumar K.V
2011-06-10 1:27 ` Venkateswararao Jujjuri
2011-06-06 17:16 ` [Qemu-devel] [PATCH 4/6] hw/9pfs: init fid list properly Aneesh Kumar K.V
2011-06-06 17:16 ` [Qemu-devel] [PATCH 5/6] hw/9pfs: Use v9fs_do_close instead of close Aneesh Kumar K.V
2011-06-10 1:32 ` Venkateswararao Jujjuri
2011-06-06 17:16 ` [Qemu-devel] [PATCH 6/6] hw/9pfs: Add directory reclaim support Aneesh Kumar K.V
2011-06-10 1:36 ` Venkateswararao Jujjuri
2011-06-09 23:10 ` [Qemu-devel] [PATCH 1/6] hw/9pfs: Add reference counting for fid Venkateswararao Jujjuri
2011-06-10 6:27 ` Aneesh Kumar K.V
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).