[Cbe-oss-dev] [RFC,PATCH] libspe2: stop using spe_fds_refcount

[Kazunori Asayama]

I'd like to raise again an issue about use of reference counters
for file descriptors (spe_fd_refcount) in libspe2/spebase.

I propose to stop using spe_fds_refcount because of the following
reasons:

  1. That mechanism does not work well currently.

     None decrements the counters incremented by mbox functions. That
     means that the current implementation of the mechanism is almost
     meaningless.

  2. Even if the problem 1. is solved, performance issues will appear.

     That is, mbox and/or SNR functions will become to close and to
     re-open the FDs frequently. It seems quite inefficient.

What do you think ?

Attached is a patch to implement such changes.

Thanks,
----
Index: libspe2/spebase/spebase.h
===================================================================
--- libspe2.orig/spebase/spebase.h
+++ libspe2/spebase/spebase.h
@@ -61,7 +61,6 @@ struct spe_context_base_priv {
 	
 	/* SPE Mailbox and Signal fds */
 	int spe_fds_array[NUM_MBOX_FDS];
-	int spe_fds_refcount[NUM_MBOX_FDS];
 	
 	/* SPE MFC Unit fd */
 	//int	fd_mfc;
Index: libspe2/spebase/create.h
===================================================================
--- libspe2.orig/spebase/create.h
+++ libspe2/spebase/create.h
@@ -26,6 +26,5 @@ extern void *mapfileat( int dir, const c
 extern int setsignotify(int dir, const char* filename);
 //extern void open_if_closed(struct spe_context *spe, int * fd, const char* filename, int oflag);
 extern int open_if_closed(struct spe_context *spe, enum fd_name fdesc);
-extern void close_if_open(struct spe_context *spe, enum fd_name fdesc);
 
 #endif
Index: libspe2/spebase/mbox.c
===================================================================
--- libspe2.orig/spebase/mbox.c
+++ libspe2/spebase/mbox.c
@@ -132,11 +132,6 @@ int _base_spe_out_intr_mbox_read(spe_con
 	if (rc == 4)
 		rc = 0;
 
-	if (signal_reg == SPE_SIG_NOTIFY_REG_1)
-		close_if_open(spectx,FD_SIG1);
-	else if (signal_reg == SPE_SIG_NOTIFY_REG_2)
-		close_if_open(spectx,FD_SIG2);
-
 	return rc;
 }
 
Index: libspe2/spebase/create.c
===================================================================
--- libspe2.orig/spebase/create.c
+++ libspe2/spebase/create.c
@@ -81,13 +81,8 @@ int open_if_closed(struct spe_context *s
 {
 	_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 {
+	if (spe->base_private->spe_fds_array[(int)fdesc] == -1) { // not open
 		spe->base_private->spe_fds_array[(int)fdesc] = openat(spe->base_private->fd_spe_dir, spe_fd_name[(int)fdesc], spe_fd_mode[(int)fdesc]);
-		if (spe->base_private->spe_fds_array[(int)fdesc] > 0)
-			spe->base_private->spe_fds_refcount[(int)fdesc]++;
 	}
 
 	_base_spe_context_unlock(spe, fdesc);
@@ -95,24 +90,6 @@ int open_if_closed(struct spe_context *s
 	return spe->base_private->spe_fds_array[(int)fdesc];
 }
 
-void close_if_open(struct spe_context *spe, enum fd_name fdesc)
-{
-	_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]--;
-		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) {
-		spe->base_private->spe_fds_refcount[(int)fdesc]--;
-	}
-		
-	_base_spe_context_unlock(spe, fdesc);
-}
-
 spe_context_ptr_t _base_spe_context_create(unsigned int flags, spe_gang_context_ptr_t gctx)
 {
 	char pathname[256];