ср, 13 янв. 2021 г. в 09:03, Steve French <smfrench@xxxxxxxxx>: > > On Wed, Jan 13, 2021 at 10:51 AM Paulo Alcantara <pc@xxxxxx> wrote: > > > > Pavel Shilovsky <piastryyy@xxxxxxxxx> writes: > > > > > Thanks for reporting the issue. > > > > > > The problem is with the recent fix which changes the error code from > > > -EINTR to -ERESTARTSYS: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/cifs/transport.c?id=6988a619f5b79e4efadea6e19dcfe75fbcd350b5 > > > > > > and this problem happens here: > > > > > > https://git.samba.org/sfrench/?p=sfrench/cifs-2.6.git;a=blob;f=fs/cifs/smb2pdu.c;h=067eb44c7baa863c1e7ccd2c2f599be0b067f320;hb=236237ab6de1cde004b0ab3e348fc530334270d5#l3251 > > > > > > So, interrupted close commands don't get restarted by the client and > > > the client leaks open handles on the server. The offending patch was > > > tagged stable, so the fix seems quite urgent. The fix itself should be > > > simple: replace -EINTR with -ERESTARTSYS in the IF condition or even > > > amend it with "||". > > > > Yes, makes sense. > > > > Maybe we should do something like below > > > > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > > index 067eb44c7baa..794fc3b68b4f 100644 > > --- a/fs/cifs/smb2pdu.c > > +++ b/fs/cifs/smb2pdu.c > > @@ -3248,7 +3248,7 @@ __SMB2_close(const unsigned int xid, struct cifs_tcon *tcon, > > free_rsp_buf(resp_buftype, rsp); > > > > /* retry close in a worker thread if this one is interrupted */ > > - if (rc == -EINTR) { > > + if (is_interrupt_error(rc)) { > > int tmp_rc; > > > > tmp_rc = smb2_handle_cancelled_close(tcon, persistent_fid, > > Seems reasonable, the other two conditions (ERESTARTRNOHAND e.g.) are > not explicitly set by cifs.ko but may come back from other libraries > so could be worth checking for, and it seems a little safer to use > is_interrupt_error. > > Paulo, > If you can do a patch quickly I will run the buildbot on it and the > other two patches currently in for-next and try to send in the next > couple days. > > (I do have a fourth patch, not currently in for-next, that I am > debugging ... the '\' handling patch ... which I can send if we can > figure out what is missing in it). I may also include the two trivial > one line style patches recently submitted to list. > > -- > Thanks, > > Steve Paulo, Yes, is_interrupt_error() is cleaner here to eliminate other potential issues. Thanks! Steve, Agree - we should send this ASAP with other non-controversial patches. -- Best regards, Pavel Shilovsky
