On Thu, 8 Jan 2009 08:23:49 -0600
"Steve French" <smfrench@xxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
