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

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

Andrew Morton wrote:
> Nick Piggin <nickpiggin@yahoo.com.au> wrote:
> 
>>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.
> 
> 
> Or not check i_size in ->readpage at all?
> 

It needn't check readpage if it gets the number of bytes to read
passed to it, or gets i_size passed to it.

With do_generic_mapping_read and ->readpage each having a different
idea of how much of the page to process(*), bad things can happen.
They have different ideas about how much they need to process due to
the each one checking i_size on its own.

* That is "copy to userspace" and "read" for do_generic_mapping_read
   and ->readpages respectively.

> If fixing this is going to cost extra fastpath cycles I'd be inclined to
> not bother, frankly.
> 

What I'm thinking of shouldn't cost any cycles, it would require a
change to ->readpage API though. Preferably one where we can tell it
how many bytes to read. I can't see how else to fix it.

If this is not acceptable for 2.6, we could use a nicer variation of
my second patch which at least fixes the truncate problem, and its
remaining race is *much* more improbable.