Re: 2.4.19rc2aa1 i_size atomic access

[Andrea Arcangeli <andrea@suse.de>]

On Mon, Jul 29, 2002 at 05:34:16PM -0700, Daniel McNeil wrote:
> Andrea,
> 
> Sorry I haven't responded, but I was on vacation all last week and
> was not near a computer.

No problem :)

> I like your code change.  Incrementing the v2 before the v1 in the
> i_size_write() is much better.  My code was definitely uglier -- but
> it was correct since the version1 and version2 where sampled before
> i_size was read and version1 and version2 where checked again after.
> It was excessive, but correct.

ok, so you had the dependency v1 == v2, so you were also implicitly
comparing v2 with the new version 1 ok.

> 
> On your patch, shouldn't non-smp preempt still use the 64-bit stuff?
> The comment says it should, but the #ifdef's are not checking for
> PREEMPT or did I miss something?

there's no preempt in 2.4, the comment was meant for anybody foward
porting it to 2.5.

> I would still be curious about the performance difference between the 
> version approach and the cmpxchg8 approach.  With SMP I'm a bit worried
> about the cacheline bouncing around and the memory bandwith wasted.

Randy didn't report any decrease in performance, so in normal loads 
shouldn't be noticeable.

> Any ideas on what kind of test would be appropriate?
> I've got access to 2-proc to 8-proc systems I could run some tests on,
> just not sure what test would be useful.  The fstat() test isn't
> realistic.

I would say dbench is a good candidate for this kind of change to verify
it's not noticeable.

then you could test two parallel reads on the same inode, for example
two parallel dd if=file of=/dev/null reading from cache, and see if
there's a difference of bandwidth with cmpxchg8b and ordered
read/writes (on a 4p you could try with 4 parallel dd).

> Increasing the versions to 32-bit is ok with -- I was just trying to
> not waste too much space.

ok, as said the int granularity is going to be atomic for all archs.

Andrea