[zaptel-commits] sruffell: branch sruffell/zaptel-1.4-transcoder r4338 - in /team/sruffell/zap...

SVN commits to the Zaptel project zaptel-commits at lists.digium.com
Mon Jun 2 11:26:55 CDT 2008


Author: sruffell
Date: Mon Jun  2 11:26:55 2008
New Revision: 4338

URL: http://svn.digium.com/view/zaptel?view=rev&rev=4338
Log:
codec_test_threaded passes all tests now.  Starting work on non-blocking
allocate in order to prevent errors with one thread that called into the
wctc4xxp driver is holding the sip channel lock for too long.

Issue: DAHDI-42

Modified:
    team/sruffell/zaptel-1.4-transcoder/kernel/wctc4xxp/base.c
    team/sruffell/zaptel-1.4-transcoder/kernel/zaptel.h
    team/sruffell/zaptel-1.4-transcoder/kernel/zttranscode.c

Modified: team/sruffell/zaptel-1.4-transcoder/kernel/wctc4xxp/base.c
URL: http://svn.digium.com/view/zaptel/team/sruffell/zaptel-1.4-transcoder/kernel/wctc4xxp/base.c?view=diff&rev=4338&r1=4337&r2=4338
==============================================================================
--- team/sruffell/zaptel-1.4-transcoder/kernel/wctc4xxp/base.c (original)
+++ team/sruffell/zaptel-1.4-transcoder/kernel/wctc4xxp/base.c Mon Jun  2 11:26:55 2008
@@ -88,7 +88,7 @@
 #define DTE_FORMAT_UNDEF  0xFF
 
 #define G729_LENGTH 20
-#define G723_LENGTH 20
+#define G723_LENGTH 30
 
 #define G729_SAMPLES 160	/* G.729 */
 #define G723_SAMPLES 240 	/* G.723.1 */
@@ -824,13 +824,16 @@
 {
 	unsigned long flags;
 	struct dte_cmd *cmd, *temp;
+	LIST_HEAD(local_list);
 
 	spin_lock_irqsave(&cpvt->lock, flags);
-	list_for_each_entry_safe(cmd, temp, &cpvt->rx_queue, node) {
+	list_splice_init(&cpvt->rx_queue, &local_list);
+	spin_unlock_irqrestore(&cpvt->lock, flags);
+
+	list_for_each_entry_safe(cmd, temp, &local_list, node) {
 		list_del(&cmd->node);
 		free_cmd(cmd);
 	}
-	spin_unlock_irqrestore(&cpvt->lock, flags);
 }
 
 static int
@@ -849,14 +852,14 @@
 	} else {
 		compl_ztc = &(wc->uencode->channels[index]);
 	}
-	WARN_ON(compl_ztc->chan_built); /* It shouldn't already have been built... */
-	compl_ztc->chan_built = 1;
-	DTE_DEBUG(DTE_DEBUG_CHANNEL_SETUP, "Marking (%p)->chan_built = %d\n", compl_ztc, compl_ztc->chan_built);
+	/* It shouldn't already have been built... */
+	WARN_ON(zt_tc_is_built(compl_ztc));
 	compl_ztc->built_fmts = ztc->dstfmt | ztc->srcfmt;
 	compl_cpvt = compl_ztc->pvt;
 	DTE_DEBUG(DTE_DEBUG_CHANNEL_SETUP, "ztc: %p is the complement to %p\n", compl_ztc, ztc);
 	compl_cpvt->chan_in_num = cpvt->chan_out_num;
 	compl_cpvt->chan_out_num = cpvt->chan_in_num;
+	zt_tc_set_built(compl_ztc);
 	wcdte_cleanup_channel_private(wc, compl_cpvt);
 
 	return 0;
@@ -884,8 +887,7 @@
 	}
 
 	/* If the channel is already built then we're done. */
