[Cbe-oss-dev] [RFC/PATCH] libspe: cleanup SPU create code
Jeremy Kerr
jk at ozlabs.org
Mon May 7 15:35:07 EST 2007
This patch is a cleanup of the code associated with creating SPU
contexts:
* Whitespace fixes
* Make coding style consistent
* Avoid some magic numbers
This should involve no functional change to the code.
Signed-off-by: Jeremy Kerr <jk at ozlabs.org>
spebase/create.c | 357 +++++++++++++++++++++++++++++-------------------------
spebase/spebase.h | 6
2 files changed, 199 insertions(+), 164 deletions(-)
Index: libspe2/spebase/create.c
===================================================================
--- libspe2.orig/spebase/create.c
+++ libspe2/spebase/create.c
@@ -33,36 +33,36 @@
#include "create.h"
#include "spebase.h"
-struct fd_attr
-{
+
+struct fd_attr {
const char *name;
int mode;
};
static const struct fd_attr spe_fd_attr[NUM_MBOX_FDS] = {
- [FD_MBOX] = { .name = "mbox", .mode = O_RDONLY, },
- [FD_MBOX_STAT] = { .name = "mbox_stat", .mode = O_RDONLY, },
- [FD_IBOX] = { .name = "ibox", .mode = O_RDONLY, },
- [FD_IBOX_NB] = { .name = "ibox", .mode = O_RDONLY | O_NONBLOCK, },
- [FD_IBOX_STAT] = { .name = "ibox_stat", .mode = O_RDONLY, },
- [FD_WBOX] = { .name = "wbox", .mode = O_WRONLY, },
- [FD_WBOX_NB] = { .name = "wbox", .mode = O_WRONLY | O_NONBLOCK, },
- [FD_WBOX_STAT] = { .name = "wbox_stat", .mode = O_RDONLY, },
- [FD_SIG1] = { .name = "signal1", .mode = O_RDWR, },
- [FD_SIG2] = { .name = "signal2", .mode = O_RDWR, },
- [FD_MFC] = { .name = "mfc", .mode = O_RDWR, },
- [FD_MSS] = { .name = "mss", .mode = O_RDWR, },
+ [FD_MBOX] = { .name = "mbox", .mode = O_RDONLY },
+ [FD_MBOX_STAT] = { .name = "mbox_stat", .mode = O_RDONLY },
+ [FD_IBOX] = { .name = "ibox", .mode = O_RDONLY },
+ [FD_IBOX_NB] = { .name = "ibox", .mode = O_RDONLY |O_NONBLOCK },
+ [FD_IBOX_STAT] = { .name = "ibox_stat", .mode = O_RDONLY },
+ [FD_WBOX] = { .name = "wbox", .mode = O_WRONLY },
+ [FD_WBOX_NB] = { .name = "wbox", .mode = O_WRONLY|O_NONBLOCK },
+ [FD_WBOX_STAT] = { .name = "wbox_stat", .mode = O_RDONLY },
+ [FD_SIG1] = { .name = "signal1", .mode = O_RDWR },
+ [FD_SIG2] = { .name = "signal2", .mode = O_RDWR },
+ [FD_MFC] = { .name = "mfc", .mode = O_RDWR },
+ [FD_MSS] = { .name = "mss", .mode = O_RDWR },
};
-void *mapfileat( int dir, const char* filename, int size)
-{
+void *mapfileat(int dir, const char *filename, int size)
+{
int fd_temp;
void *ret;
-
- fd_temp=openat(dir, filename, O_RDWR);
+
+ fd_temp = openat(dir, filename, O_RDWR);
if (fd_temp < 0) {
- DEBUG_PRINTF ("ERROR: Could not open SPE %s file.\n",filename);
- errno=EFAULT;
+ DEBUG_PRINTF("ERROR: Could not open SPE %s file.\n", filename);
+ errno = EFAULT;
return MAP_FAILED;
}
ret = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd_temp, 0);
@@ -71,17 +71,20 @@ void *mapfileat( int dir, const char* fi
return ret;
}
-int setsignotify(int dir, const char* filename)
+int setsignotify(int dir, const char *filename)
{
- int fd_sig;
- char one[] = "1";
- int ignore;
- fd_sig = openat (dir, filename, O_RDWR);
- if (fd_sig < 0) {
- return -1;
- }
- ignore=write (fd_sig, one, 1);
+ int fd_sig, rc = 0;
+ char one = '1';
+
+ fd_sig = openat(dir, filename, O_RDWR);
+ if (fd_sig < 0)
+ return -1;
+
+ if (write(fd_sig, &one, sizeof(one)) != sizeof(one))
+ rc = -1;
+
close(fd_sig);
+
return 0;
}
@@ -97,20 +100,25 @@ void _base_spe_context_unlock(spe_contex
int open_if_closed(struct spe_context *spe, enum fd_name fdesc, int locked)
{
- if (!locked) _base_spe_context_lock(spe, fdesc);
-
- if (spe->base_private->spe_fds_array[(int)fdesc] != -1) { // already open
- spe->base_private->spe_fds_refcount[(int)fdesc]++;
- }
- else {
- spe->base_private->spe_fds_array[(int)fdesc] =
- openat(spe->base_private->fd_spe_dir, spe_fd_attr[fdesc].name, spe_fd_attr[fdesc].mode);
+ if (!locked)
+ _base_spe_context_lock(spe, fdesc);
+
+ /* already open? */
+ if (spe->base_private->spe_fds_array[fdesc] != -1) {
+ spe->base_private->spe_fds_refcount[fdesc]++;
+ } else {
+ spe->base_private->spe_fds_array[fdesc] =
+ openat(spe->base_private->fd_spe_dir,
+ spe_fd_attr[fdesc].name,
+ spe_fd_attr[fdesc].mode);
+
if (spe->base_private->spe_fds_array[(int)fdesc] > 0)
spe->base_private->spe_fds_refcount[(int)fdesc]++;
}
- if (!locked) _base_spe_context_unlock(spe, fdesc);
-
+ if (!locked)
+ _base_spe_context_unlock(spe, fdesc);
+
return spe->base_private->spe_fds_array[(int)fdesc];
}
@@ -119,16 +127,16 @@ void close_if_open(struct spe_context *s
_base_spe_context_lock(spe, fdesc);
if (spe->base_private->spe_fds_array[(int)fdesc] != -1 &&
- spe->base_private->spe_fds_refcount[(int)fdesc] == 1){
-
+ spe->base_private->spe_fds_refcount[(int)fdesc] == 1) {
+
spe->base_private->spe_fds_refcount[(int)fdesc]--;
close(spe->base_private->spe_fds_array[(int)fdesc]);
-
+
spe->base_private->spe_fds_array[(int)fdesc] = -1;
- }else if (spe->base_private->spe_fds_refcount[(int)fdesc] > 0) {
+ } else if (spe->base_private->spe_fds_refcount[(int)fdesc] > 0) {
spe->base_private->spe_fds_refcount[(int)fdesc]--;
}
-
+
_base_spe_context_unlock(spe, fdesc);
}
@@ -136,26 +144,28 @@ static int free_spe_context(struct spe_c
{
int i;
- if(spe->base_private->psmap_mmap_base != MAP_FAILED) {
+ if (spe->base_private->psmap_mmap_base != MAP_FAILED) {
munmap(spe->base_private->psmap_mmap_base, PSMAP_SIZE);
- }
- else {
- if(spe->base_private->mfc_mmap_base != MAP_FAILED)
+
+ } else {
+ if (spe->base_private->mfc_mmap_base != MAP_FAILED)
munmap(spe->base_private->mfc_mmap_base, MFC_SIZE);
- if(spe->base_private->mssync_mmap_base != MAP_FAILED)
+ if (spe->base_private->mssync_mmap_base != MAP_FAILED)
munmap(spe->base_private->mssync_mmap_base, MSS_SIZE);
- if(spe->base_private->cntl_mmap_base != MAP_FAILED)
+ if (spe->base_private->cntl_mmap_base != MAP_FAILED)
munmap(spe->base_private->cntl_mmap_base, CNTL_SIZE);
- if(spe->base_private->signal1_mmap_base != MAP_FAILED)
- munmap(spe->base_private->signal1_mmap_base,SIGNAL_SIZE);
- if(spe->base_private->signal2_mmap_base != MAP_FAILED)
- munmap(spe->base_private->signal2_mmap_base, SIGNAL_SIZE);
+ if (spe->base_private->signal1_mmap_base != MAP_FAILED)
+ munmap(spe->base_private->signal1_mmap_base,
+ SIGNAL_SIZE);
+ if (spe->base_private->signal2_mmap_base != MAP_FAILED)
+ munmap(spe->base_private->signal2_mmap_base,
+ SIGNAL_SIZE);
}
- if(spe->base_private->mem_mmap_base != MAP_FAILED)
+ if (spe->base_private->mem_mmap_base != MAP_FAILED)
munmap(spe->base_private->mem_mmap_base, LS_SIZE);
- for ( i=0;i<NUM_MBOX_FDS;i++){
+ for (i = 0; i < NUM_MBOX_FDS; i++) {
if (spe->base_private->spe_fds_array[i] >= 0)
close(spe->base_private->spe_fds_array[i]);
pthread_mutex_destroy(&spe->base_private->fd_lock[i]);
@@ -170,147 +180,163 @@ static int free_spe_context(struct spe_c
return 0;
}
-spe_context_ptr_t _base_spe_context_create(unsigned int flags, spe_gang_context_ptr_t gctx, spe_context_ptr_t aff_spe)
+spe_context_ptr_t _base_spe_context_create(unsigned int flags,
+ spe_gang_context_ptr_t gctx, spe_context_ptr_t aff_spe)
{
char pathname[256];
- int i;
+ int i, aff_spe_fd = 0;
unsigned int spu_createflags = 0;
-
- /* Put some sane defaults into the SPE context
- */
-
struct spe_context *spe = NULL;
-
- spe = calloc (1, sizeof *spe);
+ struct spe_context_base_priv *priv;
+
+ /* Put some sane defaults into the SPE context */
+ spe = malloc(sizeof(*spe));
if (!spe) {
- DEBUG_PRINTF ("ERROR: Could not allocate spe context.\n");
- errno = ENOMEM;
+ DEBUG_PRINTF("ERROR: Could not allocate spe context.\n");
return NULL;
}
-
- spe->base_private = calloc (1, sizeof *spe->base_private);
+ memset(spe, 0, sizeof(*spe));
+
+ spe->base_private = malloc(sizeof(*spe->base_private));
if (!spe->base_private) {
- DEBUG_PRINTF ("ERROR: Could not allocate spe->base_private context.\n");
+ DEBUG_PRINTF("ERROR: Could not allocate "
+ "spe->base_private context.\n");
free(spe);
- errno = ENOMEM;
return NULL;
}
- spe->base_private->fd_spe_dir = -1;
-
- spe->base_private->mem_mmap_base = MAP_FAILED;
- spe->base_private->psmap_mmap_base = MAP_FAILED;
- spe->base_private->mssync_mmap_base = MAP_FAILED;
- spe->base_private->mfc_mmap_base = MAP_FAILED;
- spe->base_private->cntl_mmap_base = MAP_FAILED;
- spe->base_private->signal1_mmap_base = MAP_FAILED;
- spe->base_private->signal2_mmap_base = MAP_FAILED;
-
- for ( i=0;i<NUM_MBOX_FDS;i++){
- spe->base_private->spe_fds_array[i]=-1;
- pthread_mutex_init(&spe->base_private->fd_lock[i], NULL);
- }
+ /* just a convenience variable */
+ priv = spe->base_private;
- /* Make SPE-Create Flags
- */
+ priv->fd_spe_dir = -1;
+ priv->mem_mmap_base = MAP_FAILED;
+ priv->psmap_mmap_base = MAP_FAILED;
+ priv->mssync_mmap_base = MAP_FAILED;
+ priv->mfc_mmap_base = MAP_FAILED;
+ priv->cntl_mmap_base = MAP_FAILED;
+ priv->signal1_mmap_base = MAP_FAILED;
+ priv->signal2_mmap_base = MAP_FAILED;
+
+ for (i = 0; i < NUM_MBOX_FDS; i++) {
+ priv->spe_fds_array[i] = -1;
+ pthread_mutex_init(&priv->fd_lock[i], NULL);
+ }
- if ( flags & SPE_ISOLATE ) {
+ /* initialise spu_createflags */
+ if (flags & SPE_ISOLATE) {
flags |= SPE_MAP_PS;
- spu_createflags |= SPU_CREATE_ISOLATE | SPU_CREATE_NOSCHED;
+ spu_createflags |= SPU_CREATE_ISOLATE | SPU_CREATE_NOSCHED;
}
-
- if ( flags & SPE_EVENTS_ENABLE )
- spu_createflags |= SPU_CREATE_EVENTS_ENABLED;
-
- if ( aff_spe )
+
+ if (flags & SPE_EVENTS_ENABLE)
+ spu_createflags |= SPU_CREATE_EVENTS_ENABLED;
+
+ if (aff_spe)
spu_createflags |= SPU_CREATE_AFFINITY_SPU;
- if ( flags & SPE_AFFINITY_MEMORY )
+ if (flags & SPE_AFFINITY_MEMORY)
spu_createflags |= SPU_CREATE_AFFINITY_MEM;
-
- /* Make the SPUFS directory for the SPE
- */
-
+
+ /* Make the SPUFS directory for the SPE */
if (gctx == NULL)
- sprintf (pathname, "/spu/spethread-%i-%lu",
- getpid (), (unsigned long)spe);
- else
- sprintf (pathname, "/spu/%s/spethread-%i-%lu",
- gctx->base_private->gangname, getpid (), (unsigned long)spe);
-
- if ( aff_spe )
- spe->base_private->fd_spe_dir = spu_create(pathname, spu_createflags, S_IRUSR | S_IWUSR | S_IXUSR,
- aff_spe->base_private->fd_spe_dir);
+ sprintf(pathname, "/spu/spethread-%i-%lu",
+ getpid(), (unsigned long)spe);
else
- spe->base_private->fd_spe_dir = spu_create(pathname, spu_createflags, S_IRUSR | S_IWUSR | S_IXUSR);
- if (spe->base_private->fd_spe_dir < 0) {
- DEBUG_PRINTF ("ERROR: Could not create SPE %s\n", pathname);
+ sprintf(pathname, "/spu/%s/spethread-%i-%lu",
+ gctx->base_private->gangname, getpid(),
+ (unsigned long)spe);
+
+ if (aff_spe)
+ aff_spe_fd = aff_spe->base_private->fd_spe_dir;
+
+ priv->fd_spe_dir = spu_create(pathname, spu_createflags,
+ S_IRUSR | S_IWUSR | S_IXUSR, aff_spe_fd);
+
+ if (priv->fd_spe_dir < 0) {
+ DEBUG_PRINTF("ERROR: Could not create SPE %s\n", pathname);
perror("spu_create()");
free_spe_context(spe);
- errno=EFAULT;
+ errno = EFAULT;
return NULL;
}
-
- spe->base_private->flags=flags;
-
- /* Map the required areas into process memory
- */
-
- spe->base_private->mem_mmap_base = mapfileat( spe->base_private->fd_spe_dir, "mem", LS_SIZE);
- if ( spe->base_private->mem_mmap_base == MAP_FAILED ) {
- DEBUG_PRINTF ("ERROR: Could not map SPE memory. \n");
+
+ priv->flags = flags;
+
+ /* Map the required areas into process memory */
+ priv->mem_mmap_base = mapfileat(priv->fd_spe_dir, "mem", LS_SIZE);
+ if (priv->mem_mmap_base == MAP_FAILED) {
+ DEBUG_PRINTF("ERROR: Could not map SPE memory.\n");
free_spe_context(spe);
errno = ENOMEM;
return NULL;
}
- if ( flags & SPE_MAP_PS ){
+ if (flags & SPE_MAP_PS) {
+ /* It's possible to map the entire problem state area with
+ * one mmap - try this first */
+ priv->psmap_mmap_base = mapfileat(priv->fd_spe_dir,
+ "psmap", PSMAP_SIZE);
+
+ if (priv->psmap_mmap_base != MAP_FAILED) {
+ priv->mssync_mmap_base =
+ priv->psmap_mmap_base + MSSYNC_OFFSET;
+ priv->mfc_mmap_base =
+ priv->psmap_mmap_base + MFC_OFFSET;
+ priv->cntl_mmap_base =
+ priv->psmap_mmap_base + CNTL_OFFSET;
+ priv->signal1_mmap_base =
+ priv->psmap_mmap_base + SIGNAL1_OFFSET;
+ priv->signal2_mmap_base =
+ priv->psmap_mmap_base + SIGNAL2_OFFSET;
- spe->base_private->psmap_mmap_base = mapfileat( spe->base_private->fd_spe_dir, "psmap", PSMAP_SIZE);
-
- if (spe->base_private->psmap_mmap_base == MAP_FAILED) {
- spe->base_private->mfc_mmap_base = mapfileat( spe->base_private->fd_spe_dir, "mfc", MFC_SIZE);
- spe->base_private->mssync_mmap_base = mapfileat( spe->base_private->fd_spe_dir, "mss", MSS_SIZE);
- spe->base_private->cntl_mmap_base = mapfileat( spe->base_private->fd_spe_dir, "cntl", CNTL_SIZE);
- spe->base_private->signal1_mmap_base = mapfileat( spe->base_private->fd_spe_dir, "signal1",SIGNAL_SIZE);
- spe->base_private->signal2_mmap_base = mapfileat( spe->base_private->fd_spe_dir, "signal2", SIGNAL_SIZE);
- if ( spe->base_private->mfc_mmap_base == MAP_FAILED ||
- spe->base_private->cntl_mmap_base == MAP_FAILED ||
- spe->base_private->signal1_mmap_base == MAP_FAILED ||
- spe->base_private->signal2_mmap_base == MAP_FAILED ) {
- DEBUG_PRINTF ("ERROR: Could not map SPE PS memory. \n");
+ } else {
+ /* map each region separately */
+ priv->mfc_mmap_base =
+ mapfileat(priv->fd_spe_dir, "mfc", MFC_SIZE);
+ priv->mssync_mmap_base =
+ mapfileat(priv->fd_spe_dir, "mss", MSS_SIZE);
+ priv->cntl_mmap_base =
+ mapfileat(priv->fd_spe_dir, "cntl", CNTL_SIZE);
+ priv->signal1_mmap_base =
+ mapfileat(priv->fd_spe_dir, "signal1",
+ SIGNAL_SIZE);
+ priv->signal2_mmap_base =
+ mapfileat(priv->fd_spe_dir, "signal2",
+ SIGNAL_SIZE);
+
+ if (priv->mfc_mmap_base == MAP_FAILED ||
+ priv->cntl_mmap_base == MAP_FAILED ||
+ priv->signal1_mmap_base == MAP_FAILED ||
+ priv->signal2_mmap_base == MAP_FAILED) {
+ DEBUG_PRINTF("ERROR: Could not map SPE "
+ "PS memory.\n");
free_spe_context(spe);
errno = ENOMEM;
return NULL;
}
- } else {
- spe->base_private->mssync_mmap_base = spe->base_private->psmap_mmap_base + 0x00000;
- spe->base_private->mfc_mmap_base = spe->base_private->psmap_mmap_base + 0x03000;
- spe->base_private->cntl_mmap_base = spe->base_private->psmap_mmap_base + 0x04000;
- spe->base_private->signal1_mmap_base = spe->base_private->psmap_mmap_base + 0x14000;
- spe->base_private->signal2_mmap_base = spe->base_private->psmap_mmap_base + 0x1c000;
}
}
-
- if ( flags & SPE_CFG_SIGNOTIFY1_OR ) {
- if (setsignotify(spe->base_private->fd_spe_dir, "signal1_type")) {
- DEBUG_PRINTF ("ERROR: Could not open SPE signal1_type file.\n");
+
+ if (flags & SPE_CFG_SIGNOTIFY1_OR) {
+ if (setsignotify(priv->fd_spe_dir, "signal1_type")) {
+ DEBUG_PRINTF("ERROR: Could not open SPE "
+ "signal1_type file.\n");
free_spe_context(spe);
errno = EFAULT;
return NULL;
}
}
- if ( flags & SPE_CFG_SIGNOTIFY2_OR ) {
- if (setsignotify(spe->base_private->fd_spe_dir, "signal2_type")) {
- DEBUG_PRINTF ("ERROR: Could not open SPE signal2_type file.\n");
+ if (flags & SPE_CFG_SIGNOTIFY2_OR) {
+ if (setsignotify(priv->fd_spe_dir, "signal2_type")) {
+ DEBUG_PRINTF("ERROR: Could not open SPE "
+ "signal2_type file.\n");
free_spe_context(spe);
errno = EFAULT;
return NULL;
}
}
-
return spe;
}
@@ -330,37 +356,40 @@ spe_gang_context_ptr_t _base_spe_gang_co
char pathname[256];
struct spe_gang_context_base_priv *pgctx = NULL;
struct spe_gang_context *gctx = NULL;
-
- gctx = calloc (1, sizeof *gctx);
+
+ gctx = malloc(sizeof(*gctx));
if (!gctx) {
- DEBUG_PRINTF ("ERROR: Could not allocate spe context.\n");
- errno = ENOMEM;
+ DEBUG_PRINTF("ERROR: Could not allocate spe context.\n");
return NULL;
}
+ memset(gctx, 0, sizeof(*gctx));
- pgctx = calloc (1, sizeof *pgctx);
+ pgctx = malloc(sizeof(*pgctx));
if (!pgctx) {
- DEBUG_PRINTF ("ERROR: Could not allocate spe context.\n");
+ DEBUG_PRINTF("ERROR: Could not allocate spe context.\n");
free(gctx);
- errno = ENOMEM;
return NULL;
}
-
+ memset(pgctx, 0, sizeof(*pgctx));
+
gctx->base_private = pgctx;
-
- sprintf (gctx->base_private->gangname, "gang-%i-%lu", getpid(), (unsigned long) gctx);
- sprintf (pathname, "/spu/%s", gctx->base_private->gangname);
- gctx->base_private->fd_gang_dir = spu_create(pathname, 2, S_IRUSR | S_IWUSR | S_IXUSR);
+ sprintf(gctx->base_private->gangname, "gang-%i-%lu", getpid(),
+ (unsigned long)gctx);
+ sprintf(pathname, "/spu/%s", gctx->base_private->gangname);
+
+ gctx->base_private->fd_gang_dir = spu_create(pathname, SPU_CREATE_GANG,
+ S_IRUSR | S_IWUSR | S_IXUSR);
+
if (gctx->base_private->fd_gang_dir < 0) {
- DEBUG_PRINTF ("ERROR: Could not create Gang %s\n", pathname);
+ DEBUG_PRINTF("ERROR: Could not create Gang %s\n", pathname);
free_spe_gang_context(gctx);
- errno=EFAULT;
+ errno = EFAULT;
return NULL;
}
-
+
gctx->base_private->flags = flags;
-
+
return gctx;
}
Index: libspe2/spebase/spebase.h
===================================================================
--- libspe2.orig/spebase/spebase.h
+++ libspe2/spebase/spebase.h
@@ -104,6 +104,12 @@ struct spe_context_base_priv {
#define CNTL_SIZE 0x1000
#define SIGNAL_SIZE 0x1000
+#define MSSYNC_OFFSET 0x00000
+#define MFC_OFFSET 0x03000
+#define CNTL_OFFSET 0x04000
+#define SIGNAL1_OFFSET 0x14000
+#define SIGNAL2_OFFSET 0x1c000
+
/*
*/
#define SPE_PROGRAM_NORMAL_END 0x2000
More information about the cbe-oss-dev
mailing list