Skip to content

Commit

Permalink
Merge remote-tracking branch 'remotes/gkurz-gitlab/tags/9p-next-pull-…
Browse files Browse the repository at this point in the history 
…request' into staging

This fixes a Coverity report and improves the fid reclaim logic.

# gpg: Signature made Mon 25 Jan 2021 09:37:28 GMT
# gpg:                using RSA key B4828BAF943140CEF2A3491071D4D5E5822F73D6
# gpg: Good signature from "Greg Kurz <groug@kaod.org>" [full]
# gpg:                 aka "Gregory Kurz <gregory.kurz@free.fr>" [full]
# gpg:                 aka "[jpeg image of size 3330]" [full]
# Primary key fingerprint: B482 8BAF 9431 40CE F2A3  4910 71D4 D5E5 822F 73D6

* remotes/gkurz-gitlab/tags/9p-next-pull-request:
  9pfs: Convert reclaim list to QSLIST
  9pfs: Improve unreclaim loop
  9pfs: Convert V9fsFidState::fid_list to QSIMPLEQ
  9pfs: Convert V9fsFidState::clunked to bool
  9pfs/proxy: Check return value of proxy_marshal()

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
  • Loading branch information
@pm215
pm215 committed Jan 25, 2021
2 parents 55d9895 + 81f9766 commit 3dcfd4e
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 50 deletions.
3 changes: 2 additions & 1 deletion hw/9pfs/9p-proxy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,8 @@ static int v9fs_request(V9fsProxy *proxy, int type, void *response, ...)
}

/* marshal the header details */
proxy_marshal(iovec, 0, "dd", header.type, header.size);
retval = proxy_marshal(iovec, 0, "dd", header.type, header.size);
assert(retval == 4 * 2);
header.size += PROXY_HDR_SZ;

retval = qemu_write_full(proxy->sockfd, iovec->iov_base, header.size);
Expand Down
102 changes: 57 additions & 45 deletions hw/9pfs/9p.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ static V9fsFidState *coroutine_fn get_fid(V9fsPDU *pdu, int32_t fid)
V9fsFidState *f;
V9fsState *s = pdu->s;

