Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Factor long ioctx open/close functions (issue 108)

The allocation/open and deallocation/close are now in seperate functions
from the locking and list insertion/removal.  Error paths for the former
are taken without involving the latter, which helps clarity.

This is not expected to fix anything except readability.
  • Loading branch information...
commit 7b30880d3f72c9f12185392eafd50f4c363044b1 1 parent 0647478
@garlick garlick authored
Showing with 90 additions and 65 deletions.
  1. +88 −62 diod/ioctx.c
  2. +2 −3 diod/ops.c
View
150 diod/ioctx.c
@@ -81,7 +81,7 @@ struct path_struct {
int refcount;
char *s;
int len;
- IOCtx ioctx;
+ IOCtx ioctx; /* double-linked list of IOCtx opening this path */
};
struct pathpool_struct {
@@ -144,6 +144,69 @@ _ioctx_decref (IOCtx ioctx)
return n;
}
+static int
+_ioctx_close_destroy (IOCtx ioctx, int seterrno)
+{
+ int rc = 0;
+
+ if (ioctx->dir) {
+ rc = closedir(ioctx->dir);
+ if (rc < 0 && seterrno)
+ np_uerror (errno);
+ } else if (ioctx->fd != -1) {
+ rc = close (ioctx->fd);
+ if (rc < 0 && seterrno)
+ np_uerror (errno);
+ }
+ if (ioctx->user)
+ np_user_decref (ioctx->user);
+ pthread_mutex_destroy (&ioctx->lock);
+ free (ioctx);
+
+ return rc;
+}
+
+static IOCtx
+_ioctx_create_open (Npuser *user, Path path, int flags, u32 mode)
+{
+ IOCtx ioctx;
+ struct stat sb;
+
+ ioctx = malloc (sizeof (*ioctx));
+ if (!ioctx) {
+ np_uerror (ENOMEM);
+ goto error;
+ }
+ pthread_mutex_init (&ioctx->lock, NULL);
+ ioctx->refcount = 1;
+ ioctx->lock_type = LOCK_UN;
+ ioctx->dir = NULL;
+ ioctx->open_flags = flags;
+ ioctx->user = user;
+ np_user_incref (user);
+ ioctx->prev = ioctx->next = NULL;
+ ioctx->fd = open (path->s, flags, mode);
+ if (ioctx->fd < 0) {
+ np_uerror (errno);
+ goto error;
+ }
+ if (fstat (ioctx->fd, &sb) < 0) {
+ np_uerror (errno);
+ goto error;
+ }
+ ioctx->iounit = 0; /* if iounit=0, v9fs will use msize-P9_IOHDRSZ */
+ if (S_ISDIR(sb.st_mode) && !(ioctx->dir = fdopendir (ioctx->fd))) {
+ np_uerror (errno);
+ goto error;
+ }
+ diod_ustat2qid (&sb, &ioctx->qid);
+ return ioctx;
+error:
+ if (ioctx)
+ _ioctx_close_destroy (ioctx, 0);
+ return NULL;
+}
+
int
ioctx_close (Npfid *fid, int seterrno)
{
@@ -151,29 +214,16 @@ ioctx_close (Npfid *fid, int seterrno)
int n;
int rc = 0;
- if (f->ioctx) {
- xpthread_mutex_lock (&f->path->lock);
- n = _ioctx_decref (f->ioctx);
- if (n == 0)
- _unlink_ioctx (&f->path->ioctx, f->ioctx);
- xpthread_mutex_unlock (&f->path->lock);
- if (n == 0) {
- if (f->ioctx->dir) {
- rc = closedir(f->ioctx->dir);
- if (rc < 0 && seterrno)
- np_uerror (errno);
- } else if (f->ioctx->fd != -1) {
- rc = close (f->ioctx->fd);
- if (rc < 0 && seterrno)
- np_uerror (errno);
- }
- if (f->ioctx->user)
- np_user_decref (f->ioctx->user);
- pthread_mutex_destroy (&f->ioctx->lock);
- free (f->ioctx);
- }
- f->ioctx = NULL;
- }
+ NP_ASSERT (f->ioctx != NULL);
+
+ xpthread_mutex_lock (&f->path->lock);
+ n = _ioctx_decref (f->ioctx);
+ if (n == 0)
+ _unlink_ioctx (&f->path->ioctx, f->ioctx);
+ xpthread_mutex_unlock (&f->path->lock);
+ if (n == 0)
+ rc = _ioctx_close_destroy (f->ioctx, seterrno);
+ f->ioctx = NULL;
return rc;
}
@@ -182,15 +232,12 @@ int
ioctx_open (Npfid *fid, u32 flags, u32 mode)
{
Fid *f = fid->aux;
- struct stat sb;
- IOCtx ip;
- int sharable = ((f->flags & DIOD_FID_FLAGS_SHAREFD)
- && (flags & 3) == O_RDONLY);
+ IOCtx ip = NULL;
NP_ASSERT (f->ioctx == NULL);
xpthread_mutex_lock (&f->path->lock);
- if (sharable) {
+ if ((f->flags & DIOD_FID_FLAGS_SHAREFD) && (flags & 3) == O_RDONLY) {
for (ip = f->path->ioctx; ip != NULL; ip = ip->next) {
if (ip->qid.type != P9_QTFILE)
continue;
@@ -199,46 +246,20 @@ ioctx_open (Npfid *fid, u32 flags, u32 mode)
if (ip->user->uid != fid->user->uid)
continue;
/* NOTE: we could do a stat and check qid? */
- f->ioctx = _ioctx_incref (ip);
+ _ioctx_incref (ip);
break;
}
}
- if (f->ioctx == NULL) {
- f->ioctx = malloc (sizeof (*f->ioctx));
- if (!f->ioctx) {
- np_uerror (ENOMEM);
- goto error;
- }
- pthread_mutex_init (&f->ioctx->lock, NULL);
- f->ioctx->refcount = 1;
- f->ioctx->lock_type = LOCK_UN;
- f->ioctx->dir = NULL;
- f->ioctx->open_flags = flags;
- np_user_incref (fid->user);
- f->ioctx->user = fid->user;
- f->ioctx->prev = f->ioctx->next = NULL;
- f->ioctx->fd = open (f->path->s, flags, mode);
- if (f->ioctx->fd < 0) {
- np_uerror (errno);
- goto error;
- }
- if (fstat (f->ioctx->fd, &sb) < 0) {
- np_uerror (errno);
- goto error;
- }
- f->ioctx->iounit = 0; /* if iounit=0, v9fs will use msize-P9_IOHDRSZ */
- if (S_ISDIR(sb.st_mode) && !(f->ioctx->dir = fdopendir (f->ioctx->fd))) {
- np_uerror (errno);
- goto error;
- }
- diod_ustat2qid (&sb, &f->ioctx->qid);
- _link_ioctx (&f->path->ioctx, f->ioctx);
+ if (!ip) {
+ if ((ip = _ioctx_create_open (fid->user, f->path, flags, mode)))
+ _link_ioctx (&f->path->ioctx, ip);
}
xpthread_mutex_unlock (&f->path->lock);
+ if (!ip)
+ goto error;
+ f->ioctx = ip;
return 0;
error:
- xpthread_mutex_unlock (&f->path->lock);
- ioctx_close (fid, 0);
return -1;
}
@@ -346,6 +367,11 @@ ioctx_qid (IOCtx ioctx)
return &ioctx->qid;
}
+/* N.B. When diod_fidclone() calls path_incref(), the path will not be
+ * removed from the pool even though the ppool lock is not held,
+ * because the fid being cloned holds a reference on the path.
+ */
+
static void
_path_free (Path path)
{
View
5 diod/ops.c
@@ -105,7 +105,6 @@ Npfcall *diod_write (Npfid *fid, u64 offset, u32 count, u8 *data,
Npreq *req);
Npfcall *diod_clunk (Npfid *fid);
Npfcall *diod_remove (Npfid *fid);
-void diod_fiddestroy(Npfid *fid);
Npfcall *diod_statfs (Npfid *fid);
Npfcall *diod_lopen (Npfid *fid, u32 mode);
@@ -653,6 +652,7 @@ diod_lopen (Npfid *fid, u32 flags)
}
if (!(res = np_create_rlopen (ioctx_qid (f->ioctx),
ioctx_iounit (f->ioctx)))) {
+ (void)ioctx_close (fid, 0);
np_uerror (ENOMEM);
goto error;
}
@@ -662,7 +662,6 @@ diod_lopen (Npfid *fid, u32 flags)
fid->user->uname, np_conn_get_client_id (fid->conn),
path_s (f->path));
error_quiet:
- ioctx_close (fid, 0);
return NULL;
}
@@ -704,6 +703,7 @@ diod_lcreate(Npfid *fid, Npstr *name, u32 flags, u32 mode, u32 gid)
}
if (!((ret = np_create_rlcreate (ioctx_qid (f->ioctx),
ioctx_iounit (f->ioctx))))) {
+ (void)ioctx_close (fid, 0);
(void)unlink (path_s (f->path));
np_uerror (ENOMEM);
goto error;
@@ -715,7 +715,6 @@ diod_lcreate(Npfid *fid, Npstr *name, u32 flags, u32 mode, u32 gid)
fid->user->uname, np_conn_get_client_id (fid->conn),
opath ? path_s (opath) : path_s (f->path), name->len, name->str);
error_quiet:
- ioctx_close (fid, 0);
if (opath) {
if (f->path)
path_decref (srv, f->path);
Please sign in to comment.
Something went wrong with that request. Please try again.