[linux-cifs-client] Re: [PATCH] cifs: remove dnotify thread code

[Jeff Layton]

On Thu, 8 Jan 2009 08:23:49 -0600
"Steve French" <smfrench at gmail.com> wrote:

> We really need to revisit the dnotify/inotify code ... the SMB spec
> lists what the protocol allows, but AFAIK no one has had a chance to
> prototype this.   It is among the highest priority features still
> needed to implement in cifs.
> 
> As we have talked about before, dnotify (or inotify) is even more
> useful for network file systems than local file systems (since the
> alternative, polling, is too expensive to do over the network).   CIFS
> and SMB2 protocols, unlike NFS, has a mechanism to handle dnotify, but
> mapping it to the VFS has not been investigated sufficiently
> 

I'm all for proper dnotify/inotify interfaces, but until we get that
code in place I don't see any need to keep this kthread around. It
doesn't serve any useful purpose currently -- it's just waking up
tasks that don't need to be woken up.

We may very well need a separate kthread for the notification code
eventually, but I think we should just plan to add it back when the
need for it is clear.

> On Thu, Jan 8, 2009 at 8:15 AM, Jeff Layton <jlayton at redhat.com> wrote:
> > Al Viro recently removed the dir_notify code from the kernel along with
> > the CIFS code that used it. We can also get rid of the dnotify thread
> > as well.
> >
> > In actuality, it never had anything to do with dir_notify anyway. All
> > it did was unnecessarily wake up all the tasks waiting on the response
> > queues every 15s. Previously that happened to prevent tasks from hanging
> > indefinitely when the server went unresponsive, but we put those to
> > sleep with proper timeouts now so there's no reason to keep this around.
> >
> > Signed-off-by: Jeff Layton <jlayton at redhat.com>
> > ---
> >  fs/cifs/cifsfs.c |   47 -----------------------------------------------
> >  1 files changed, 0 insertions(+), 47 deletions(-)
> >
> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> > index 13ea532..5626af2 100644
> > --- a/fs/cifs/cifsfs.c
> > +++ b/fs/cifs/cifsfs.c
> > @@ -66,9 +66,6 @@ unsigned int sign_CIFS_PDUs = 1;
> >  extern struct task_struct *oplockThread; /* remove sparse warning */
> >  struct task_struct *oplockThread = NULL;
> >  /* extern struct task_struct * dnotifyThread; remove sparse warning */
> > -#ifdef CONFIG_CIFS_EXPERIMENTAL
> > -static struct task_struct *dnotifyThread = NULL;
> > -#endif
> >  static const struct super_operations cifs_super_ops;
> >  unsigned int CIFSMaxBufSize = CIFS_MAX_MSGSIZE;
> >  module_param(CIFSMaxBufSize, int, 0);
> > @@ -1039,34 +1036,6 @@ static int cifs_oplock_thread(void *dummyarg)
> >        return 0;
> >  }
> >
> > -#ifdef CONFIG_CIFS_EXPERIMENTAL
> > -static int cifs_dnotify_thread(void *dummyarg)
> > -{
> > -       struct list_head *tmp;
> > -       struct TCP_Server_Info *server;
> > -
> > -       do {
> > -               if (try_to_freeze())
> > -                       continue;
> > -               set_current_state(TASK_INTERRUPTIBLE);
> > -               schedule_timeout(15*HZ);
> > -               /* check if any stuck requests that need
> > -                  to be woken up and wakeq so the
> > -                  thread can wake up and error out */
> > -               read_lock(&cifs_tcp_ses_lock);
> > -               list_for_each(tmp, &cifs_tcp_ses_list) {
> > -                       server = list_entry(tmp, struct TCP_Server_Info,
> > -                                        tcp_ses_list);
> > -                       if (atomic_read(&server->inFlight))
> > -                               wake_up_all(&server->response_q);
> > -               }
> > -               read_unlock(&cifs_tcp_ses_lock);
> > -       } while (!kthread_should_stop());
> > -
> > -       return 0;
> > -}
> > -#endif
> > -
> >  static int __init
> >  init_cifs(void)
> >  {
> > @@ -1143,21 +1112,8 @@ init_cifs(void)
> >                goto out_unregister_dfs_key_type;
> >        }
> >
> > -#ifdef CONFIG_CIFS_EXPERIMENTAL
> > -       dnotifyThread = kthread_run(cifs_dnotify_thread, NULL, "cifsdnotifyd");
> > -       if (IS_ERR(dnotifyThread)) {
> > -               rc = PTR_ERR(dnotifyThread);
> > -               cERROR(1, ("error %d create dnotify thread", rc));
> > -               goto out_stop_oplock_thread;
> > -       }
> > -#endif
> > -
> >        return 0;
> >
> > -#ifdef CONFIG_CIFS_EXPERIMENTAL
> > - out_stop_oplock_thread:
> > -#endif
> > -       kthread_stop(oplockThread);
> >  out_unregister_dfs_key_type:
> >  #ifdef CONFIG_CIFS_DFS_UPCALL
> >        unregister_key_type(&key_type_dns_resolver);
> > @@ -1195,9 +1151,6 @@ exit_cifs(void)
> >        cifs_destroy_inodecache();
> >        cifs_destroy_mids();
> >        cifs_destroy_request_bufs();
> > -#ifdef CONFIG_CIFS_EXPERIMENTAL
> > -       kthread_stop(dnotifyThread);
> > -#endif
> >        kthread_stop(oplockThread);
> >  }
> >
> > --
> > 1.5.5.1
> >
> >
> 
> 
> 
> -- 
> Thanks,
> 
> Steve


-- 
Jeff Layton <jlayton at redhat.com>