On Tue, 28 Feb 2012 15:18:02 -0600 Steve French <smfrench@xxxxxxxxx> wrote: > On Tue, Feb 28, 2012 at 2:53 PM, Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote: > > 2012/2/29 Steve French <smfrench@xxxxxxxxx>: > >> Attempt to summarize after discussions with jra and Chris: > >> > >> - don't make default case to Samba worse performance than it already is > >> - don't "fix" by adding restrictions on echo retries and blocking > >> locks when it is not broken (windows allows these, and counts blocking > >> locks against the pid not against the session). For servers which > >> allow few mpx, perhaps those for 10 or fewer - if you think they are > >> buggy, I don't mind treating echo and blocking locks differently than > >> we do now - but echo and blocking locks don't need to be restricted > >> the way Pavel suggested to windows and samba and any normal server > >> (and there is some risk in restricting them that way). We could > >> probably limit the number of blocking locks (out of resource error, > >> returning ENOLOCK) when there are 50 (e.g.) on a process (or 10 if the > >> server supports maxmpx of 10, and zero if maxmpx is less than 3 or > >> less than 10). > >> - for other smb requests (the ones we enforce today) enforce the limit > >> the server returns (rather than 50 always) > >> > > > > But for SMB2 we need to count both echos and blocking locks. It even > > more compicated because the number of credits is dynamic: we need to > > increment/decrement the maximum number of parallel blocking locks to > > make sure we don't exceed available credits. > > Yes - for smb2 the number of available requests (credits) can change > on ANY smb2 response. It is not related to how cifs requests are > processed. > > Also note that for cifs for Windows, apparently the smb echo is > handled at a lower level (before checking mpx) - (and also Samba > server does not enforce either). > (cc'ing Chris and Jeremy to make sure I understand) The description above is a good start but doesn't quite outline the clear concise "rules" for this that I was looking for. So if I understand what Steve wrote above, he's basically saying that we should enforce the maxmpx for everything but blocking locks and echoes? Is that correct? If so then I think that's wrong and won't fix any of the problems that people have reported with the existing code. It's all well and good that *some* servers allow you to exceed the maxmpx in *some* cases, but we can't code to that assumption. We know well that many servers do not handle exceeding the maxmpx gracefully at all. As always we have to code to the lowest common denominator by default, and then optionally allow people to exceed that if they choose to do so. I think this is a case where we need a good description in a human language (preferably english) of how this should work (aka a specification), and then write code to match that description (aka code to the spec). Anything else is going to send us down the rabbit hole where we are today with cifs.ko -- a bunch of ad-hoc, broken code that no one really understands. While we may not like it, a hard cap on the number of outstanding requests is required by the protocol docs that Chris helped write. Allow me to quote from MS-CIFS: MaxMpxCount (2 bytes): The maximum number of outstanding SMB operations that the server supports. This value includes existing OpLocks, the NT_TRANSACT_NOTIFY_CHANGE subcommand, and any other commands that are pending on the server. There's nothing in there that says anything about certain commands being excepted from that value. -- Jeff Layton <jlayton@xxxxxxxxx> -- 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
