Re: [PATCH 02/12] Btrfs-progs: don't close the file descriptor 0 when closing a device

[Filipe David Manana <fdmanana@gmail.com>]

On Thu, Jul 4, 2013 at 2:30 AM, Miao Xie <miaox@cn.fujitsu.com> wrote:
> On wed, 3 Jul 2013 15:17:02 +0100, Filipe David Manana wrote:
>> On Wed, Jul 3, 2013 at 2:25 PM, Miao Xie <miaox@cn.fujitsu.com> wrote:
>>>
>>> +++ b/disk-io.c
>>> @@ -1270,12 +1270,13 @@ static int close_all_devices(struct btrfs_fs_info *fs_info)
>>>         while (!list_empty(list)) {
>>>                 device = list_entry(list->next, struct btrfs_device, dev_list);
>>>                 list_del_init(&device->dev_list);
>>> -               if (device->fd) {
>>> +               if (device->fd != -1) {
>>>                         fsync(device->fd);
>>>                         if (posix_fadvise(device->fd, 0, 0, POSIX_FADV_DONTNEED))
>>>                                 fprintf(stderr, "Warning, could not drop caches\n");
>>> +                       close(device->fd);
>>> +                       device->fd = -1;
>>>                 }
>>> -               close(device->fd);
>>>                 kfree(device->name);
>>>                 kfree(device->label);
>>>                 kfree(device);
>>
>> I deal with this part too at https://patchwork.kernel.org/patch/2787291/
>
> Sorry, I don't know you have dealt with it. But your patch didn't fix the problem completely,
> there are still some functions that you didn't deal with.

Yes, I was addressing a different problem. My comment was relative
only to this change in disk-io.c:close_all_devices().

thanks

>
>> Is there any reason to set device->fd to -1 if we just kfree(device)
>> shortly after?
>
> Right, I will update my patch.
>
> Thanks
> Miao
>
>> thanks
>>
>>
>> --
>> Filipe David Manana,
>>
>> "Reasonable men adapt themselves to the world.
>>  Unreasonable men adapt the world to themselves.
>>  That's why all progress depends on unreasonable men."
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>



--
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."