Message ID | 1307380618-1963-2-git-send-email-aneesh.kumar@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
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
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
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
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(-)