On Tue, Mar 22, 2011 at 12:03 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > On Tue, 22 Mar 2011 11:46:54 -0500 > Steve French <smfrench@xxxxxxxxx> wrote: > >> On Tue, Mar 22, 2011 at 11:42 AM, Steve French <smfrench@xxxxxxxxx> wrote: >> > Isn't there a lock across lseek? >> > >> >>> but how could we have PageDirty? We just did filemap_write_and_wait >> >>> (unless some private mmap case where it may be ok?). >> >>> >> >> >> >> Nothing prevents the page from being redirtied after writeback. >> >> If it is redirtied, we immediately do a filemap_fdatawrite anyway (in >> the write/aio_write path) so there isn't a delay in writing data to >> the server. >> > > The bottom line is that you're asking to revalidate the file, finding > that it has changed and are trying to invalidate the pagecache. Some of > those pages are may not be invalidated though, so you won't end up with > new data from the server if you go to do a read. > > The whole point of this patch is to fix data-integrity issues, right? > So why only do a half-assed job of that? The data gets written (immediately). The page gets freed either immediately or (in the unlikely event it was busy) the next time we do a seek or revalidate or any of the other places that checks the invalidate flag. Not sure how there is a data integrity issue. If we do return EBUSY - it makes no sense on seek (we got the updated file size) - the only case it might make sense on is strictcache read which was racing with an mmapped write and in that case the data the client has is up to date and the next read will toss the page out of cache since the mapping is marked invalid. Seems a lot of complexity to return EBUSY to strictcache read since there isn't anything it can do about it other than spin and retry and we would retry anyway on the next operation on that file. -- Thanks, Steve -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html