[Libdlo] [PATCH] udlfb: high-throughput urb pool

[Greg KH]

On Fri, Dec 17, 2010 at 02:17:34PM -0600, Andrew Kephart wrote:
> udlfb: add parameters for URB pool size and for nonblocking URB pool
> 
> For high-throughput usage (e.g. mplayer -vo fbdev), more than 4 URBs may
> be needed to keep the pipeline fed.  This patch adds a module parameter
> for specifying the initial depth of the URB pool.  This patch also adds
> a boolean module parameter that enables a "nowait" mode, wherein no
> semaphore is used (only the spinlock is needed for list coherency). 
> 
> In my testing with defio mode, I found that the delayed work mechanism
> (for releasing the URB semaphore) could not be scheduled quickly enough,
> causing unnecessary delays between URB availability and semaphore
> release yielding pool exhaustion regardless of initial depth.  By adding
> the nowait mode (i.e. removing the semaphore), URBs are made available
> immediately upon completion, even if defio is active.  
> 
> In cases where you have low bandwidth requirements, but with occasional
> small burstiness, the nowait mode has a higher probability of lost pixel
> data, but in the higher-bandwidth conditions, the nowait mode actually
> has a much lower probability of pixel loss, since the URBs are much more
> likely to be available at request time.  
> 
> For tuning purposes, this patch also adds URB cache stats to the sysfs
> metrics.
> 
> We may not want to incorporate this into udlfb as-is, but it marks a
> starting point for the throughput discussion....
> 
> -andrew
> 
> ---
> 
> 
> 
> diff --git a/udlfb.c b/udlfb.c
> index 03ba2d9..8b4c075 100644
> --- a/udlfb.c
> +++ b/udlfb.c
> @@ -67,6 +67,8 @@ MODULE_DEVICE_TABLE(usb, id_table);
>  /* module options */
>  static int console;   /* Optionally allow fbcon to consume first framebuffer */
>  static int fb_defio;  /* Optionally enable experimental fb_defio mmap support */
> +static int num_urb=WRITES_IN_FLIGHT;  /* Number of URBs to pre-allocate */
> +static int urb_nowait=0; /* Optionally enable nowait mode */
>  
>  /*
>   * When building as a separate module against an arbitrary kernel,
> @@ -1001,6 +1003,12 @@ static void dlfb_free(struct kref *kref)
>  
>         dl_warn("freeing dlfb_data %p\n", dev);
>  
> +    if (urb_nowait)
> +        dl_info("URB cache hits: %d, misses: %d, min_avail: %d\n",
> +                atomic_read(&dev->urb_cache_hits),
> +                atomic_read(&dev->urb_cache_misses),
> +                atomic_read(&dev->urb_cache_min));
> +
>         kfree(dev);
>  }
>  
> @@ -1454,6 +1462,38 @@ static ssize_t metrics_cpu_kcycles_used_show(struct device *fbdev,
>                         atomic_read(&dev->cpu_kcycles_used));
>  }
>  
> +static ssize_t metrics_urb_cache_hits_show(struct device *fbdev,
> +                                  struct device_attribute *a, char *buf) {
> +       struct fb_info *fb_info = dev_get_drvdata(fbdev);
> +       struct dlfb_data *dev = fb_info->par;
> +       return snprintf(buf, PAGE_SIZE, "%u\n",
> +                       atomic_read(&dev->urb_cache_hits));
> +}

All of these "metrics" should go into debugfs, not sysfs, in the end, as
that is what it is there for.

thanks,

greg k-h