Re: [PATCH] 2.5.21 Nonlinear CPU support

[H. Peter Anvin]

Anton Altaparmakov wrote:
> 
> The code would be run outside the critical region... But correct about
> the race. I thought that was obvious and wasn't suggesting the above to be
> the actual code... That was supposed to be obvious from lack of error
> handling etc... Never mind. My mistake, I should have been more precise
> the first time round, here is the actual code I had in mind:
> 
> [snip]
> 	if (unlikely(!ntfs_compression_buffers)) {
> 		int err;
> 
> 		/*
> 		 * This code path only ever triggers once so we take it
> 		 * out of line.
> 		 */
> 		if ((err = try_to_allocate_compression_buffers())) {
> 			// TODO: do appropriate cleanups
> 			return err;
> 		}
> 	}
> 	disable_preempt();
> 	cb = ntfs_compression_buffers[smp_processor_id()];
> [snip]
> 
> and try_to_allocate_compression_buffers would be:
> 
> int try_to_allocate_compression_buffers(void)
> {
> 	int err = 0;
> 
> 	down(&ntfs_lock);
> 	if (likely(!ntfs_compression_buffers))
> 		err = allocate_compression_buffers();
> 	up(&ntfs_lock);
> 	return err;
> }
> 
> and allocate_compression_buffers() is the same as it is now. Actually I
> was going to fuse try_to_allocate and allocate into one function but as I
> am showing above it is clearer to see what I had in mind...
> 
> Happy now? This basically just defers the allocation to a bit later. As it
> is at the moment the allocation happens at mount time of a partition which
> supports compression. Note that the code in super.c would still need to
> exist due to reference counting so we know when we can free the buffers
> again. The only thing changed in super.c will be to remove the actual call
> to allocate_compression_buffers, all else stays in place. Otherwise we
> have no way to tell when we can throw away the buffers.
> 

I presume allocate_compression_buffers() allocates *all* buffers, and 
doesn't return error if there is nothing to allocate?  If so, the above 
code should be OK.

If allocate_compression_buffers() either doesn't check if it has already 
allocated, or returns an error if buffers were already allocated, then 
the above code is OK *EXCEPT IN THE CASE OF HOTSWAP CPUs*.

My originally proposed code allocated one buffer at a time, and should 
be correct even in the presence of hotswap CPUs.

	-hpa