Re: [dpdk-dev] [PATCH v2 01/16] Separate igb_uio mapping into a separate file

[Thomas Monjalon <thomas.monjalon@6wind.com>]

Hi Anatoly,

2014-05-19 16:51, Anatoly Burakov:
> In order to make the code a bit more clean while using multiple
> drivers, IGB_UIO mapping has been separated into its own file.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

[...]
>  /* map a particular resource from a file */
> -static void *
> -pci_map_resource(void *requested_addr, const char *devname, off_t offset,
> +void *
> +pci_map_resource(void *requested_addr, int fd, off_t offset,
>  		 size_t size)
>  {
> -	int fd;
>  	void *mapaddr;
> 
> -	/*
> -	 * open devname, to mmap it
> -	 */
> -	fd = open(devname, O_RDWR);
> -	if (fd < 0) {
> -		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
> -			devname, strerror(errno));
> -		goto fail;
> -	}
> -
>  	/* Map the PCI memory resource of device */
>  	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
>  			MAP_SHARED, fd, offset);
> -	close(fd);
>  	if (mapaddr == MAP_FAILED ||
>  			(requested_addr != NULL && mapaddr != requested_addr)) {
> -		RTE_LOG(ERR, EAL, "%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
> -			" %s (%p)\n", __func__, devname, fd, requested_addr,
> +		RTE_LOG(ERR, EAL, "%s(): cannot mmap(%d, %p, 0x%lx, 0x%lx):"
> +			" %s (%p)\n", __func__, fd, requested_addr,
>  			(unsigned long)size, (unsigned long)offset,
>  			strerror(errno), mapaddr);
>  		goto fail;
[...]
> -static int
> -pci_uio_map_resource(struct rte_pci_device *dev)
[...]
> -		/* if matching map is found, then use it */
> -		if (j != nb_maps) {
> -			offset = j * pagesz;
> -			if (maps[j].addr != NULL ||
> -			    (mapaddr = pci_map_resource(NULL, devname,
> -							(off_t)offset,
> -							(size_t)maps[j].size)
> -			    ) == NULL) {
> -				rte_free(uio_res);
> -				return (-1);
> -			}
> -
> -			maps[j].addr = mapaddr;
> -			maps[j].offset = offset;
> -			dev->mem_resource[i].addr = mapaddr;
> -		}
[...]
> +		/* if matching map is found, then use it */
> +		if (j != nb_maps) {
> +			offset = j * pagesz;
> +
> +			/*
> +			 * open devname, to mmap it
> +			 */
> +			fd = open(uio_res->path, O_RDWR);
> +			if (fd < 0) {
> +				RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
> +					uio_res->path, strerror(errno));
> +				rte_free(uio_res);
> +				return -1;
> +			}
> +
> +			if (maps[j].addr != NULL
> +					|| (mapaddr = pci_map_resource(NULL, fd,
> +							(off_t) offset, (size_t) maps[j].size)) == NULL) 
{
> +				rte_free(uio_res);
> +				close(fd);
> +				return (-1);
> +			}
> +			close(fd);
> +
> +			maps[j].addr = mapaddr;
> +			maps[j].offset = offset;
> +			dev->mem_resource[i].addr = mapaddr;
> +		}

Looking at pci_map_resource(), it seems you are not only moving functions in 
another file.

Please split this patch in a way we can clearly see what is changing.

Thanks
-- 
Thomas