Re: [PATCH][RFC] truncate vs add_to_page_cache race

[Nick Piggin <nickpiggin@yahoo.com.au>]

Nick Piggin wrote:

> 
> This following patch should be right.
> 
> It causes the zeros to not get copied back unless i_size
> gets extended again.
> 

OK, nobody has looked at this yet, so I'll just summarise my
ramblings.

The truncate vs add_to_page_cache race which I first misidentified
to be the problem is actually harmless and there by design as
Andrew points out. My first patch did "fix" it AFAIKS, but doubtful
if it is worth the locking and complexity.

The real problem is that readpage doesn't use the same i_size as
do_generic_mapping_read (ie. it could be changed by another racing
operation). This race window is quite large at the moment, containing
cond_resched and a page allocation.

Is this transient returning of zero fill actually a problem? Transient,
ie. a second read could see the correct i_size and not return the zeros.

My second patch gets closer to the heart of the problem: I think it
fixes all truncate races. However a write(2) could still expand the
i_size between ->readpage and calculation of nr, thus causing zero fill
to appear transiently again (one would either want to return the
written data, or a short read, not zeros).

I think the entire problem can be fixed by ensuring ->readpage and
do_generic_mapping read see the same i_size. This would either mean
passing i_size to or from ->readpage, *or* having ->readpage return
the number of bytes read, for example.