Re: [PATCH] uio: add UIO_MEM_CUSTOM support

Re: [PATCH] uio: add UIO_MEM_CUSTOM support

[Greg KH]

On Wed, Feb 15, 2017 at 12:43:06PM +0800, lixiubo@cmss.chinamobile.com wrote:
> From: Xiubo Li <lixiubo@cmss.chinamobile.com>
> 
> This will allow UIO based drivers, like TCMU, have opportunities to
> implement their own mmap magics.
> 
> Signed-off-by: Xiubo Li <lixiubo@cmss.chinamobile.com>
> ---
>  drivers/uio/uio.c          | 2 ++
>  include/linux/uio_driver.h | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> index fba021f..6ca0ae0 100644
> --- a/drivers/uio/uio.c
> +++ b/drivers/uio/uio.c
> @@ -708,6 +708,8 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma)
>  		case UIO_MEM_LOGICAL:
>  		case UIO_MEM_VIRTUAL:
>  			return uio_mmap_logical(vma);
> +		case UIO_MEM_CUSTOM:
> +			return 0;

How does this help?

Can you provide an update to the documentation in
Documentation/driver-api/uio-howto.rst to show how to use this?

thanks,

greg k-h

Re: [PATCH] uio: add UIO_MEM_CUSTOM support

[Xiubo Li]

>> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
>> index fba021f..6ca0ae0 100644
>> --- a/drivers/uio/uio.c
>> +++ b/drivers/uio/uio.c
>> @@ -708,6 +708,8 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma)
>>   		case UIO_MEM_LOGICAL:
>>   		case UIO_MEM_VIRTUAL:
>>   			return uio_mmap_logical(vma);
>> +		case UIO_MEM_CUSTOM:
>> +			return 0;
> How does this help?
For example, the TCMU will use the map area as ISCSI commands & data ring
buffer(uio0 --> map0). Currently the TCMU will using the fixed small 
size map
area as the ring buffer, but this will be the bottleneck for high iops.

Without knowing how large it is enough, so the new scheme will use the fixed
small ring buffer area(about 64M ~ 128M) + dynamically "growing" ring buffer
area(about 1.5G).

The fixed small area will be using vmalloc() when initializing, and 
dynamically
"growing" area will must use kmalloc() for some reasons.

...

> Can you provide an update to the documentation in
> Documentation/driver-api/uio-howto.rst to show how to use this?
Yes, I will update this.

Thanks.

BRs
Xiubo

> thanks,
>
> greg k-h

Re: [PATCH] uio: add UIO_MEM_CUSTOM support

[Andy Grover]

On 02/15/2017 05:34 PM, Xiubo Li wrote:
>>> --- a/drivers/uio/uio.c
>>> +++ b/drivers/uio/uio.c
>>> @@ -708,6 +708,8 @@ static int uio_mmap(struct file *filep, struct
>>> vm_area_struct *vma)
>>>           case UIO_MEM_LOGICAL:
>>>           case UIO_MEM_VIRTUAL:
>>>               return uio_mmap_logical(vma);
>>> +        case UIO_MEM_CUSTOM:
>>> +            return 0;
>> How does this help?
> For example, the TCMU will use the map area as ISCSI commands & data ring
> buffer(uio0 --> map0). Currently the TCMU will using the fixed small
> size map
> area as the ring buffer, but this will be the bottleneck for high iops.
> 
> Without knowing how large it is enough, so the new scheme will use the
> fixed
> small ring buffer area(about 64M ~ 128M) + dynamically "growing" ring
> buffer
> area(about 1.5G).

The following code is in uio_mmap() in uio.c:

if (idev->info->mmap) {
	ret = idev->info->mmap(idev->info, vma);
	return ret;
}

switch (idev->info->mem[mi].memtype) {
	case UIO_MEM_PHYS:
		return uio_mmap_physical(vma);
	case UIO_MEM_LOGICAL:
	case UIO_MEM_VIRTUAL:
		return uio_mmap_logical(vma);
	default:
		return -EINVAL;
}

We already have the equivalent of a CUSTOM memtype because TCMU sets the
info->mmap fn, overriding uio's default handling choices in favor of its
own.

HTH -- Regards -- Andy

Re: [PATCH] uio: add UIO_MEM_CUSTOM support

[Xiubo Li]

>> For example, the TCMU will use the map area as ISCSI commands & data ring
>> buffer(uio0 --> map0). Currently the TCMU will using the fixed small
>> size map
>> area as the ring buffer, but this will be the bottleneck for high iops.
>>
>> Without knowing how large it is enough, so the new scheme will use the
>> fixed
>> small ring buffer area(about 64M ~ 128M) + dynamically "growing" ring
>> buffer
>> area(about 1.5G).
> The following code is in uio_mmap() in uio.c:
>
> if (idev->info->mmap) {
> 	ret = idev->info->mmap(idev->info, vma);
> 	return ret;

Yes, just missed this return.
> }
>
> switch (idev->info->mem[mi].memtype) {
> 	case UIO_MEM_PHYS:
> 		return uio_mmap_physical(vma);
> 	case UIO_MEM_LOGICAL:
> 	case UIO_MEM_VIRTUAL:
> 		return uio_mmap_logical(vma);
> 	default:
> 		return -EINVAL;
> }
>
> We already have the equivalent of a CUSTOM memtype because TCMU sets the
> info->mmap fn, overriding uio's default handling choices in favor of its
> own.
>
> HTH -- Regards -- Andy
>

Re: [PATCH] uio: add UIO_MEM_CUSTOM support

[Xiubo Li]

>> buffer(uio0 --> map0). Currently the TCMU will using the fixed small
>> size map
>> area as the ring buffer, but this will be the bottleneck for high iops.
>>
>> Without knowing how large it is enough, so the new scheme will use the
>> fixed
>> small ring buffer area(about 64M ~ 128M) + dynamically "growing" ring
>> buffer
>> area(about 1.5G).
> The following code is in uio_mmap() in uio.c:
>
> if (idev->info->mmap) {
> 	ret = idev->info->mmap(idev->info, vma);
> 	return ret;
> }
>
> switch (idev->info->mem[mi].memtype) {
> 	case UIO_MEM_PHYS:
> 		return uio_mmap_physical(vma);
> 	case UIO_MEM_LOGICAL:
> 	case UIO_MEM_VIRTUAL:
> 		return uio_mmap_logical(vma);
> 	default:
> 		return -EINVAL;
> }
>
> We already have the equivalent of a CUSTOM memtype because TCMU sets the
> info->mmap fn, overriding uio's default handling choices in favor of its
> own.
For the driver like TCMU, the UIO_MEM_NONE could be used with
implementing its own ->mmap() magic, instead of adding new
UIO_MEM_CUSTOM. But will the semantic be clearer by using
_CUSTOM instead of _NONE  ?

Thanks,

BRs
Xiubo
> HTH -- Regards -- Andy
>