-	DTE_DEBUG(DTE_DEBUG_CHANNEL_SETUP, "(%p)->chan_built = %d\n", ztc, ztc->chan_built);
-	if (ztc->chan_built) {
+	if (zt_tc_is_built(ztc)) {
 		up(&wc->chansem);
 		DTE_DEBUG(DTE_DEBUG_CHANNEL_SETUP, "Allocating channel %p which is already built.\n", ztc);
 		LEAVING();
@@ -907,7 +909,7 @@
 	}
 
 	/* Mark this channel as built */
-	ztc->chan_built = 1;
+	zt_tc_set_built(ztc);
 	ztc->built_fmts = ztc->dstfmt | ztc->srcfmt;
 
 	/* Mark the channel complement (other half of encoder/decoder pair) as built */
@@ -956,7 +958,7 @@
 	BUG_ON(!compl_ztc);
 
 	/* If the channel complement (other half of the encoder/decoder pair) is being used... */
-	if ((compl_ztc->flags & ZT_TC_FLAG_BUSY)) {
+	if (test_bit(ZT_TC_FLAG_BUSY, &compl_ztc->flags)) {
 		up(&wc->chansem);
 		LEAVING();
 		return -EBUSY;
@@ -973,15 +975,13 @@
 
 	DTE_DEBUG(DTE_DEBUG_CHANNEL_SETUP, "Releasing channel: %p\n", ztc);
 	/* Mark this channel as not built */
-	ztc->chan_built = 0;
-	DTE_DEBUG(DTE_DEBUG_CHANNEL_SETUP, "(%p)->chan_built = %d\n", ztc, ztc->chan_built);
+	zt_tc_clear_built(ztc);
 	ztc->built_fmts = 0;
 	cpvt->chan_in_num = INVALID;
 	cpvt->chan_out_num = INVALID;
 	
 	/* Mark the channel complement as not built */
-	compl_ztc->chan_built = 0;
-	DTE_DEBUG(DTE_DEBUG_CHANNEL_SETUP, "(%p)->chan_built = %d\n", compl_ztc, compl_ztc->chan_built);
+	zt_tc_clear_built(compl_ztc);
 	compl_ztc->built_fmts = 0;
 	compl_cpvt = compl_ztc->pvt;
 	compl_cpvt->chan_in_num = INVALID;
@@ -1132,11 +1132,11 @@
 	}
 
 	if (ZT_FORMAT_G723_1 == ztc->srcfmt) {
-		if (G723_LENGTH != count) {
+		if (G723_5K_BYTES != count) {
 			DTE_DEBUG(DTE_DEBUG_GENERAL, 
 			   "Trying to transcode packet into G723 format " \
 			   "that is %Zu bytes instead of the expected " \
-			   "%d bytes.\n", count, G723_LENGTH);
+			   "%d bytes.\n", count, G723_5K_BYTES);
 			return -EINVAL;
 		}
 	}
@@ -1169,6 +1169,17 @@
 	atomic_inc(&cpvt->stats.packets_sent);
 	LEAVING();
 	return count;
+}
+
+static unsigned int dte_poll(struct file *file, struct poll_table_struct *wait_table)
+{
+	struct zt_transcoder_channel *ztc = file->private_data;
+
+	if (!ztc)
+		return -EINVAL;
+
+	poll_wait(file, &ztc->ready, wait_table);
+	return test_bit(ZT_TC_FLAG_BUSY, &ztc->flags) ? 0 : POLLPRI;
 }
 
 static int dte_operation(struct zt_transcoder_channel *ztc, int op)
@@ -1183,8 +1194,7 @@
 		break;
 	case ZT_TCOP_TRANSCODE:
 		/* This operation is deprecated.  Callers should call read /
-		 * write directly in order to transcode packets. 
-		 */
+		 * write directly in order to transcode packets.  */
 		res = -EINVAL;
 		break;
 	default:
@@ -1359,8 +1369,7 @@
 			/* If NO_AUTO_FREE_CMD is set, then some other thread
 			 * is waiting for this packet to complete transmit and
 			 * will cleanup the command.  Signal that the hardware
-			 * has completed transmitting the command.
-			 */
+			 * has completed transmitting the command.  */
 			complete(&cmd->complete);
 		} else {
 			free_cmd(cmd);
@@ -1840,9 +1849,6 @@
 	ENTERING();
 
 	BUG_ON(!wc || !cpvt);
-	length = (DTE_FORMAT_G729A == complicated) ? G729_LENGTH : 
-		(DTE_FORMAT_G723_1 == complicated) ? G723_LENGTH : 0;
-
 	if (cpvt->encoder) {
 		part1_id = cpvt->timeslot_in_num;
 		part2_id = cpvt->timeslot_out_num;
@@ -1855,6 +1861,9 @@
 		complicated = temp;
 	}
 
+	length = (DTE_FORMAT_G729A == complicated) ? G729_LENGTH : 
+		(DTE_FORMAT_G723_1 == complicated) ? G723_LENGTH : 0;
+
 	/* Create complex channel */
 	if ((res=zt_send_cmd(wc, CMD_MSG_CREATE_CHANNEL(wc->seq_num++, part1_id)))) {
 		return res;
@@ -2131,8 +2140,9 @@
 static void dte_setup_file_operations(struct file_operations *fops)
 {
 	fops->owner = THIS_MODULE;
-	fops->read = dte_read;
+	fops->read =  dte_read;
 	fops->write = dte_write;
+	fops->poll =  dte_poll;
 }
 
 #ifdef CONFIG_DEBUG_FS
@@ -2412,7 +2422,6 @@
 
 	min_numchannels = min(g723_numchannels, g729_numchannels);
 
-#if 1 /* Still working on G.723 with new interface. Limit to G.729 for now. */
 	if (!mode || strlen(mode) < 4) {
 		sprintf(wc->complexname, "G.729a / G.723.1");
 		complexfmts = ZT_FORMAT_G729A | ZT_FORMAT_G723_1;
@@ -2430,11 +2439,6 @@
 		complexfmts = ZT_FORMAT_G729A | ZT_FORMAT_G723_1;
 		wc->numchannels = min_numchannels;
 	}
-#else
-	snprintf(wc->complexname, sizeof(wc->complexname), "G.729a");
-	complexfmts = ZT_FORMAT_G729A;
-	wc->numchannels = g729_numchannels;
-#endif
 
 	if ((res = initialize_encoders(wc, complexfmts))) {
 		goto error_exit_swinit;

Modified: team/sruffell/zaptel-1.4-transcoder/kernel/zaptel.h
URL: http://svn.digium.com/view/zaptel/team/sruffell/zaptel-1.4-transcoder/kernel/zaptel.h?view=diff&rev=4338&r1=4337&r2=4338
==============================================================================
--- team/sruffell/zaptel-1.4-transcoder/kernel/zaptel.h (original)
+++ team/sruffell/zaptel-1.4-transcoder/kernel/zaptel.h Mon Jun  2 11:26:55 2008
@@ -1740,28 +1740,35 @@
 	void *pvt;
 	struct zt_transcoder *parent;
 	wait_queue_head_t ready;
-	int errorstatus;
-	int offset;
-	unsigned int chan_built;
 	unsigned int built_fmts;
-	unsigned int flags;
-	unsigned int srcfmt;
-	unsigned int dstfmt;
-};
-
-#define ZT_TC_FLAG_BUSY       (1 << 0)
-#define ZT_TC_FLAG_TRANSIENT  (1 << 1)
-
+#define ZT_TC_FLAG_BUSY       1
+#define ZT_TC_FLAG_CHAN_BUILT 2 
+	unsigned long flags;
+	unsigned int  srcfmt;
+	unsigned int  dstfmt;
+};
+
+static inline int zt_tc_is_built(struct zt_transcoder_channel *ztc) {
+	return test_bit(ZT_TC_FLAG_CHAN_BUILT, &ztc->flags);
+}
+
+static inline void zt_tc_set_built(struct zt_transcoder_channel *ztc) {
+	set_bit(ZT_TC_FLAG_CHAN_BUILT, &ztc->flags);
+}
+
+static inline void zt_tc_clear_built(struct zt_transcoder_channel *ztc) {
+	clear_bit(ZT_TC_FLAG_CHAN_BUILT, &ztc->flags);
+}
 
 struct zt_transcoder {
-	struct zt_transcoder *next;
+	struct list_head node;
 	char name[80];
 	int numchannels;
 	unsigned int srcfmts;
 	unsigned int dstfmts;
 	int (*operation)(struct zt_transcoder_channel *channel, int op);
 	struct file_operations fops;
-	const struct file_operations *original_fops;
+	/* !!! const struct file_operations *original_fops; */
 	/* Transcoder channels */
 	struct zt_transcoder_channel channels[0];
 };

Modified: team/sruffell/zaptel-1.4-transcoder/kernel/zttranscode.c
URL: http://svn.digium.com/view/zaptel/team/sruffell/zaptel-1.4-transcoder/kernel/zttranscode.c?view=diff&rev=4338&r1=4337&r2=4338
==============================================================================
--- team/sruffell/zaptel-1.4-transcoder/kernel/zttranscode.c (original)
+++ team/sruffell/zaptel-1.4-transcoder/kernel/zttranscode.c Mon Jun  2 11:26:55 2008
@@ -49,8 +49,8 @@
 #include <linux/moduleparam.h>
 #endif
 
-static int debug = 0;
-static struct zt_transcoder *trans;
+static int debug;
+LIST_HEAD(trans);
 static spinlock_t translock = SPIN_LOCK_UNLOCKED;
 
 EXPORT_SYMBOL(zt_transcoder_register);
@@ -73,17 +73,14 @@
 	ztc->numchannels = numchans;
 	for (x=0;x<ztc->numchannels;x++) {
 		init_waitqueue_head(&ztc->channels[x].ready);
+		INIT_LIST_HEAD(&ztc->node);
 		ztc->channels[x].parent = ztc;
-		ztc->channels[x].offset = x;
-		ztc->channels[x].chan_built = 0;
-		ztc->channels[x].built_fmts = 0;
 	}
 
 	WARN_ON(!zt_transcode_fops);
 	/* Individual transcoders should supply their own file_operations for
 	 * write and read.  But they will by default use the file_operations
-	 * provided by the zttranscode layer. 
-	 */
+	 * provided by the zttranscode layer.  */
 	memcpy(&ztc->fops, zt_transcode_fops, sizeof(*zt_transcode_fops));
 	return ztc;
 }
