[linux-cifs-client] Re: [PATCH] cifs: fix O_APPEND on directio mounts

[Jeff Layton]

On Wed, 27 Aug 2008 21:24:29 -0500
"Steve French" <smfrench at gmail.com> wrote:

> On Wed, Aug 27, 2008 at 7:24 PM, Jeff Layton <jlayton at redhat.com> wrote:
> > The direct I/O write codepath for CIFS is done through
> > cifs_user_write(). That function does not currently call
> > generic_write_checks() so the file position isn't being properly set
> > when the file is opened with O_APPEND.  It's also not doing the other
> > "normal" checks that should be done for a write call.
> >
> > The problem is currently that when you open a file with O_APPEND on a
> > mount with the directio mount option, the file position is set to the
> > beginning of the file. This makes any subsequent writes clobber the data
> > in the file starting at the beginning.
> 
> Is this important enough to go into stable after it goes to mainline
> (seems like a good candidate)?
> 

Possibly, though I'll couch this with the statement that this has seen
only very light testing (basically testing that it fixed the reported
problem). I've not regression tested it much at all. Still, it seems
unlikely to make anything worse and should only affect directio mounts.

> > This seems to fix the problem in cursory testing. It is, however
> > important to note that NFS disallows the combination of
> > (O_DIRECT|O_APPEND). If my understanding is correct, the concern is
> > races with multiple clients appending to a file clobbering each others'
> > data. Since the write model for CIFS and NFS is pretty similar in this
> > regard,
> At least for oplocked files (which NFSv3 does not have support for) we
> could allow append.   (when appending on an O_DIRECT open or mount,
> also might be possible to open with DENY_WRITE mode rather than
> failing if races with other clients must be prevented but that could
> break apps).
> 

Interesting. I hadn't considered oplocks or share/deny modes. We
probably could use those to our advantage here. I want to do a bit more
research into why NFS specifically denies this, and then maybe we can
see what can be done to mitigate it.

It's probably also time to consider properly handling O_DIRECT. I don't
think that will be too tough, but we'll probably have to consider the
best way to test it.
-- 
Jeff Layton <jlayton at redhat.com>