Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:46604 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757742AbcEEQkN (ORCPT ); Thu, 5 May 2016 12:40:13 -0400 Subject: Re: [PATCH 2/2] gssd: move read of upcall into main thread To: "Kornievskaia, Olga" References: <1462286792-8889-1-git-send-email-kolga@netapp.com> <1462286792-8889-2-git-send-email-kolga@netapp.com> <50edf6a8-4d4c-07d5-af0a-363ac66dbfa5@RedHat.com> <574d004f-4b42-d4aa-60f6-607ffae6dd1b@RedHat.com> Cc: "linux-nfs@vger.kernel.org" From: Steve Dickson Message-ID: <222b4f6c-6f7a-2951-6d4e-75a522cde905@RedHat.com> Date: Thu, 5 May 2016 12:40:11 -0400 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Sender: linux-nfs-owner@vger.kernel.org List-ID: On 05/05/2016 11:40 AM, Kornievskaia, Olga wrote: > >> On May 5, 2016, at 11:35 AM, Kornievskaia, Olga > wrote: >> >>> >>> On May 5, 2016, at 10:17 AM, Steve Dickson > wrote: >>> >>> Hello, >>> >>> On 05/04/2016 10:15 AM, Kornievskaia, Olga wrote: >>>> I?m having a hard time picturing what you have in mind. I don?t see how you can allocate, >>>> populate and free in one routine. The logic is we allocate memory, then read the upcall >>>> and only then we create the thread. If either upcall or creation of the thread >>>> fails we need to free the allocated memory. >>> Something like this: >>> >>> gssd_clnt_gssd_cb(int UNUSED(fd), short UNUSED(which), void *data) >>> { >>> struct clnt_info *clp = data; >>> - pthread_t th; >>> - int ret; >>> + struct clnt_upcall_info *info; >>> >>> - ret = pthread_create(&th, NULL, (void *)handle_gssd_upcall, >>> - (void *)clp); >>> - if (ret != 0) { >>> - printerr(0, "ERROR: pthread_create failed: ret %d: %s\n", >>> - ret, strerror(errno)); >>> + info = alloc_upcall_info(clp); >>> + if (info == NULL) >>> + return; >>> + >>> + info->lbuflen = read(clp->gssd_fd, info->lbuf, sizeof(info->lbuf)); >>> + if (info->lbuflen <= 0 || info->lbuf[info->lbuflen-1] != '\n') { >>> + printerr(0, "WARNING: %s: failed reading request\n", __func__); >>> + free(info); >>> return; >>> } >>> - wait_for_child_and_detach(th); >>> + info->lbuf[info->lbuflen-1] = 0; >>> + >>> + start_upcall_thread(handle_gssd_upcall, info); >>> + >>> + free(info); >>> + return; >> >> Gotcha! Sounds good! You can make the modifications. Or just in case since you are traveling, > I?ll just send another patch. Please also add any other lines like Modified-by or Signed-off-by that are need. > > I?m sorry I spoke too soon. ?info? is allocated and given to the thread > to process so the main thread can?t free it. I'm curious as to why? The main thread allocated it, is handing allocated memory to a thread something special? What am I missing? steved.