[linux-cifs-client] [PATCH 4/5] cifs: take reference to inode for oplock breaks

[Steve French]

On Fri, Aug 21, 2009 at 2:01 PM, Steve French <smfrench at gmail.com> wrote:

>
>
> On Fri, Aug 21, 2009 at 10:48 AM, Jeff Layton <jlayton at redhat.com> wrote:
>
>>
>> As long as you can do so in a non-racy way, then I'm not opposed to
>> that long-term. The problem though is that I don't have a lot of
>> confidence in the open file tracking code. It's extremely hard to
>> follow and definitely has races. I don't think it's really possible to
>> do what you suggest safely until the open file tracking code has been
>> fixed.
>>
>> For now, I'm pretty sure this set should fix the problems that users
>> are hitting in the oplock codepath today. I'd like to fix that first
>> before we embark on a redesign of it.
>>
>
> You may be right that this should be two stages, but your 2nd and 3rd patch
> are already large, so I doubt that it would grow (and might make it easier
> to follow and more logical).
>

There is one other thing to consider (which might lean toward your inode
based approach rather than what I suggested about tie of the
oplock to the file struct for the fid) ... we need to move to 1st attempting
"batch oplocks" (as was discussed in June) to allow safer caching
across close (and would also helps with multiple opens from the same
client) - this will necessitate making the routine which frees inodes
aware of oplocks.   In the short run I was planning on trying this approach
(grab one batch oplock on open, and only rely on the other type of oplocks
when we can't get or lose the batch oplock) with the smb2 code and
after a few months if it works ok consider fixing the cifs code.  In the
meantime if we uses patches 2 through 4 as is (one and five are
obviously fine), it would be great if we could get at least one more ack.
I haven't found any problems yet but they are big.


-- 
Thanks,

Steve
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.samba.org/pipermail/linux-cifs-client/attachments/20090821/d2e94517/attachment.html>