for (f = s->fid_list; f; f = f->next) {
QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
BUG_ON(f->clunked);
if (f->fid == fid) {
/*
Expand Down Expand Up @@ -295,7 +295,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
{
V9fsFidState *f;

for (f = s->fid_list; f; f = f->next) {
QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
/* If fid is already there return NULL */
BUG_ON(f->clunked);
if (f->fid == fid) {
Expand All @@ -311,8 +311,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
* reclaim won't close the file descriptor
*/
f->flags |= FID_REFERENCED;
f->next = s->fid_list;
s->fid_list = f;
QSIMPLEQ_INSERT_TAIL(&s->fid_list, f, next);

v9fs_readdir_init(s->proto_version, &f->fs.dir);
v9fs_readdir_init(s->proto_version, &f->fs_reclaim.dir);
Expand Down Expand Up @@ -401,29 +400,27 @@ static int coroutine_fn put_fid(V9fsPDU *pdu, V9fsFidState *fidp)

static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid)
{
V9fsFidState **fidpp, *fidp;
V9fsFidState *fidp;

for (fidpp = &s->fid_list; *fidpp; fidpp = &(*fidpp)->next) {
if ((*fidpp)->fid == fid) {
break;
QSIMPLEQ_FOREACH(fidp, &s->fid_list, next) {
if (fidp->fid == fid) {
QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next);
fidp->clunked = true;
return fidp;
}
}
if (*fidpp == NULL) {
return NULL;
}
fidp = *fidpp;
*fidpp = fidp->next;
fidp->clunked = 1;
return fidp;
return NULL;
}

void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
{
int reclaim_count = 0;
V9fsState *s = pdu->s;
V9fsFidState *f, *reclaim_list = NULL;
V9fsFidState *f;
QSLIST_HEAD(, V9fsFidState) reclaim_list =
QSLIST_HEAD_INITIALIZER(reclaim_list);

for (f = s->fid_list; f; f = f->next) {
QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
/*
* Unlink fids cannot be reclaimed. Check
* for them and skip them. Also skip fids
Expand Down Expand Up @@ -453,8 +450,7 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
* a clunk request won't free this fid
*/
f->ref++;
f->rclm_lst = reclaim_list;
reclaim_list = f;
QSLIST_INSERT_HEAD(&reclaim_list, f, reclaim_next);
f->fs_reclaim.fd = f->fs.fd;
f->fs.fd = -1;
reclaim_count++;
Expand All @@ -466,8 +462,7 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
* a clunk request won't free this fid
*/
f->ref++;
f->rclm_lst = reclaim_list;
reclaim_list = f;
QSLIST_INSERT_HEAD(&reclaim_list, f, reclaim_next);
f->fs_reclaim.dir.stream = f->fs.dir.stream;
f->fs.dir.stream = NULL;
reclaim_count++;
Expand All @@ -481,15 +476,14 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
* 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;
while (!QSLIST_EMPTY(&reclaim_list)) {
f = QSLIST_FIRST(&reclaim_list);
QSLIST_REMOVE(&reclaim_list, f, V9fsFidState, reclaim_next);
if (f->fid_type == P9_FID_FILE) {
v9fs_co_close(pdu, &f->fs_reclaim);
} else if (f->fid_type == P9_FID_DIR) {
v9fs_co_closedir(pdu, &f->fs_reclaim);
}
f->rclm_lst = NULL;
/*
* Now drop the fid reference, free it
* if clunked.
Expand All @@ -502,32 +496,50 @@ static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU *pdu, V9fsPath *path)
{
int err;
V9fsState *s = pdu->s;
V9fsFidState *fidp;
V9fsFidState *fidp, *fidp_next;

again:
for (fidp = s->fid_list; fidp; fidp = fidp->next) {
if (fidp->path.size != path->size) {
continue;
}
if (!memcmp(fidp->path.data, path->data, path->size)) {
fidp = QSIMPLEQ_FIRST(&s->fid_list);
if (!fidp) {
return 0;
}

/*
* v9fs_reopen_fid() can yield : a reference on the fid must be held
* to ensure its pointer remains valid and we can safely pass it to
* QSIMPLEQ_NEXT(). The corresponding put_fid() can also yield so
* we must keep a reference on the next fid as well. So the logic here
* is to get a reference on a fid and only put it back during the next
* iteration after we could get a reference on the next fid. Start with
* the first one.
*/
for (fidp->ref++; fidp; fidp = fidp_next) {
if (fidp->path.size == path->size &&
!memcmp(fidp->path.data, path->data, path->size)) {
/* Mark the fid non reclaimable. */
fidp->flags |= FID_NON_RECLAIMABLE;

/* reopen the file/dir if already closed */
err = v9fs_reopen_fid(pdu, fidp);
if (err < 0) {
put_fid(pdu, fidp);
return err;
}
}

fidp_next = QSIMPLEQ_NEXT(fidp, next);

if (fidp_next) {
/*
* Go back to head of fid list because
* the list could have got updated when
* switched to the worker thread
* Ensure the next fid survives a potential clunk request during
* put_fid() below and v9fs_reopen_fid() in the next iteration.
*/
if (err == 0) {
goto again;
}
fidp_next->ref++;
}

/* We're done with this fid */
put_fid(pdu, fidp);
}

return 0;
}

Expand All @@ -537,14 +549,14 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
V9fsFidState *fidp;

/* Free all fids */
while (s->fid_list) {
while (!QSIMPLEQ_EMPTY(&s->fid_list)) {
/* Get fid */
fidp = s->fid_list;
fidp = QSIMPLEQ_FIRST(&s->fid_list);
fidp->ref++;

/* Clunk fid */
s->fid_list = fidp->next;
fidp->clunked = 1;
QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next);
fidp->clunked = true;

put_fid(pdu, fidp);
}
Expand Down Expand Up @@ -3121,7 +3133,7 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp,
* Fixup fid's pointing to the old name to
* start pointing to the new name
*/
for (tfidp = s->fid_list; tfidp; tfidp = tfidp->next) {
QSIMPLEQ_FOREACH(tfidp, &s->fid_list, next) {
if (v9fs_path_is_ancestor(&fidp->path, &tfidp->path)) {
/* replace the name */
v9fs_fix_path(&tfidp->path, &new_path, strlen(fidp->path.data));
Expand Down Expand Up @@ -3215,7 +3227,7 @@ static int coroutine_fn v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir,
* Fixup fid's pointing to the old name to
* start pointing to the new name
*/
for (tfidp = s->fid_list; tfidp; tfidp = tfidp->next) {
QSIMPLEQ_FOREACH(tfidp, &s->fid_list, next) {
if (v9fs_path_is_ancestor(&oldpath, &tfidp->path)) {
/* replace the name */
v9fs_fix_path(&tfidp->path, &newpath, strlen(oldpath.data));
Expand Down Expand Up @@ -4081,7 +4093,7 @@ int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
s->ctx.fmode = fse->fmode;
s->ctx.dmode = fse->dmode;

s->fid_list = NULL;
QSIMPLEQ_INIT(&s->fid_list);
qemu_co_rwlock_init(&s->rename_lock);

if (s->ops->init(&s->ctx, errp) < 0) {
Expand Down
8 changes: 4 additions & 4 deletions hw/9pfs/9p.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,9 @@ struct V9fsFidState {
int open_flags;
uid_t uid;
int ref;
int clunked;
V9fsFidState *next;
V9fsFidState *rclm_lst;
bool clunked;
QSIMPLEQ_ENTRY(V9fsFidState) next;
QSLIST_ENTRY(V9fsFidState) reclaim_next;
};

typedef enum AffixType_t {
Expand Down Expand Up @@ -339,7 +339,7 @@ typedef struct {
struct V9fsState {
QLIST_HEAD(, V9fsPDU) free_list;
QLIST_HEAD(, V9fsPDU) active_list;
V9fsFidState *fid_list;
QSIMPLEQ_HEAD(, V9fsFidState) fid_list;
FileOperations *ops;
FsContext ctx;
char *tag;
Expand Down

0 comments on commit 3dcfd4e

Please sign in to comment.