2017-03-24 09:02:26

by Denis Kirjanov

[permalink] [raw]
Subject: __link_block_group uses GFP_KERNEL

Hi guys,

Looks like that current code does GFP_KERNEL allocation inside
__link_block_group.
the function invokes kobject_add and internally creates sysfs files
with the GFP_KERNEL flag set.
But since do_chunk_alloc executes insides the btrfs transaction it's
not allowed to sleep.

Thanks!


2017-03-24 23:13:58

by Jeff Mahoney

[permalink] [raw]
Subject: Re: __link_block_group uses GFP_KERNEL

On 3/24/17 5:02 AM, Denis Kirjanov wrote:
> Hi guys,
>
> Looks like that current code does GFP_KERNEL allocation inside
> __link_block_group.
> the function invokes kobject_add and internally creates sysfs files
> with the GFP_KERNEL flag set.

Yep, that's a bug.

> But since do_chunk_alloc executes insides the btrfs transaction it's
> not allowed to sleep.

It's allowed to sleep but isn't allowed to do reclaim that involves file
system writeback. Michal Hocko's allocation context idea would fix
this, but it's not there yet, so we'll need to defer the kobject_add
until we can use GFP_KERNEL.

-Jeff

--
Jeff Mahoney
SUSE Labs


Attachments:
signature.asc (819.00 B)
OpenPGP digital signature

2017-03-25 06:48:37

by Denis Kirjanov

[permalink] [raw]
Subject: Re: __link_block_group uses GFP_KERNEL

On 3/25/17, Jeff Mahoney <[email protected]> wrote:
> On 3/24/17 5:02 AM, Denis Kirjanov wrote:
>> Hi guys,
>>
>> Looks like that current code does GFP_KERNEL allocation inside
>> __link_block_group.
>> the function invokes kobject_add and internally creates sysfs files
>> with the GFP_KERNEL flag set.
>
> Yep, that's a bug.
>
>> But since do_chunk_alloc executes insides the btrfs transaction it's
>> not allowed to sleep.
>
> It's allowed to sleep but isn't allowed to do reclaim that involves file
> system writeback. Michal Hocko's allocation context idea would fix
> this, but it's not there yet, so we'll need to defer the kobject_add
> until we can use GFP_KERNEL.

Ok, I see. Can you point out to the initial patchset?

Thanks!

>
> -Jeff
>
> --
> Jeff Mahoney
> SUSE Labs
>
>

2017-03-27 17:20:24

by David Sterba

[permalink] [raw]
Subject: Re: __link_block_group uses GFP_KERNEL

On Sat, Mar 25, 2017 at 09:48:28AM +0300, Denis Kirjanov wrote:
> On 3/25/17, Jeff Mahoney <[email protected]> wrote:
> > On 3/24/17 5:02 AM, Denis Kirjanov wrote:
> >> Hi guys,
> >>
> >> Looks like that current code does GFP_KERNEL allocation inside
> >> __link_block_group.
> >> the function invokes kobject_add and internally creates sysfs files
> >> with the GFP_KERNEL flag set.
> >
> > Yep, that's a bug.
> >
> >> But since do_chunk_alloc executes insides the btrfs transaction it's
> >> not allowed to sleep.
> >
> > It's allowed to sleep but isn't allowed to do reclaim that involves file
> > system writeback. Michal Hocko's allocation context idea would fix
> > this, but it's not there yet, so we'll need to defer the kobject_add
> > until we can use GFP_KERNEL.
>
> Ok, I see. Can you point out to the initial patchset?

https://lwn.net/Articles/716323/

Fixing this properly is a lot of work so we might need to add a
temporary workaround, as Jeff suggests, to move calling into sysfs to a
later time.

2017-03-28 08:16:53

by Denis Kirjanov

[permalink] [raw]
Subject: Re: __link_block_group uses GFP_KERNEL

On 3/27/17, David Sterba <[email protected]> wrote:
> On Sat, Mar 25, 2017 at 09:48:28AM +0300, Denis Kirjanov wrote:
>> On 3/25/17, Jeff Mahoney <[email protected]> wrote:
>> > On 3/24/17 5:02 AM, Denis Kirjanov wrote:
>> >> Hi guys,
>> >>
>> >> Looks like that current code does GFP_KERNEL allocation inside
>> >> __link_block_group.
>> >> the function invokes kobject_add and internally creates sysfs files
>> >> with the GFP_KERNEL flag set.
>> >
>> > Yep, that's a bug.
>> >
>> >> But since do_chunk_alloc executes insides the btrfs transaction it's
>> >> not allowed to sleep.
>> >
>> > It's allowed to sleep but isn't allowed to do reclaim that involves
>> > file
>> > system writeback. Michal Hocko's allocation context idea would fix
>> > this, but it's not there yet, so we'll need to defer the kobject_add
>> > until we can use GFP_KERNEL.
>>
>> Ok, I see. Can you point out to the initial patchset?
>
> https://lwn.net/Articles/716323/
>
> Fixing this properly is a lot of work so we might need to add a
> temporary workaround, as Jeff suggests, to move calling into sysfs to a
> later time.
>

Care to send a patch?
Or I can dig a bit.

Thanks!