[PATCH] Speed up DVB TS stream delivery from DMA buffer into dvb-core's buffer

[PATCH] Speed up DVB TS stream delivery from DMA buffer into dvb-core's buffer

[Marko Ristola]

Avoid unnecessary DVB TS 188 sized packet copying from DMA buffer into stack.
Backtrack one 188 sized packet just after some garbage bytes when possible.
This obsoletes patch https://patchwork.kernel.org/patch/118147/

Signed-off-by: Marko Ristola marko.ristola@kolumbus.fi
diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c b/drivers/media/dvb/dvb-core/dvb_demux.c
index 4a88a3e..faa3671 100644
--- a/drivers/media/dvb/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb/dvb-core/dvb_demux.c
@@ -478,97 +478,94 @@ void dvb_dmx_swfilter_packets(struct dvb_demux *demux, const u8 *buf,
 
 EXPORT_SYMBOL(dvb_dmx_swfilter_packets);
 
-void dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, size_t count)
+static inline int find_next_packet(const u8 *buf, int pos, size_t count,
+				   const int pktsize)
 {
-	int p = 0, i, j;
+	int start = pos, lost;
 
-	spin_lock(&demux->lock);
-
-	if (demux->tsbufp) {
-		i = demux->tsbufp;
-		j = 188 - i;
-		if (count < j) {
-			memcpy(&demux->tsbuf[i], buf, count);
-			demux->tsbufp += count;
-			goto bailout;
-		}
-		memcpy(&demux->tsbuf[i], buf, j);
-		if (demux->tsbuf[0] == 0x47)
-			dvb_dmx_swfilter_packet(demux, demux->tsbuf);
-		demux->tsbufp = 0;
-		p += j;
+	while (pos < count) {
+		if (buf[pos] == 0x47 ||
+		    (pktsize == 204 && buf[pos] == 0xB8))
+			break;
+		pos++;
 	}
 
-	while (p < count) {
-		if (buf[p] == 0x47) {
-			if (count - p >= 188) {
-				dvb_dmx_swfilter_packet(demux, &buf[p]);
-				p += 188;
-			} else {
-				i = count - p;
-				memcpy(demux->tsbuf, &buf[p], i);
-				demux->tsbufp = i;
-				goto bailout;
-			}
-		} else
-			p++;
+	lost = pos - start;
+	if (lost) {
+		/* This garbage is part of a valid packet? */
+		int backtrack = pos - pktsize;
+		if (backtrack >= 0 && (buf[backtrack] == 0x47 ||
+		    (pktsize == 204 && buf[backtrack] == 0xB8)))
+			return backtrack;
 	}
 
-bailout:
-	spin_unlock(&demux->lock);
+	return pos;
 }
 
-EXPORT_SYMBOL(dvb_dmx_swfilter);
-
-void dvb_dmx_swfilter_204(struct dvb_demux *demux, const u8 *buf, size_t count)
+/* Filter all pktsize= 188 or 204 sized packets and skip garbage. */
+static inline void _dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf,
+		size_t count, const int pktsize)
 {
 	int p = 0, i, j;
-	u8 tmppack[188];
+	const u8 *q;
 
 	spin_lock(&demux->lock);
 
-	if (demux->tsbufp) {
+	if (demux->tsbufp) { /* tsbuf[0] is now 0x47. */
 		i = demux->tsbufp;
-		j = 204 - i;
+		j = pktsize - i;
 		if (count < j) {
 			memcpy(&demux->tsbuf[i], buf, count);
 			demux->tsbufp += count;
 			goto bailout;
 		}
 		memcpy(&demux->tsbuf[i], buf, j);
-		if ((demux->tsbuf[0] == 0x47) || (demux->tsbuf[0] == 0xB8)) {
-			memcpy(tmppack, demux->tsbuf, 188);
-			if (tmppack[0] == 0xB8)
-				tmppack[0] = 0x47;
-			dvb_dmx_swfilter_packet(demux, tmppack);
-		}
+		if (demux->tsbuf[0] == 0x47) /* double check */
+			dvb_dmx_swfilter_packet(demux, demux->tsbuf);
 		demux->tsbufp = 0;
 		p += j;
 	}
 
-	while (p < count) {
-		if ((buf[p] == 0x47) || (buf[p] == 0xB8)) {
-			if (count - p >= 204) {
-				memcpy(tmppack, &buf[p], 188);
-				if (tmppack[0] == 0xB8)
-					tmppack[0] = 0x47;
-				dvb_dmx_swfilter_packet(demux, tmppack);
-				p += 204;
-			} else {
-				i = count - p;
-				memcpy(demux->tsbuf, &buf[p], i);
-				demux->tsbufp = i;
-				goto bailout;
-			}
-		} else {
-			p++;
+	while (1) {
+		p = find_next_packet(buf, p, count, pktsize);
+		if (p >= count)
+			break;
+		if (count - p < pktsize)
+			break;
+
+		q = &buf[p];
+
+		if (pktsize == 204 && (*q == 0xB8)) {
+			memcpy(demux->tsbuf, q, 188);
+			demux->tsbuf[0] = 0x47;
+			q = demux->tsbuf;
 		}
+		dvb_dmx_swfilter_packet(demux, q);
+		p += pktsize;
+	}
+
+	i = count - p;
+	if (i) {
+		memcpy(demux->tsbuf, &buf[p], i);
+		demux->tsbufp = i;
+		if (pktsize == 204 && demux->tsbuf[0] == 0xB8)
+			demux->tsbuf[0] = 0x47;
 	}
 
 bailout:
 	spin_unlock(&demux->lock);
 }
 
+void dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, size_t count)
+{
+	_dvb_dmx_swfilter(demux, buf, count, 188);
+}
+EXPORT_SYMBOL(dvb_dmx_swfilter);
+
+void dvb_dmx_swfilter_204(struct dvb_demux *demux, const u8 *buf, size_t count)
+{
+	_dvb_dmx_swfilter(demux, buf, count, 204);
+}
 EXPORT_SYMBOL(dvb_dmx_swfilter_204);
 
 static struct dvb_demux_filter *dvb_dmx_filter_alloc(struct dvb_demux *demux)

Re: [PATCH] Speed up DVB TS stream delivery from DMA buffer into dvb-core's buffer

[Marko Ristola]

Here is some statistics without likely and with likely functions.

It seems that using likely() gives better performance with Phenom I too.

	%	Plain knl %	No likely	%	With likely	%
dvb_ringbuffer_write	5,9	62,8	8,7	81,3	5,7	79,2
dvb_dmx_swfilter_packet	1,2	12,8	0,7	6,5	0,8	11,1
dvb_dmx_swfilter_204	2,3	24,5	1,3	12,1	0,7	9,7


Here "Plain knl %" is "perf top -d 30" percentage.
24,5 12,1 and 9,7 are percentages without a patch, with basic patch
and last is with "likely" functions using patch.

Regards,
Marko Ristola


08.04.2011 18:40, Marko Ristola kirjoitti:
> Avoid unnecessary DVB TS 188 sized packet copying from DMA buffer into stack.
> Backtrack one 188 sized packet just after some garbage bytes when possible.
> This obsoletes patch https://patchwork.kernel.org/patch/118147/
> 
> Signed-off-by: Marko Ristola marko.ristola@kolumbus.fi
> diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c b/drivers/media/dvb/dvb-core/dvb_demux.c
> index 4a88a3e..faa3671 100644
> --- a/drivers/media/dvb/dvb-core/dvb_demux.c
> +++ b/drivers/media/dvb/dvb-core/dvb_demux.c
> @@ -478,97 +478,94 @@ void dvb_dmx_swfilter_packets(struct dvb_demux *demux, const u8 *buf,
>  
>  EXPORT_SYMBOL(dvb_dmx_swfilter_packets);
>  

Re: [PATCH] Speed up DVB TS stream delivery from DMA buffer into dvb-core's buffer

[Andreas Oberritter]

On 04/08/2011 05:40 PM, Marko Ristola wrote:
> Avoid unnecessary DVB TS 188 sized packet copying from DMA buffer into stack.
> Backtrack one 188 sized packet just after some garbage bytes when possible.
> This obsoletes patch https://patchwork.kernel.org/patch/118147/
> 
> Signed-off-by: Marko Ristola marko.ristola@kolumbus.fi

Did you intentionally send a version that doesn't use likely()? Anyway,
those optimizations can still be done in an incremental patch.

Acked-by: Andreas Oberritter <obi@linuxtv.org>

> diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c b/drivers/media/dvb/dvb-core/dvb_demux.c
> index 4a88a3e..faa3671 100644
> --- a/drivers/media/dvb/dvb-core/dvb_demux.c
> +++ b/drivers/media/dvb/dvb-core/dvb_demux.c
> @@ -478,97 +478,94 @@ void dvb_dmx_swfilter_packets(struct dvb_demux *demux, const u8 *buf,
>  
>  EXPORT_SYMBOL(dvb_dmx_swfilter_packets);
>  
> -void dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, size_t count)
> +static inline int find_next_packet(const u8 *buf, int pos, size_t count,
> +				   const int pktsize)
>  {
> -	int p = 0, i, j;
> +	int start = pos, lost;
>  
> -	spin_lock(&demux->lock);
> -
> -	if (demux->tsbufp) {
> -		i = demux->tsbufp;
> -		j = 188 - i;
> -		if (count < j) {
> -			memcpy(&demux->tsbuf[i], buf, count);
> -			demux->tsbufp += count;
> -			goto bailout;
> -		}
> -		memcpy(&demux->tsbuf[i], buf, j);
> -		if (demux->tsbuf[0] == 0x47)
> -			dvb_dmx_swfilter_packet(demux, demux->tsbuf);
> -		demux->tsbufp = 0;
> -		p += j;
> +	while (pos < count) {
> +		if (buf[pos] == 0x47 ||
> +		    (pktsize == 204 && buf[pos] == 0xB8))
> +			break;
> +		pos++;
>  	}
>  
> -	while (p < count) {
> -		if (buf[p] == 0x47) {
> -			if (count - p >= 188) {
> -				dvb_dmx_swfilter_packet(demux, &buf[p]);
> -				p += 188;
> -			} else {
> -				i = count - p;
> -				memcpy(demux->tsbuf, &buf[p], i);
> -				demux->tsbufp = i;
> -				goto bailout;
> -			}
> -		} else
> -			p++;
> +	lost = pos - start;
> +	if (lost) {
> +		/* This garbage is part of a valid packet? */
> +		int backtrack = pos - pktsize;
> +		if (backtrack >= 0 && (buf[backtrack] == 0x47 ||
> +		    (pktsize == 204 && buf[backtrack] == 0xB8)))
> +			return backtrack;
>  	}
>  
> -bailout:
> -	spin_unlock(&demux->lock);
> +	return pos;
>  }
>  
> -EXPORT_SYMBOL(dvb_dmx_swfilter);
> -
> -void dvb_dmx_swfilter_204(struct dvb_demux *demux, const u8 *buf, size_t count)
> +/* Filter all pktsize= 188 or 204 sized packets and skip garbage. */
> +static inline void _dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf,
> +		size_t count, const int pktsize)
>  {
>  	int p = 0, i, j;
> -	u8 tmppack[188];
> +	const u8 *q;
>  
>  	spin_lock(&demux->lock);
>  
> -	if (demux->tsbufp) {
> +	if (demux->tsbufp) { /* tsbuf[0] is now 0x47. */
>  		i = demux->tsbufp;
> -		j = 204 - i;
> +		j = pktsize - i;
>  		if (count < j) {
>  			memcpy(&demux->tsbuf[i], buf, count);
>  			demux->tsbufp += count;
>  			goto bailout;
>  		}
>  		memcpy(&demux->tsbuf[i], buf, j);
> -		if ((demux->tsbuf[0] == 0x47) || (demux->tsbuf[0] == 0xB8)) {
> -			memcpy(tmppack, demux->tsbuf, 188);
> -			if (tmppack[0] == 0xB8)
> -				tmppack[0] = 0x47;
> -			dvb_dmx_swfilter_packet(demux, tmppack);
> -		}
> +		if (demux->tsbuf[0] == 0x47) /* double check */
> +			dvb_dmx_swfilter_packet(demux, demux->tsbuf);
>  		demux->tsbufp = 0;
>  		p += j;
>  	}
>  
> -	while (p < count) {
> -		if ((buf[p] == 0x47) || (buf[p] == 0xB8)) {
> -			if (count - p >= 204) {
> -				memcpy(tmppack, &buf[p], 188);
> -				if (tmppack[0] == 0xB8)
> -					tmppack[0] = 0x47;
> -				dvb_dmx_swfilter_packet(demux, tmppack);
> -				p += 204;
> -			} else {
> -				i = count - p;
> -				memcpy(demux->tsbuf, &buf[p], i);
> -				demux->tsbufp = i;
> -				goto bailout;
> -			}
> -		} else {
> -			p++;
> +	while (1) {
> +		p = find_next_packet(buf, p, count, pktsize);
> +		if (p >= count)
> +			break;
> +		if (count - p < pktsize)
> +			break;
> +
> +		q = &buf[p];
> +
> +		if (pktsize == 204 && (*q == 0xB8)) {
> +			memcpy(demux->tsbuf, q, 188);
> +			demux->tsbuf[0] = 0x47;
> +			q = demux->tsbuf;
>  		}
> +		dvb_dmx_swfilter_packet(demux, q);
> +		p += pktsize;
> +	}
> +
> +	i = count - p;
> +	if (i) {
> +		memcpy(demux->tsbuf, &buf[p], i);
> +		demux->tsbufp = i;
> +		if (pktsize == 204 && demux->tsbuf[0] == 0xB8)
> +			demux->tsbuf[0] = 0x47;
>  	}
>  
>  bailout:
>  	spin_unlock(&demux->lock);
>  }
>  
> +void dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, size_t count)
> +{
> +	_dvb_dmx_swfilter(demux, buf, count, 188);
> +}
> +EXPORT_SYMBOL(dvb_dmx_swfilter);
> +
> +void dvb_dmx_swfilter_204(struct dvb_demux *demux, const u8 *buf, size_t count)
> +{
> +	_dvb_dmx_swfilter(demux, buf, count, 204);
> +}
>  EXPORT_SYMBOL(dvb_dmx_swfilter_204);
>  
>  static struct dvb_demux_filter *dvb_dmx_filter_alloc(struct dvb_demux *demux)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Speed up DVB TS stream delivery from DMA buffer into dvb-core's buffer

[Andreas Oberritter]

On 04/08/2011 09:37 PM, Marko Ristola wrote:
> 
> Here is some statistics without likely and with likely functions.
> 
> It seems that using likely() gives better performance with Phenom I too.

I'm not sure whether that's the right conclusion. See below.

> 	%	Plain knl %	No likely	%	With likely	%
> dvb_ringbuffer_write		5,9	62,8	8,7	81,3	5,7	79,2
> dvb_dmx_swfilter_packet	1,2	12,8	0,7	6,5	0,8	11,1
> dvb_dmx_swfilter_204		2,3	24,5	1,3	12,1	0,7	9,7
> 
> 
> Here "Plain knl %" is "perf top -d 30" percentage.
> 24,5 12,1 and 9,7 are percentages without a patch, with basic patch
> and last is with "likely" functions using patch.

The varying ratio between dvb_ringbuffer_write and
dvb_dmx_swfilter_packet, which haven't been modified by the patch, seems
to indicate that the numbers were influenced by other activities on the
system or by the data used during the tests. 8,7% for
dvb_ringbuffer_write in test #2 could also indicate a higher data rate
than in #1 and #3.

However, adding likely() certainly won't do any harm. Feel free to send
an incremental patch.

Regards,
Andreas