2005-04-06 02:43:20

by NeilBrown

[permalink] [raw]
Subject: Re: Re: corruption over NFS with 2.6 client, locking, truncating and appending...

On Tuesday April 5, [email protected] wrote:
>
> OK. Attached is a patch that appears to fix things up for me (at least
> for the case of fcntl() locking). It should apply on top of the latest
> -mm kernel with the SEEKDIR_END patch that I sent you last night.

Thanks. It seems to be doing the right thing for me, both with fcntl
locking and with .lock file locking.

>
> Basically, it says that the client must synchronize to the server-given
> value of fattr->size _unless_ we still have pending writes (in which
> case we only accept size values > inode->i_size) or if the getattr() RPC
> call raced with an attribute-changing RPC call (a cached write would
> indeed count as an "attribute-changing RPC call").
>
> Note that this differs greatly from the current behaviour where we only
> accept updates to inode->i_size if the file is not open for writing and
> we didn't race.
>

I assume it is adequately safe....
It protects against concurrent writes changing the size, but not
concurrent setattrs. .... But as the setattr updates i_size *after*
the NFS request completes, it should be safe, yes?

Would this be a candidate for 2.6.11.7 ??

Thanks again,

NeilBrown


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2005-04-06 03:40:42

by Trond Myklebust

[permalink] [raw]
Subject: Re: Re: corruption over NFS with 2.6 client, locking, truncating and appending...

on den 06.04.2005 Klokka 12:43 (+1000) skreiv Neil Brown:

> I assume it is adequately safe....
> It protects against concurrent writes changing the size, but not
> concurrent setattrs. .... But as the setattr updates i_size *after*
> the NFS request completes, it should be safe, yes?

So, setattrs that change the file size are supposed to take the
inode->i_sem semaphore first, something which should suffice to prevent
anyone from calling __generic_file_aio_write_nolock(), and hence
dirtying new pages while we're doing our thing.
In addition, nfs_setattr() ensures that it writes out any pre-existing
dirty pages and cached writes before it actually launches the setattr
RPC call.

I believe the case of mmap() is also pretty safe, since it calls
deny_write_access(), which again should prevent the calls to
get_write_access() in sys_truncate()/sys_ftruncate() from succeeding.

Cheers,
Trond

--
Trond Myklebust <[email protected]>



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs