[Cbe-oss-dev] [PATCH 4/6] spufs: extension of spu_create to support affinity definition

[Jeremy Kerr]

Andre,

> +		if (!list_empty(&neighbor->aff_list) &&
> +		    !(neighbor->aff_flags & AFF_SUBLIST_HEAD) &&
> +		    !list_entry(neighbor->aff_list.next, struct spu_context,
> +		    aff_list)->aff_flags & AFF_SUBLIST_HEAD)
> +			return ERR_PTR(-EEXIST);

Just trying to follow some of the logic here - why is it OK for 
neighbour->aff_list.next to be the AFF_SUBLIST_HEAD? In fact, what 
exactly does the AFF_SUBLIST_HEAD flag signify?


> +		list_for_each_entry(tmp, &gang->aff_list_head, aff_list)
> +			count++;
> +		if (list_empty(&neighbor->aff_list))
> +			count++;
> +
> +		for (node = 0; node < MAX_NUMNODES; node++) {
> +			if ((be_spu_info[node].n_spus - atomic_read(
> +				&be_spu_info[node].isolated_spus)) > count)
> +				break;
> +		}

This one is tricky, as isolated contexts can come and go at any time. 
Maybe checking only n_spus here would be better, but would create 
scheduling problems later, if the isolated context was long-lived.

However, we'll have the same problem if a new isolated context starts on 
one of the SPEs in the node that this finds.

> +	if (ret) {
> +		if (flags & (SPU_CREATE_AFFINITY_MEM | SPU_CREATE_AFFINITY_SPU))
> +			mutex_unlock(&gang->aff_mutex);
>  		goto out_unlock;
> +	}
> +
> +	if (flags & (SPU_CREATE_AFFINITY_MEM | SPU_CREATE_AFFINITY_SPU)) {
> +		spufs_set_affinity(flags, SPUFS_I(dentry->d_inode)->i_ctx,
> +					neighbor);
> +		mutex_unlock(&gang->aff_mutex);
> +	}

how about:

	if (flags & (SPU_CREATE_AFFINITY_MEM | SPU_CREATE_AFFINITY_SPU)) {
		if (!ret)
			spufs_set_affinity(flags,
				SPUFS_I(dentry->d_inode)->i_ctx, neighbor);
		mutex_unlock(&gang->aff_mutex);
	}

	if (ret)
		goto out_unlock;

?

Regards,


Jeremy