[linux-cifs-client] cifs: remove dnotify thread code
diff mbox

Message ID 1231424128-5598-1-git-send-email-jlayton@redhat.com
State New, archived
Headers show

Commit Message

Jeff Layton Jan. 8, 2009, 2:15 p.m. UTC
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@redhat.com>
---
 fs/cifs/cifsfs.c |   47 -----------------------------------------------
 1 files changed, 0 insertions(+), 47 deletions(-)

Comments

Steve French Jan. 8, 2009, 2:23 p.m. UTC | #1
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

On Thu, Jan 8, 2009 at 8:15 AM, Jeff Layton <jlayton@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@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
>
>
Jeff Layton Jan. 8, 2009, 2:51 p.m. UTC | #2
On Thu, 8 Jan 2009 08:23:49 -0600
"Steve French" <smfrench@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@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@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
Jamie Lokier Jan. 9, 2009, 12:07 a.m. UTC | #3
Steve French 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 think NFSv4 can do it with delegations, although the exact semantics
an app could rely on would be different.

There are several different kinds of notify that apps could use for
different purposes: 1. Queued (an event is posted after a change, will
eventually reach the app).  2. Coherent (the app can ask "any
notifications for me" and if the answer is no, it can be sure that an
attribute/data read issued prior to asking would have yielded what the
app already cached; a sort of app cache validation).  3. Lease (the
app is notified and must respond prior to a change being allowed to
proceed).

These form a hierarchy: if the OS/filesystem provides Lease (3), the
app can use it in place of Coherent (2) and Queued (1).  If the
OS/filesystem provides Lease (3) or Coherent (2), the app can use
either in place of Queued (1).

All of these have different, useful applications.  GUIs like Nautilus
are happy with Queued (1).  For content indexers, it depends how you
want them to behave, and how up to date they should seem.  For
something which computes things from file contents or attributes, such
as (possible beneficiaries) any scripting language, Make, Git, JIT
system, or web templating system, it needs Coherent (2) notifications;
Queued (1) is not reliable for caching.  Lease (3) is potentially
useful for distributed databases.

For CIFS/SMB it looks like all three could be implemented, in
different ways.  I'm not sure if NFSv4 can do Queued, but I think with
delegations it can do Coherent and Lease.

For local filesystems, I think Linux provides Coherent but I haven't
looked closely.  If not, it provides Queued.

Al Viro said (long ago) apps should not rely upon timely
dnotify/inotify events, and therefore should not use them for
consistent app caching.  However, timely delivery isn't required,
what's required is that reading the inotify descriptor (or reading a
flag set by delivery of the dnotify signal / inotify SIGIO?) will
definitely return an event if the corresponding file change has become
observable by other means.  It is about ordering guarantees.

If there is a revamp of the fsnotify code for networking especially,
please at least have a little think about the different event delivery
models and what apps can expect, when which semantics to support.  The
different models are each useful, and may involve different parts of
the networked filesystem protocol.

-- Jamie
Jeff Layton Jan. 9, 2009, 1:28 a.m. UTC | #4
On Fri, 9 Jan 2009 00:07:08 +0000
Jamie Lokier <jamie@shareable.org> wrote:

> Steve French 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 think NFSv4 can do it with delegations, although the exact semantics
> an app could rely on would be different.
> 

I don't think delegations help here since it's entirely up to the
server whether to grant one or not. I've only given inotify/dnotify a
drive-by look, but I'm pretty sure you'd want the client to be able to
set up events to monitor.

> There are several different kinds of notify that apps could use for
> different purposes: 1. Queued (an event is posted after a change, will
> eventually reach the app).  2. Coherent (the app can ask "any
> notifications for me" and if the answer is no, it can be sure that an
> attribute/data read issued prior to asking would have yielded what the
> app already cached; a sort of app cache validation).  3. Lease (the
> app is notified and must respond prior to a change being allowed to
> proceed).
> 
> These form a hierarchy: if the OS/filesystem provides Lease (3), the
> app can use it in place of Coherent (2) and Queued (1).  If the
> OS/filesystem provides Lease (3) or Coherent (2), the app can use
> either in place of Queued (1).
> 
> All of these have different, useful applications.  GUIs like Nautilus
> are happy with Queued (1).  For content indexers, it depends how you
> want them to behave, and how up to date they should seem.  For
> something which computes things from file contents or attributes, such
> as (possible beneficiaries) any scripting language, Make, Git, JIT
> system, or web templating system, it needs Coherent (2) notifications;
> Queued (1) is not reliable for caching.  Lease (3) is potentially
> useful for distributed databases.
> 
> For CIFS/SMB it looks like all three could be implemented, in
> different ways.  I'm not sure if NFSv4 can do Queued, but I think with
> delegations it can do Coherent and Lease.
> 
> For local filesystems, I think Linux provides Coherent but I haven't
> looked closely.  If not, it provides Queued.
> 
> Al Viro said (long ago) apps should not rely upon timely
> dnotify/inotify events, and therefore should not use them for
> consistent app caching.  However, timely delivery isn't required,
> what's required is that reading the inotify descriptor (or reading a
> flag set by delivery of the dnotify signal / inotify SIGIO?) will
> definitely return an event if the corresponding file change has become
> observable by other means.  It is about ordering guarantees.
> 
> If there is a revamp of the fsnotify code for networking especially,
> please at least have a little think about the different event delivery
> models and what apps can expect, when which semantics to support.  The
> different models are each useful, and may involve different parts of
> the networked filesystem protocol.
> 

