Re: [PATCH 5/5] cifs: cancel oplock release callbacks during reconnect

On Fri, 28 Aug 2009 14:14:43 -0500
Shirish Pargaonkar <[email protected]> wrote:

> On Tue, Aug 18, 2009 at 1:07 PM, Jeff Layton<[email protected]> wrote:
> > cifs_oplock_thread has a check for pTcon->needs_reconnect and will skip
> > the CIFSSMBLock call if it's set.
> >
> > Problem: what if the tcon has this set and then gets reconnected before
> > the call goes out on the wire? The oplock release isn't needed and could
> > be a bad thing at that point if the filehandle was reclaimed.
> >
> > Cancel oplock release calls during a reconnect event.
> >
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> >  fs/cifs/cifsfs.c   |    2 +-
> >  fs/cifs/cifsglob.h |    1 +
> >  fs/cifs/connect.c  |    9 +++++++++
> >  fs/cifs/misc.c     |    1 +
> >  4 files changed, 12 insertions(+), 1 deletions(-)
> >
> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> > index 647c5bc..92e06c1 100644
> > --- a/fs/cifs/cifsfs.c
> > +++ b/fs/cifs/cifsfs.c
> > @@ -1024,7 +1024,7 @@ static int cifs_oplock_thread(void *dummyarg)
> >                         * to server still is disconnected since oplock
> >                         * already released by the server in that case
> >                         */
> > -                       if (!tcon->need_reconnect) {
> > +                       if (!oplock->cancel) {
> >                                rc = CIFSSMBLock(0, tcon, oplock->netfid,
> >                                                0 /* len */ , 0 /* offset */, 0,
> >                                                0, LOCKING_ANDX_OPLOCK_RELEASE,
> > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> > index 363dbcf..676c107 100644
> > --- a/fs/cifs/cifsglob.h
> > +++ b/fs/cifs/cifsglob.h
> > @@ -448,6 +448,7 @@ struct oplock_q_entry {
> >        struct inode *pinode;
> >        struct cifsTconInfo *tcon;
> >        __u16 netfid;
> > +       bool cancel;
> >  };
> >
> >  /* for pending dnotify requests */
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index f49304d..b7b67e9 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -121,6 +121,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
> >        struct cifsSesInfo *ses;
> >        struct cifsTconInfo *tcon;
> >        struct mid_q_entry *mid_entry;
> > +       struct oplock_q_entry *oplock;
> >
> >        spin_lock(&GlobalMid_Lock);
> >        if (server->tcpStatus == CifsExiting) {
> > @@ -137,6 +138,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
> >
> >        /* before reconnecting the tcp session, mark the smb session (uid)
> >                and the tid bad so they are not used until reconnected */
> > +       spin_lock(&cifs_oplock_lock);
> >        read_lock(&cifs_tcp_ses_lock);
> >        list_for_each(tmp, &server->smb_ses_list) {
> >                ses = list_entry(tmp, struct cifsSesInfo, smb_ses_list);
> > @@ -145,9 +147,16 @@ cifs_reconnect(struct TCP_Server_Info *server)
> >                list_for_each(tmp2, &ses->tcon_list) {
> >                        tcon = list_entry(tmp2, struct cifsTconInfo, tcon_list);
> >                        tcon->need_reconnect = true;
> > +                       list_for_each_entry(oplock, &cifs_oplock_list, qhead) {
> > +                               if (oplock->tcon == tcon)
> > +                                       oplock->cancel = true;
> > +                       }
> >                }
> > +
> >        }
> >        read_unlock(&cifs_tcp_ses_lock);
> > +       spin_unlock(&cifs_oplock_lock);
> > +
> >        /* do not want to be sending data on a socket we are freeing */
> >        mutex_lock(&server->srv_mutex);
> >        if (server->ssocket) {
> > diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> > index 3bf3a52..4a2d297 100644
> > --- a/fs/cifs/misc.c
> > +++ b/fs/cifs/misc.c
> > @@ -612,6 +612,7 @@ is_valid_oplock_break(struct smb_hdr *buf, struct TCP_Server_Info *srv,
> >                                oplock->tcon = tcon;
> >                                oplock->pinode = inode;
> >                                oplock->netfid = netfile->netfid;
> > +                               oplock->cancel = false;
> >                                spin_lock(&cifs_oplock_lock);
> >                                list_add_tail(&oplock->qhead,
> >                                              &cifs_oplock_list);
> > --
> > 1.6.0.6
> >
> > _______________________________________________
> > linux-cifs-client mailing list
> > [email protected]
> > https://lists.samba.org/mailman/listinfo/linux-cifs-client
> >
> 
> 
> Acked-by: Shirish Pargaonkar <[email protected]>
> 
> with a question, does what you say about pTcon->needs_reconnect  apply
> to oplock->cancel also?

I don't think so. oplock->cancel doesn't get cleared on a reconnect.

...or did I misunderstand the question?

Thanks,
-- 
Jeff Layton <[email protected]>