[linux-cifs-client] Cannot allocate memory

Commit Message

Pavel Shilovsky March 13, 2009, 12:06 p.m. UTC

Jeff Layton wrote:

>> >> @@ -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);
>>     
> >
> >^^^ this is pretty yucky though. Polling and sleeping isn't the way to do 
> >this. A completion variable + wait_for_completion() might be a better fit.
>   

Ok, thanks a lot for advices.



-- Best regards, Pavel Shilovsky.

Comments

Jeff Layton March 14, 2009, 8:07 p.m. UTC | #1

On Fri, 13 Mar 2009 15:06:30 +0300
Pavel Shilovsky <[e-mail]> wrote:

> Jeff Layton wrote:
> 
> >> >> @@ -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);
> >>     
> > >
> > >^^^ this is pretty yucky though. Polling and sleeping isn't the way to do 
> > >this. A completion variable + wait_for_completion() might be a better fit.
> >   
> 
> Ok, thanks a lot for advices.
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 9fbf4df..a5989aa 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 */
> +       struct completion done;
>  };
> 
>  /*
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index cd4ccc8..480c48e 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;
> 
> +       init_completion(&server->done);
>         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);
>         }
> 
> +       complete_all(&server->done);
> +
>         module_put_and_exit(0);
>  }
> 
> @@ -1418,6 +1420,10 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
>         task = xchg(&server->tsk, NULL);
>         if (task)
>                 force_sig(SIGKILL, task);
> +
> +       wait_for_completion_interruptible(&server->done);
> +
> +       kfree(server);
>  }
> 
>  static struct TCP_Server_Info *
> 

Looks good to me. It might be nice to see this in conjunction with some
patches that make cifs_demultiplex_thread shut down more quickly, but I
don't guess we need to wait for that to take this one. You can add my
ack...

Acked-by: Jeff Layton <[e-mail]>

Patch

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 9fbf4df..a5989aa 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 */
+       struct completion done;
 };

 /*
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index cd4ccc8..480c48e 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;

+       init_completion(&server->done);
        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);
        }

+       complete_all(&server->done);
+
        module_put_and_exit(0);
 }

@@ -1418,6 +1420,10 @@  cifs_put_tcp_session(struct TCP_Server_Info *server)
        task = xchg(&server->tsk, NULL);
        if (task)
                force_sig(SIGKILL, task);
+
+       wait_for_completion_interruptible(&server->done);
+
+       kfree(server);
 }

 static struct TCP_Server_Info *