Re: [Ext2-devel] [PATCH 2/2] ext2/3: Support2^32-1blocks(e2fsprogs)

[Ravikiran G Thirumalai <kiran@scalex86.org>]

On Wed, Mar 29, 2006 at 10:35:10AM -0800, Mingming Cao wrote:
> On Wed, 2006-03-29 at 11:13 +0200, Laurent Vivier wrote:
> > 
> > You're right, Mingming.
> > 
> > But I think instead of thinking to change "long" by "long long" we
> > should think about changing "long" by "unsigned long" in the per-cpu
> > counter structure.
> > 
> > Is there someone knowing why this counter is signed ?
> 
> I am wondering the same thing asked by Laurent. Initially I thought the
> signed value is there to prevent overflow, or to maintain a "int" type
> counters. Are those the intentions, kiran?

I don't know if the local counter version values can be unsigned in this
case.  Consider a case like this with the initial counter value to be 0,
and FBC_BATCH is 32 (8cpusx4)

cpu 1				cpu 2			cpu 3
--------			-------			--------
add(10)
//local = 10 fbc = 0.
				sub(5)
				//local = -5 fbc = 0
							add(31)
							//local = 31 fbc = 0

				sub(30)
				//local = 0 fbc = -35
				--------------->(A)

Now if the local counters were unsigned, and the global counters unsigned
too, counter read at A would result in a large value, which would mislead
the app.  Maybe it doesn't matter if we use percpu_counter_exceeds at
critical places, so these get caught, but that would mean going on all cpus
more often than before..and that would also mean weird values when we just
use percpu_counter_read to print these counters.

So maybe using long long is a simpler solution here? Andrew, thoughts?

> 
> But it seems the per cpu counters used in ext2/3 are all number of free
> blocks/inodes/directories.  So it should be always positive values.  It
> seems fine to change the percpu counters to type "unsigned long" for
> ext2/3 itself. But I am not sure if this will cause issues for other
> users of percpu counters.  Kiran, could you please confirm this?

I guess most of the uses for per-cpu counters will be up counters, we don't
need the signedness if it wasn't for the issues above.  The nr_files,
memory_allocated counters are up counters too.

Thanks,
Kiran