CIFS has a call that tells the server to notify the client when a
directory changes (NT_TRANSACT_NOTIFY_CHANGE). This, in principle would
allow us to implement a subset of inotify/dnotify across the network.
I'm not sure which kind we'd be able to implement (probably "queued
at best).

I wholeheartedly agree however that this is something that we should
implement with care. What little code there was in place for this is
now mostly gone. The only thing that remains is this kthread that
currently serves no purpose. When/if we do this, we'll want to
reimplement all of this from the ground up anyway.
Steve French Jan. 9, 2009, 3:32 a.m. UTC | #5
What worries me most about losing the ability to add directory change
notification in the future (without putting the entry point back) the
lack of directory change notification is quite visible to the user
when an open directory object on the desktop (gnome or kde) does not
automatically refresh within a few seconds as files are added on the
remote system (or on another client).

The old cifs implementation was broken (partially implemented) but it
was "dead code" so was harmless (it required configuring with
CONFIG_CIFS_EXPERIMENTAL and turning on a run time flag in
/proc/fs/cifs to enable it) but it might not have been too bad to
finish the implementation.

On Thu, Jan 8, 2009 at 6:07 PM, Jamie Lokier <jamie@shareable.org> wrote:
> Steve French 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 think NFSv4 can do it with delegations, although the exact semantics
> an app could rely on would be different.
>
> There are several different kinds of notify that apps could use for
> different purposes: 1. Queued (an event is posted after a change, will
> eventually reach the app).  2. Coherent (the app can ask "any
> notifications for me" and if the answer is no, it can be sure that an
> attribute/data read issued prior to asking would have yielded what the
> app already cached; a sort of app cache validation).  3. Lease (the
> app is notified and must respond prior to a change being allowed to
> proceed).
>
> These form a hierarchy: if the OS/filesystem provides Lease (3), the
> app can use it in place of Coherent (2) and Queued (1).  If the
> OS/filesystem provides Lease (3) or Coherent (2), the app can use
> either in place of Queued (1).
>
> All of these have different, useful applications.  GUIs like Nautilus
> are happy with Queued (1).  For content indexers, it depends how you
> want them to behave, and how up to date they should seem.  For
> something which computes things from file contents or attributes, such
> as (possible beneficiaries) any scripting language, Make, Git, JIT
> system, or web templating system, it needs Coherent (2) notifications;
> Queued (1) is not reliable for caching.  Lease (3) is potentially
> useful for distributed databases.
>
> For CIFS/SMB it looks like all three could be implemented, in
> different ways.  I'm not sure if NFSv4 can do Queued, but I think with
> delegations it can do Coherent and Lease.
>
> For local filesystems, I think Linux provides Coherent but I haven't
> looked closely.  If not, it provides Queued.
>
> Al Viro said (long ago) apps should not rely upon timely
> dnotify/inotify events, and therefore should not use them for
> consistent app caching.  However, timely delivery isn't required,
> what's required is that reading the inotify descriptor (or reading a
> flag set by delivery of the dnotify signal / inotify SIGIO?) will
> definitely return an event if the corresponding file change has become
> observable by other means.  It is about ordering guarantees.
>
> If there is a revamp of the fsnotify code for networking especially,
> please at least have a little think about the different event delivery
> models and what apps can expect, when which semantics to support.  The
> different models are each useful, and may involve different parts of
> the networked filesystem protocol.
>
> -- Jamie
>
Jeff Layton Jan. 9, 2009, 11:33 a.m. UTC | #6
On Thu, 8 Jan 2009 21:32:09 -0600
"Steve French" <smfrench@gmail.com> wrote:

