Re: [patch 34/56] openmosix/openmosix-alloc_fd_bitmap-spinlock-protection.patch

Florian Delizy <[email protected]> Mon, 13 Nov 2006 16:07:07 +0100
Newsgroups gmane.linux.cluster.openmosix.devel
Message-ID <[email protected]>
Vincent Hanquez wrote:
> On Thu, Nov 02, 2006 at 10:56:51PM +0100, Florian Delizy wrote:
>   
>
>   
>> Index: linux/hpc/kcom.c
>> ===================================================================
>> --- linux.orig/hpc/kcom.c	2006-11-02 22:51:58.000000000 +0100
>> +++ linux/hpc/kcom.c	2006-11-02 22:52:23.000000000 +0100
>> @@ -143,10 +143,10 @@
>>  		return 0;
>>  
>>  	n = max(fd,max(fd4, fd6));
>> -	// spin_lock(&kcom_nodes_lock);
>> -	list_for_each_entry(node, &kcom_nodes, list)
>> -		n = max(node->fd, n);
>> -	// spin_unlock(&kcom_nodes_lock);
>> +
>> +	read_lock(&kcom_nodes_lock);
>> +	list_for_each_entry(node, &kcom_nodes, list) n = max(node->fd, n);
>> +	read_unlock(&kcom_nodes_lock);
>>     
>
> NACK, don't change the middle line.
>   

Well, this does not *only* change the middle line, please, when it's all 
about one line being indented, I'd like you just indent it (I already 
took the work of integrating kcomd, you can take the one of indenting a 
single line ...)

And moreover, be more precise when you add comment about the code ?? I 
suspect that you don't like the 'n=max...' being up one line, (so the 
reason of the last comment), but I am not sure that much, are you 
disagreeing the use of read_lock/read_unlock to protect the linked list 
parsing ?



-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642