[linux-cifs-client] [PATCH] Cannot allocate memory

[Pavel Shilovsky]

Jeff Layton wrote:
> You have no guarantee that "server" is still a valid pointer when you
> go to dereference it. In most cases it probably will be, but hard to
> hit races are *worse* since they're harder to track down.
>
> We have to *know* that this pointer will be valid here before you do
> something like this. Rather than adding arbitrary delays like this, a
> better scheme would use a completion variable to indicate when the
> thread is down.
>   
Yes, you are right. I fixed it.
> Finally, I'm not convinced of the need for this at all. Why should we
> guarantee that the module is able to be unplugged the instant that the
> last mount vanishes? I'd much rather my umounts return quickly rather
> than have them wait for the thread to come down.

I want to have cifs module not in use when I unmounted all share bases. 
I need it, for example, when I right any script that unmouts all shares 
and then deletes cifs module. In your solution, I have to wait some 
unknown time before deleting module. This is uncomfortably, I think. 
It's much more logical to think that all stuff connected with share 
disappears as soon as unmount finishes it's work.

Here is my new patch:

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 9fbf4df..0ec5a2f 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -182,6 +182,7 @@ struct TCP_Server_Info {
     struct mac_key mac_signing_key;
     char ntlmv2_hash[16];
     unsigned long lstrp; /* when we got last response from this server */
+    int running;
 };
 
 /*
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index cd4ccc8..bc5841d 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -337,6 +337,7 @@ cifs_demultiplex_thread(struct TCP_Server_Info *server)
     bool isMultiRsp;
     int reconnect;
 
+    server->running = 1;
     current->flags |= PF_MEMALLOC;
     cFYI(1, ("Demultiplex PID: %d", task_pid_nr(current)));
 
@@ -747,7 +748,6 @@ multi_t2_fnd:
 
     kfree(server->hostname);
     task_to_wake = xchg(&server->tsk, NULL);
-    kfree(server);
 
     length = atomic_dec_return(&tcpSesAllocCount);
     if (length  > 0)
@@ -764,6 +764,8 @@ multi_t2_fnd:
         set_current_state(TASK_RUNNING);
     }
 
+    server->running = 0;
+
     module_put_and_exit(0);
 }
 
@@ -1418,6 +1420,11 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
     task = xchg(&server->tsk, NULL);
     if (task)
         force_sig(SIGKILL, task);
+
+    while(server->running == 1)
+        msleep(10);
+
+    kfree(server);
 }
 
 static struct TCP_Server_Info *




--
Best regards,
Pavel Shilovsky.