> What worries me most about losing the ability to add directory change
> notification in the future (without putting the entry point back) the
> lack of directory change notification is quite visible to the user
> when an open directory object on the desktop (gnome or kde) does not
> automatically refresh within a few seconds as files are added on the
> remote system (or on another client).
> 
> The old cifs implementation was broken (partially implemented) but it
> was "dead code" so was harmless (it required configuring with
> CONFIG_CIFS_EXPERIMENTAL and turning on a run time flag in
> /proc/fs/cifs to enable it) but it might not have been too bad to
> finish the implementation.
> 

This patch doesn't remove any capability of the current code. It just
gets rid of this kthread that doesn't do anything useful. There's
nothing stopping us from putting it back later once we have working dir
notification, but until then it's just doing unnecessary wakeups.
Al Viro Jan. 9, 2009, 6:18 p.m. UTC | #7
On Thu, Jan 08, 2009 at 09:32:09PM -0600, Steve French wrote:
> What worries me most about losing the ability to add directory change
> notification in the future (without putting the entry point back) the

If you put that one entry point back, please make sure to reserve a CVE number
first.

> lack of directory change notification is quite visible to the user
> when an open directory object on the desktop (gnome or kde) does not
> automatically refresh within a few seconds as files are added on the
> remote system (or on another client).
> 
> The old cifs implementation was broken (partially implemented) but it
> was "dead code" so was harmless (it required configuring with
> CONFIG_CIFS_EXPERIMENTAL and turning on a run time flag in
> /proc/fs/cifs to enable it) but it might not have been too bad to
> finish the implementation.

The interface for it had been inherently broken.  Whatever you do with the
cifs side, you'd have to change the VFS side.  Might as well start from scratch.
Again, *any* experimenting with that crap will have to start with figuring
out what to do with destruction of these suckers.
Jamie Lokier Jan. 10, 2009, 1:35 a.m. UTC | #8
Jeff Layton wrote:
> CIFS has a call that tells the server to notify the client when a
> directory changes (NT_TRANSACT_NOTIFY_CHANGE). This, in principle would
> allow us to implement a subset of inotify/dnotify across the network.
> I'm not sure which kind we'd be able to implement (probably "queued
> at best).

With oplocks you can implement "lease" (and therefore "coherent") for
file data reads and writes, at least.  I'm not sure if it also covers
attributes and directory operations.

You would use NT_TRANSACT_NOTIFY_CHANGE for "queued" when possible,
because it's much cheaper and intended for this, although you could
use oplocks (expensively) for this too.

-- Jamie
Jamie Lokier Jan. 10, 2009, 1:41 a.m. UTC | #9
Jeff Layton wrote:
> > I think NFSv4 can do it with delegations, although the exact semantics
> > an app could rely on would be different.
> 
> I don't think delegations help here since it's entirely up to the
> server whether to grant one or not. I've only given inotify/dnotify a
> drive-by look, but I'm pretty sure you'd want the client to be able to
> set up events to monitor.

The notify API needs the ability to report, in general, whether you'll
get notify events from a particular filesystem / file / directory /
whatever-condition anyway.  And also whether you get events for all
(remote) changes, or just local changes.

You might as well _try_ to get a delegate, and if you fail, tell the
caller that it won't receive notifications on this file and will have
to poll - just like most other remote filesystems.

-- Jamie
Steve French Jan. 10, 2009, 6:19 p.m. UTC | #10
It would be really helpful if we could get directory delegations over
CIFS (or SMB2) so we could avoid worrying about notifications of
directory change events, but in practice it seems like the performance
impact (to Windows servers, Samba servers, and various NAS filers) of
the existing "multi-shot" directory change notifications have not been
too bad.

File change notifications are helpful in some environments, and with
leases (oplocks) don't even have to be sent to the server but it seems
like directory change notifications (adding/deleting files in a
directory) is the more "user visible"

Interesting the notify code was originally added to Linux to support
Samba (at Tridge's request many years ago) which needed it to handle
Windows (client) explorer desktop (which calls the Windows equivalent
notify)

On Fri, Jan 9, 2009 at 7:41 PM, Jamie Lokier <jamie@shareable.org> wrote:
> Jeff Layton wrote:
>> > I think NFSv4 can do it with delegations, although the exact semantics
>> > an app could rely on would be different.
>>
>> I don't think delegations help here since it's entirely up to the
>> server whether to grant one or not. I've only given inotify/dnotify a
>> drive-by look, but I'm pretty sure you'd want the client to be able to
>> set up events to monitor.
>
> The notify API needs the ability to report, in general, whether you'll
> get notify events from a particular filesystem / file / directory /
> whatever-condition anyway.  And also whether you get events for all
> (remote) changes, or just local changes.
>
> You might as well _try_ to get a delegate, and if you fail, tell the
> caller that it won't receive notifications on this file and will have
> to poll - just like most other remote filesystems.
>
> -- Jamie
>

Patch
diff mbox

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);
 }