Re: [patch 13/13] lseek speedup

[Anton Altaparmakov]

At 06:31 17/07/02, Andrew Morton wrote:
>This is a fairly dopey patch to fix up the i_sem contention in lseek.
>Better ideas are welcome, but I'm offline until Monday so don't think
>I'm ignoring them...

I am afraid I don't have any better ideas but I don't think your patch is 
safe. )-:

>We need to decide what we really want to lock in there.  If multiple
>threads are updating f_pos and/or i_size at the same time then they are
>going to see strange results whatever the kernel does.  I'd maintain
>that the only real obligation which the kernel has in this situation is
>to not oops, to not munch data and to not return competely outlandish
>results.

That can only be guaranteed by holding i_sem. Since the BKL was taken out 
of ->readdir(), i_sem is now used for exclusion between ->readdir() and 
->llseek() (well NTFS uses it anyway but many other fs which switched to 
generic_file_llseek() do the same). This exclusion is necessary IMHO.

->readdir() may well blow up or at the very least give "completely 
outlandish results" if someone modifies f_pos while it is running. 
Depending on when exactly it happens, ntfs_readdir() is going to be ok as 
it mostly sets f_pos instead of incrementing it but still, a succeeding 
->llseek(), may actually end up in a different position to the one 
requested because of a concurrent ->readdir().

I know doing the two things at the same time requires a completely crazy 
multithreaded application, but then again many application developers fall 
into that category. (-;

>And the only way we can return outlandish results is on 32-bit SMP if
>one CPU reads i_size or f_pos while another CPU is in the middle of
>modifying it.

I believe it can also happen if ->llseek() is allowed to run concurrently 
with a ->readdir()...

Best regards,

         Anton