From patchwork Wed Mar 11 14:05:00 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Shilovsky X-Patchwork-Id: 11143 Received: from lists.samba.org (mail.samba.org [66.70.73.150]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n2BE5WOs030752 for ; Wed, 11 Mar 2009 14:05:32 GMT Received: from dp.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 0050D163CE7 for ; Wed, 11 Mar 2009 14:05:17 +0000 (GMT) X-Spam-Checker-Version: SpamAssassin 3.1.7 (2006-10-05) on dp.samba.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.8 tests=AWL, BAYES_00 autolearn=ham version=3.1.7 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from mail.etersoft.ru (mail.etersoft.ru [87.249.47.46]) by lists.samba.org (Postfix) with ESMTP id 1546C163B5C for ; Wed, 11 Mar 2009 14:04:40 +0000 (GMT) Received: from localhost (as.office.etersoft.ru [192.168.0.10]) by mail.etersoft.ru (Postfix) with ESMTP id 4F3402DD728; Wed, 11 Mar 2009 17:04:54 +0300 (MSK) X-Virus-Scanned: amavisd-new at office.etersoft.ru Received: from mail.etersoft.ru ([192.168.0.1]) by localhost (as.office.etersoft.ru [192.168.0.10]) (amavisd-new, port 10024) with LMTP id YOjg267oW83P; Wed, 11 Mar 2009 17:04:50 +0300 (MSK) Message-ID: <49B7C50C.5040204@etersoft.ru> Date: Wed, 11 Mar 2009 17:05:00 +0300 From: Pavel Shilovsky User-Agent: Thunderbird 2.0.0.18 (X11/20081125) MIME-Version: 1.0 To: Jeff Layton Subject: Re: [linux-cifs-client] [PATCH] Cannot allocate memory References: <49B687D8.2060600@etersoft.ru> <20090310150128.46aa7853@tleilax.poochiereds.net> <200903110043.29073.piastry@etersoft.ru> <20090310191133.4cc7624c@tleilax.poochiereds.net> In-Reply-To: <20090310191133.4cc7624c@tleilax.poochiereds.net> Cc: linux-cifs-client@lists.samba.org X-BeenThere: linux-cifs-client@lists.samba.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: The Linux CIFS VFS client List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cifs-client-bounces+patchwork-cifs-client=patchwork.kernel.org@lists.samba.org Errors-To: linux-cifs-client-bounces+patchwork-cifs-client=patchwork.kernel.org@lists.samba.org 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: --- Best regards, Pavel Shilovsky. 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 *