On Mon, Jul 24, 2000 at 12:04:39PM -0400, Benjamin C.R. LaHaise wrote:
> On Mon, 24 Jul 2000, Alexander Viro wrote:
> 
> > Not needed. That race is taken care of - readpage() never does access past
> > i_size and has the page locked, while vmtruncate() starts with setting
> > i_size (on shrinking truncate(), that is), then goes through all pages in
> > affected area and grabs the lock on all of them (one by one, not
> > simultaneously, indeed) and only then calls ->truncate().
> 
> That assumes that i_size is atomically updated, which currently is not the
> case on 32 bit platforms.
> 
>               -ben

I found that i_size is in most cases reasonably protected by i_sem.
Except for Coda where inode->i_size != inode->i_mapping->host->i_size,
because we `borrow' the mapping of the underlying container file.

At the moment I believe that the introduction of i_mapping->host might
have been a mistake. The readpage/writepage operations use
i_mapping->host->i_size, but sys_stat and generic_write_file (for
a file with O_APPEND) are using inode->i_size. So for Coda we are
copying the i_size back to the Coda inode in a wrapper around
generic_file_write.

AFAIK Coda is the only FS that has an address space mapping that
associated with another inode, and we need to lock the i_sem of that
inode to avoid problems. Maybe the Coda file write should become
something like:

    coda_file_write()
    {
        down(&container_inode->i_sem);
        n = generic_file_write(file, ...);
        container_inode->i_size = file->f_dentry->d_inode->i_size;
        up(&container_inode->i_sem);
        return n;
    }

That way all writes are protected by the semaphore on the container
file's inode. And the use of i_mapping->host, along with the nasty cast
to (struct inode *) could be removed.

Jan

Reply via email to