[linux-cifs-client] Re: [PATCH 4/4] cifs: flush data on any setattr

[Linus Torvalds]

On Thu, 26 Mar 2009, Jeff Layton wrote:
> On Thu, 26 Mar 2009 15:10:49 -0500
> Steve French <smfrench at gmail.com> wrote:
> 
> > On Thu, Mar 26, 2009 at 12:35 PM, Jeff Layton <jlayton at redhat.com> wrote:
> > > We already flush all the dirty pages for an inode before doing
> > > ATTR_SIZE and ATTR_MTIME changes. There's another problem though -- if
> > > we change the mode so that the file becomes read-only then we may not
> > > be able to write data to it after a reconnect.
> > 
> > Do we have any idea of how badly this would hurt performance?
> > 
> > Changing the mode or owner may be rare enough ... but I don't have any
> > data to prove that.
> > 
> > Changing the file's mode on the fly while writing could cause problems
> > for file systems other than cifs too, not sure we want to hurt
> > performance for a corner case of a bad application.  I don't this
> > posix specifies that chmod does an implied fsync
> > 
> 
> (cc'ing Linus since he brought this to our attention and might have
> input)
> 
> No, posix doesn't specify that chmod does an implied fsync, but if
> that's what's required to ensure that we don't break writeback across a
> reconnect then I think it's the right thing to do. We can try to
> optimize it away on some setattr cases, but they're probably not worth
> it.

Yes, I don't care about anything but obvious correctness.

Right now, CIFS is _broken_. Real programs have seen real breakage. I 
don't see why Steve argues against the patch on some specious grounds of 
it "not being necessary", when it clearly is, and there clearly is a CIFS 
bug there.

Performance is totally irrelevant. It doesn't matter at all, when the 
alternative is "lost data".

			Linus