On Sun, 28 Nov 2010 06:17:54 -0600 Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote: > On Sat, Nov 27, 2010 at 7:12 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Sat, 27 Nov 2010 11:38:26 -0600 > > Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote: > > > >> On Sat, Nov 27, 2010 at 10:35 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > >> > On Sat, 27 Nov 2010 09:08:01 -0600 > >> > Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote: > >> > > >> >> On Sat, Nov 27, 2010 at 7:01 AM, Jeff Layton <jlayton@xxxxxxxxxxxxxxx> wrote: > >> >> > This patchset is intended to fix the unreliable behavior in CIFS in the > >> >> > face of a server that's taking a long time to process requests. > >> >> > > >> >> > In general, the current code sets a timeout for all requests that are > >> >> > sent on the wire. If the server doesn't respond to the request within > >> >> > that timeout, the client performs a reconnect and retries the request. > >> >> > > >> >> > This is dangerous and wasteful behavior for the client. Much of the > >> >> > state of a CIFS mount is bound to the socket connection. Break the > >> >> > socket connection and state is lost. > >> >> > > >> >> > I believe this the root cause of some data corruption issues that have > >> >> > been reported to me. We had a partner report that when they copied a > >> >> > large file to a CIFS server and then compare the result to the original, > >> >> > there is sometimes a mismatch. The problem is highly correlated to > >> >> > messages in the ring buffer that indicate that the client reconnected > >> >> > the socket during the test run. > >> >> > > >> >> > Another problem that I can reliably reproduce -- I have win2k8 > >> >> > installed as a VM guest. When I run connectathon tests to that server, > >> >> > it frequently fails on the test that writes 4GB past the EOF. The > >> >> > storage on this server is slow, and it can take longer than 180s for > >> >> > it to zero-fill the output file. > >> >> > > >> >> > The intent of this patchset is to fundamentally change when the client > >> >> > decides to reconnect the socket. Instead of the old behavior, this > >> >> > patchset makes the client wait indefinitely for a response. Rather than > >> >> > waiting in TASK_UNINTERRUPTIBLE sleep however, the client waits in > >> >> > TASK_KILLABLE sleep so that fatal signals will end the sleep and > >> >> > return -ERESTARTSYS to the caller. > >> >> > > >> >> > In order to determine whether the server is completely dead or just > >> >> > taking a long time to process requests, this patchset has the client > >> >> > do an asynchronous SMB echo request every 60s when the client hasn't > >> >> > gotten a reponse. If the server doesn't respond after 5 mins, the > >> >> > client will attempt to reconnect the socket. > >> >> > > >> >> > With this patchset, I can reliably run the connectathon tests against > >> >> > my slow server. Preliminary results using the proprietary test that > >> >> > was seeing data corruption have also been promising. > >> >> > > >> >> > I'd like to see this set considered for inclusion into 2.6.38. > >> >> > > >> >> > Comments and suggestions welcome. > >> >> > > >> >> > Jeff Layton (14): > >> >> > cifs: move mid result processing into common function > >> >> > cifs: wait indefinitely for responses > >> >> > cifs: don't reconnect server when we don't get a response > >> >> > cifs: clean up handle_mid_response > >> >> > cifs: allow for different handling of received response > >> >> > cifs: don't fail writepages on -EAGAIN errors > >> >> > cifs: handle cancelled requests better > >> >> > cifs: make wait_for_free_request take a TCP_Server_Info pointer > >> >> > cifs: add cifs_call_async > >> >> > cifs: add ability to send an echo request > >> >> > cifs: set up recurring workqueue job to do SMB echo requests > >> >> > cifs: reconnect unresponsive servers > >> >> > cifs: make hard mounts the default > >> >> > cifs: remove code for setting timeouts on requests > >> >> > > >> >> > fs/cifs/cifs_debug.c | 8 +- > >> >> > fs/cifs/cifsglob.h | 19 ++-- > >> >> > fs/cifs/cifspdu.h | 15 +++ > >> >> > fs/cifs/cifsproto.h | 7 + > >> >> > fs/cifs/cifssmb.c | 55 ++++++++- > >> >> > fs/cifs/connect.c | 128 +++++++++++++++---- > >> >> > fs/cifs/file.c | 48 ++------ > >> >> > fs/cifs/sess.c | 2 +- > >> >> > fs/cifs/transport.c | 344 ++++++++++++++++++++++---------------------------- > >> >> > 9 files changed, 355 insertions(+), 271 deletions(-) > >> >> > > >> >> > -- > >> >> > 1.7.3.2 > >> >> > > >> >> > -- > >> >> > 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 > >> >> > > >> >> > >> >> General question, during all this process and wait, does user know > >> >> in any way (ideally without setting any debug flags/options) that a server is > >> >> unresponsive, perhaps everytime an echo does not come back within > >> >> expected time, and then when it is back to normal i.e. responding? > >> > > >> > There's no feedback of that sort in the current patches, but something > >> > like that could be added... > >> > > >> > Note that the behavior here isn't quite analogous to NFS/RPC since we > >> > don't time out requests. So, we can't really base this on a "major > >> > timeout" like we do there. > >> > > >> > What sort of behavior do you think would make the most sense here? How > >> > often and under what conditions should we do a printk? > >> > > >> > -- > >> > Jeff Layton <jlayton@xxxxxxxxxx> > >> > > >> > >> I am not sure. But user has to have some indication if a command does not > >> return in expected time. I think whenever cifs gets ready to send SMB Echo, > >> command would perhaps be a time to log that server is not responding. > >> If that SMB Echo response comes back within expected time, server is back > >> to normal timing and that can be logged. > >> > > > > I don't think so. Just because we're sending an echo doesn't mean that > > we should spam the logs. What exactly is an "expected time" anyway? > > That value is highly variable, dependent on many of different factors > > and often impossible to predict. > > > > OTOH, if the server isn't responding to echoes, then I think we probably > > are justified in doing a printk. CIFS Servers are expected to respond > > to an echo in a timely fashion even if they are busy. > > > >> I am not sure whether what gets logged should be either at debug level > >> or error level, error level is preferrable at it would be visible to > >> the user without > >> setting any debug flags. > >> But some test scenarios may want to run clean and if "server not responding", > >> "server responding" type of messages are logged, it may be flagged or > >> deemed annoying. > >> > >> There are certain commands that can't be "hard mount" kind, they have to be > >> returned within certain time e.g. if we are unmounting a share and server is > >> not responding. > > > > I'm afraid I have to disagree here. "timing out" certain commands is > > what has given us the problems we have today. CIFS currently falls flat > > on its face when a server is slow to respond to certain command s. > > > > I'll submit that the best behavior to aim for is the one I'm proposing. > > The client ought to wait indefinitely for responses from the server, > > and only give up if the server isn't responding to echoes. The wait > > Is it possible that server is so slow that client can't even send any > more commands such as smb echo? This could be a temporary glitch > in the life of a server! > I think that would be indicative of a broken server. We need some metric to know whether the socket is dead and that it needs to be reconnected. If not SMB echoes, then what do you suggest? > I am really concerned about a scenario where for example, smbds > on a Samba server are killed. Would an user be able to unmount a share? > Because there will be no response from the server, reconnects are not > going help, and user on client machine can only kill a command. > What's the exact scenario that you're concerned with here? For the purposes of argument, let's assume that the mount is idle -- no dirty data or open files. We issue a umount on the mountpoint and cifs_umount is eventually called. The tcon is put and a CIFSSMBTDis is issued. The smbds on the server are down however, and the it isn't responding to echoes. The client will send the TDis and wait for the reply, no replies come and we start sending echoes. The echoes won't be responded to however and eventually cifs_reconnect is called. The mid is marked MID_RETRY_NEEDED and the umount process is woken up. SendReceiveNoRsp returns -EAGAIN and CIFSSMBTDis returns 0 and the umount proceeds. So, it may take a little while but eventually the umount would proceed. If you're really concerned about that situation however, you could make CIFSSMBTDis use an async call (support for that is added in this patchset). That would make the umount return even more quickly than it does today. -- Jeff Layton <jlayton@xxxxxxxxxx> -- 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
