Re: [PATCH] bulkstat fixups

[David Chinner <dgc@sgi.com>]

On Wed, Nov 21, 2007 at 03:17:47PM +0000, Christoph Hellwig wrote:
> +#define XFS_BULKSTAT_UBLEFT(ubleft)	((ubleft) >= statstruct_size)
> +
> 
> 	I don't think this macro is really helpful.  An inline would
> 	have been useful if statstruct_size was constant, but this
> 	way it's much better to just write out the comparism the four
> 	times it's used.
> 
> +	if (!ubcountp || *ubcountp <= 0) {
> +		return EINVAL;
> +	}
> 
> 	No need for the braces here.
> 
> 
> I also must say I don't like the cond_resched() calls very much.  They
> look entirely random to me.  We really should only need cond_resched
> when it's absolutely needed, and it deserves a comment why it's needed
> then.

I think I mentioned that we don't need them in the innermost loop.
The reason for adding them is that if the inode clusters are in cache,
bulkstat will not yield the cpu at all and so holds off other
things from operating on that CPU. And when bulkstat has got itself
stuck in a loop, if it's running on the same CPU as I/O completion
events are running on (i.e. disk interrupts delivered to) it basically
hangs the filesystem. If that CPU is taking interrupts for your root
filesystem....

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group