@@ -93,21 +90,23 @@
 	kfree(ztc);
 }
 
+/* Returns 1 if the item is on the list pointed to by head, otherwise, returns
+ * 0 */
+static int is_on_list(struct list_head *entry, struct list_head *head)
+{
+	struct list_head *cur;
+	list_for_each(cur, head) {
+		if (cur == entry) return 1;
+	}
+	return 0;
+}
+
 /* Register a transcoder */
 int zt_transcoder_register(struct zt_transcoder *tc)
 {
-	struct zt_transcoder *cur;
-
 	spin_lock(&translock);
-	for (cur = trans; cur; cur = cur->next) {
-		if (cur == tc) {
-			spin_unlock(&translock);
-			return -EBUSY;
-		}
-	}
-
-	tc->next = trans;
-	trans = tc;
+	BUG_ON(is_on_list(&tc->node, &trans));
+	list_add_tail(&tc->node, &trans);
 	spin_unlock(&translock);
 
 	printk(KERN_INFO "%s: Registered codec translator '%s' " \
@@ -121,25 +120,19 @@
 /* Unregister a transcoder */
 int zt_transcoder_unregister(struct zt_transcoder *tc) 
 {
-	struct zt_transcoder *cur, *prev;
 	int res = -EINVAL;
 
+	/* \todo Perhaps we should check to make sure there isn't a channel
+	 * that is still in use? */
+
 	spin_lock(&translock);
-	for (cur = trans, prev = NULL; cur; prev = cur, cur = cur->next) {
-		if (cur == tc)
-			break;
-	}
-
-	if (!cur) {
+	if (!is_on_list(&tc->node, &trans)) {
 		spin_unlock(&translock);
-		return res;
-	}
-
-	if (prev)
-		prev->next = tc->next;
-	else
-		trans = tc->next;
-	tc->next = NULL;
+		printk(KERN_WARNING "%s: Failed to unregister %s, which is " \
+		       "not currently registerd.\n", THIS_MODULE->name, tc->name);
+		return -EINVAL;
+	}
+	list_del_init(&tc->node);
 	spin_unlock(&translock);
 
 	printk(KERN_INFO "Unregistered codec translator '%s' with %d " \
@@ -167,9 +160,10 @@
 	original_fops = file->f_op;
 	file->f_op = zt_transcode_fops;
 	file->private_data = NULL;
-	/* Under normal operation, this releases the reference on zaptel that
-	 * was created when the file was opened. zt_open is responsible for
-	 * taking a reference out on this module before calling this function. 
+	/* Under normal operation, this releases the reference on the zaptel
+	 * module that was created when the file was opened. zt_open is
+	 * responsible for taking a reference out on this module before
+	 * calling this function. 
 	 */
 	module_put(original_fops->owner);
 	return 0;
@@ -181,9 +175,7 @@
 	if (ztc->parent && ztc->parent->operation) {
 		ztc->parent->operation(ztc, ZT_TCOP_RELEASE);
 	}
-	spin_lock(&translock);
-	ztc->flags &= ~(ZT_TC_FLAG_BUSY);
-	spin_unlock(&translock);
+	clear_bit(ZT_TC_FLAG_BUSY, &ztc->flags);
 }
 
 static int zt_tc_release(struct inode *inode, struct file *file)
@@ -203,13 +195,16 @@
 get_free_channel(struct zt_transcoder *tc)
 
 {
+	struct zt_transcoder_channel *ztc;
 	int i;
 	/* Should be called with the translock held. */
 	WARN_ON(!spin_is_locked(&translock));
+
 	for (i = 0; i < tc->numchannels; i++) {
-		if (!(tc->channels[i].flags & ZT_TC_FLAG_BUSY)) {
-			tc->channels[i].flags |= ZT_TC_FLAG_BUSY;
-			return &tc->channels[i];
+		ztc = &tc->channels[i];
+		if (!test_bit(ZT_TC_FLAG_BUSY, &ztc->flags)) {
+			set_bit(ZT_TC_FLAG_BUSY, &ztc->flags);
+			return ztc;
 		}
 	}
 	return NULL;
@@ -228,12 +223,14 @@
 
 	/* Find new transcoder */
 	spin_lock(&translock);
-	for (tc = trans; tc && !ztc; tc = tc->next) {
+	list_for_each_entry(tc, &trans, node) {
 		if ((tc->dstfmts & fmts.dstfmt)) {
 			/* We found a transcoder that can handle our formats.
 			 * Now look for an available channel. */
 			match = 1;
-			ztc = get_free_channel(tc);
+			if ((ztc = get_free_channel(tc))) {
+				break;
+			}
 		}
 	}
 	spin_unlock(&translock);
@@ -249,8 +246,7 @@
 
 	if (file->private_data) {
 		/* This open file is moving to a new channel. Cleanup and
-		 * close the old channel here. 
-		 */
+		 * close the old channel here.  */
 		ztc_release(file->private_data);
 	}
 
@@ -258,8 +254,7 @@
 	if (ztc->parent->fops.owner != file->f_op->owner) {
 		if (!try_module_get(ztc->parent->fops.owner)) {
 			/* Failed to get a reference on the driver for the
-			 * actual transcoding hardware. 
-			 */
+			 * actual transcoding hardware.  */
 			return -EINVAL;
 		}
 		/* Release the reference on the existing driver. */
@@ -278,24 +273,33 @@
 {
 	struct zt_transcoder_info info;
 	unsigned int x;
-	struct zt_transcoder *tc;
+	struct zt_transcoder *cur;
+	struct zt_transcoder *tc = NULL;
 	
-	if (copy_from_user(&info, (struct zt_transcoder_info *) data, sizeof(info)))
+	if (copy_from_user(&info, (const void *) data, sizeof(info))) {
 		return -EFAULT;
-
+	}
+
+	x = 0;
 	spin_lock(&translock);
-	for (tc = trans, x = info.tcnum; tc && x; tc = tc->next, x--);
+	list_for_each_entry(cur, &trans, node) {
+		if (x++ == info.tcnum) {
+			tc = cur;
+			break;
+		} 
+	}
 	spin_unlock(&translock);
 
-	if (!tc)
+	if (!tc) {
 		return -ENOSYS;
+	}
 
 	zap_copy_string(info.name, tc->name, sizeof(info.name));
 	info.numchannels = tc->numchannels;
 	info.srcfmts = tc->srcfmts;
 	info.dstfmts = tc->dstfmts;
 
-	return copy_to_user((struct zt_transcoder_info *) data, &info, sizeof(info)) ? -EFAULT : 0;
+	return copy_to_user((void *) data, &info, sizeof(info)) ? -EFAULT : 0;
 }
 
 static ssize_t zt_tc_write(struct file *file, __user const char *usrbuf, size_t count, loff_t *ppos)
@@ -359,7 +363,7 @@
 		return -EINVAL;
 
 	poll_wait(file, &ztc->ready, wait_table);
-	return ztc->flags & ZT_TC_FLAG_BUSY ? 0 : POLLPRI;
+	return test_bit(ZT_TC_FLAG_BUSY, &ztc->flags) ? 0 : POLLPRI;
 }
 
 static struct file_operations __zt_transcode_fops = {




More information about the zaptel-commits mailing list