Re: [PATCH 2/5] lib: logic_pio: Add logic_pio_unregister_range()

[Bjorn Helgaas]

On Thu, Jun 20, 2019 at 06:31:53PM +0800, John Garry wrote:
> Add a function to unregister a logical PIO range.
> 
> The method used to allocate LOGIC_PIO_CPU_MMIO regions during registration
> is slightly modified to ensure that we get no overlap when regions are
> unregistered. This is needed because the allocation scheme assumed that no
> regions are ever unregistered.
> 
> Logical PIO space can still be leaked when unregistering certain
> LOGIC_PIO_CPU_MMIO regions, but this acceptable for now since there are no
> callers to unregister LOGIC_PIO_CPU_MMIO regions, and the logical PIO
> region allocation scheme would need significant work to improve this.
> 
> Signed-off-by: John Garry <john.garry@xxxxxxxxxx>
> ---
>  include/linux/logic_pio.h |  1 +
>  lib/logic_pio.c           | 16 +++++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/logic_pio.h b/include/linux/logic_pio.h
> index cbd9d8495690..88e1e6304a71 100644
> --- a/include/linux/logic_pio.h
> +++ b/include/linux/logic_pio.h
> @@ -117,6 +117,7 @@ struct logic_pio_hwaddr *find_io_range_by_fwnode(struct fwnode_handle *fwnode);
>  unsigned long logic_pio_trans_hwaddr(struct fwnode_handle *fwnode,
>  			resource_size_t hw_addr, resource_size_t size);
>  int logic_pio_register_range(struct logic_pio_hwaddr *newrange);
> +void logic_pio_unregister_range(struct logic_pio_hwaddr *range);
>  resource_size_t logic_pio_to_hwaddr(unsigned long pio);
>  unsigned long logic_pio_trans_cpuaddr(resource_size_t hw_addr);
>  
> diff --git a/lib/logic_pio.c b/lib/logic_pio.c
> index 761296376fbc..45eb57af2574 100644
> --- a/lib/logic_pio.c
> +++ b/lib/logic_pio.c
> @@ -56,7 +56,7 @@ int logic_pio_register_range(struct logic_pio_hwaddr *new_range)
>  			/* for MMIO ranges we need to check for overlap */
>  			if (start >= range->hw_start + range->size ||
>  			    end < range->hw_start) {
> -				mmio_sz += range->size;
> +				mmio_sz = range->io_start + range->size;

Should this be renamed to something like "mmio_end"?  Computing a
"size" as "start + size" looks wrong at first glance.  The code overall
probably makes sense, but maybe breaking this out as a separate "avoid
overlaps" patch that renames "mmio_sz" might make it clearer.

>  			} else {
>  				ret = -EFAULT;
>  				goto end_register;
> @@ -98,6 +98,20 @@ int logic_pio_register_range(struct logic_pio_hwaddr *new_range)
>  	return ret;
>  }
>  
> +/**
> + * logic_pio_unregister_range - unregister logical PIO range for a host
> + * @range: pointer to the IO range which has been already registered.
> + *
> + * Unregister a previously-registered IO range node.
> + */
> +void logic_pio_unregister_range(struct logic_pio_hwaddr *range)
> +{
> +	mutex_lock(&io_range_mutex);
> +	list_del_rcu(&range->list);
> +	mutex_unlock(&io_range_mutex);
> +	synchronize_rcu();
> +}
> +
>  /**
>   * find_io_range_by_fwnode - find logical PIO range for given FW node
>   * @fwnode: FW node handle associated with logical PIO range
> -- 
> 2.17.1
>