[openib-general] [PATCH 06/12] SRP: Changing ibsrpdm

[Muli Ben-Yehuda]

On Mon, May 01, 2006 at 02:28:48PM +0300, Ishai Rabinovitz wrote:
> 
> Support a display of list of target from user level.
> 
> Signed-off-by: Ishai Rabinovitz <ishai at mellanox.co.il>
> Index: last_stable/drivers/infiniband/ulp/srp/ib_srp.c
> ===================================================================
> --- last_stable.orig/drivers/infiniband/ulp/srp/ib_srp.c	2006-04-21 01:13:04.000000000 +0300
> +++ last_stable/drivers/infiniband/ulp/srp/ib_srp.c	2006-04-21 03:56:05.000000000 +0300
> @@ -1730,6 +1730,63 @@ end:
>  
>  static CLASS_DEVICE_ATTR(remove_target, S_IWUSR, NULL, srp_remove_target);
>  
> +#define TARGET_INFO_BUF_SIZE 126
> +
> +static ssize_t list_targets(struct class_device *class_dev, char *buf)
> +{
> +	struct srp_host *host =
> +		container_of(class_dev, struct srp_host, class_dev);
> +	struct srp_target_port *target;
> +	int printed=0, ret;
> +
> +	mutex_lock(&host->target_mutex);
> +	list_for_each_entry(target, &host->target_list, list)

Can this race with list addition / removal? I saw that you removed the
lock in an earlier patch?

> +		if (target->state == SRP_TARGET_LIVE) {

You'd have an easier time with the indentation if you'd do 

if (target->state != SRP_TARGET_LIVE)
        continue;

here

> +			ret = sprintf(buf+printed,
> +				"id_ext=%016llx,ioc_guid=%016llx,"
> +				"dgid=%04x%04x%04x%04x%04x%04x%04x%04x,"
> +				"pkey=%04x,service_id=%016llx\n",
> +				(unsigned long long)
> +					be64_to_cpu(target->id_ext),
> +				(unsigned long long)
> +					be64_to_cpu(target->ioc_guid),
> +				(int) be16_to_cpu(*(__be16 *)
> +					&target->path.dgid.raw[0]),
> +				(int) be16_to_cpu(*(__be16 *)
> +					&target->path.dgid.raw[2]),
> +				(int) be16_to_cpu(*(__be16 *)
> +					&target->path.dgid.raw[4]),
> +				(int) be16_to_cpu(*(__be16 *)
> +					&target->path.dgid.raw[6]),
> +				(int) be16_to_cpu(*(__be16 *)
> +					&target->path.dgid.raw[8]),
> +				(int) be16_to_cpu(*(__be16 *)
> +					&target->path.dgid.raw[10]),
> +				(int) be16_to_cpu(*(__be16 *)
> +					&target->path.dgid.raw[12]),
> +				(int) be16_to_cpu(*(__be16 *)
> +					&target->path.dgid.raw[14]),

This is pretty horrible - could you use show_dgid() here?

Cheers,
Muli