Re: [PATCH 2/2] gssd: move read of upcall into main thread

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On May 5, 2016, at 12:40 PM, Steve Dickson <SteveD@xxxxxxxxxx> wrote:
> 
> 
> 
> On 05/05/2016 11:40 AM, Kornievskaia, Olga wrote:
>> 
>>> On May 5, 2016, at 11:35 AM, Kornievskaia, Olga <Olga.Kornievskaia@xxxxxxxxxx <mailto:Olga.Kornievskaia@xxxxxxxxxx>> wrote:
>>> 
>>>> 
>>>> On May 5, 2016, at 10:17 AM, Steve Dickson <SteveD@xxxxxxxxxx <mailto:SteveD@xxxxxxxxxx>> 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?

The main thread allocates memory, gives it to the the child thread and goes to the next libevent. We detach the thread so if we free the memory in the main thread, the child thread would segfault.

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux