From: Thiago Rafael Becker <email@example.com> To: "Aurélien Aptel" <firstname.lastname@example.org> Cc: ronnie sahlberg <email@example.com>, linux-cifs <firstname.lastname@example.org>, Steve French <email@example.com>, LKML <firstname.lastname@example.org> Subject: Re: [PATCH] cifs: retry lookup and readdir when EAGAIN is returned. Date: Wed, 16 Jun 2021 12:40:26 -0300 [thread overview] Message-ID: <YMobapLxHv/BPzFP@nyarly.rlyeh.local> (raw) In-Reply-To: <email@example.com> Thanks for the comments, I'm working on them. On Wed, Jun 16, 2021 at 12:07:50PM +0200, Aurélien Aptel wrote: > I guess it looks ok as a quick fix for the issue at hand but: > - should we check for more error codes (use is_retryable_error()?) > - all syscalls will need something similar, the session can be closed at > any point > * either we add a loop everywhere > * or we change cifs_send_receive to retry (most?) calls > * might be worth looking at what nfs does here Some syscall can return EAGAIN, so we would need to check which operation is retryable and if the error is retryable. I don't know if all Linux syscalls mappable to smb2 operations in the wire, but it may be worth mapping. NFS requeues the calls, and fails after a configurable timeout for soft mounts, or issues an error message and requeues the request for hard mounts (retrans and timeo mount options). > - Should this be done for both soft (default) and hard mounts? I guess > for hard we would retry indefinitely Good point, but the correct option is to retry on hard mounts until the operation succeeds. NFS hard mounts create issues like being unable to do a clean shutdown on a server failure, because oustanding I/O blocks it. The NFS has atempted to fix this by adding options to kill all outstanding I/O, but I'm not current on the efforts/issues in this front. So this would create the same issue with outstanding mounts on cifs shares, so a similar solution to NFS may be adapted/created in the future. One thing that I forgot to mention in the patch is that this uncovered a bug in compuond requests, where wait_for_compund_request will block the readdir request with EDEADLK, if the share needs reconnect. I'm looking into this patch, and will submmit it separatelly, as this looks like a corner case uncovered by the patch and specific conditions in the tests. > Cheers, Best regards, Thiago
next prev parent reply other threads:[~2021-06-16 15:47 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-15 16:42 Thiago Rafael Becker 2021-06-16 6:09 ` ronnie sahlberg 2021-06-16 10:07 ` Aurélien Aptel 2021-06-16 15:40 ` Thiago Rafael Becker [this message] 2021-06-16 17:09 ` Steve French
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=YMobapLxHv/BPzFP@nyarly.rlyeh.local \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH] cifs: retry lookup and readdir when EAGAIN is returned.' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).