[PATCH v2] bdi: show error log when fail to create bdi debugfs entry

[PATCH v2] bdi: show error log when fail to create bdi debugfs entry

[weiping zhang]

bdi debugfs dir/file may create fail, add error log here.

Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
---
V1->V2:
fix indentation and make log message more clear

 mm/backing-dev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index b5f940c..0a49665 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -885,7 +885,9 @@ int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args)
 	cgwb_bdi_register(bdi);
 	bdi->dev = dev;
 
-	bdi_debug_register(bdi, dev_name(dev));
+	if (bdi_debug_register(bdi, dev_name(dev)))
+		pr_warn("blkdev %s: creation of bdi debugfs entries failed.\n",
+			dev_name(dev));
 	set_bit(WB_registered, &bdi->wb.state);
 
 	spin_lock_bh(&bdi_lock);
-- 
2.9.4

Re: [PATCH v2] bdi: show error log when fail to create bdi debugfs entry

[weiping zhang]

2018-01-11 0:36 GMT+08:00 weiping zhang <zhangweiping@didichuxing.com>:
> bdi debugfs dir/file may create fail, add error log here.
>
> Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
> ---
> V1->V2:
> fix indentation and make log message more clear
>
>  mm/backing-dev.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index b5f940c..0a49665 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -885,7 +885,9 @@ int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args)
>         cgwb_bdi_register(bdi);
>         bdi->dev = dev;
>
> -       bdi_debug_register(bdi, dev_name(dev));
> +       if (bdi_debug_register(bdi, dev_name(dev)))
> +               pr_warn("blkdev %s: creation of bdi debugfs entries failed.\n",
> +                       dev_name(dev));
>         set_bit(WB_registered, &bdi->wb.state);
>
>         spin_lock_bh(&bdi_lock);
> --

Hi Jens,

madam has no permission to create debuts entry if SELINUX is enable at
Fedora and Centos,
and we have revert 6d0e4827b72 Revert "bdi: add error handle for
bdi_debug_register", that is to say
bdi debugfs is not the key component of block device, this patch just
add warning log.

Thanks

Re: [PATCH v2] bdi: show error log when fail to create bdi debugfs entry

[Jens Axboe]

On 1/19/18 10:36 AM, weiping zhang wrote:
> 2018-01-11 0:36 GMT+08:00 weiping zhang <zhangweiping@didichuxing.com>:
>> bdi debugfs dir/file may create fail, add error log here.
>>
>> Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
>> ---
>> V1->V2:
>> fix indentation and make log message more clear
>>
>>  mm/backing-dev.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
>> index b5f940c..0a49665 100644
>> --- a/mm/backing-dev.c
>> +++ b/mm/backing-dev.c
>> @@ -885,7 +885,9 @@ int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args)
>>         cgwb_bdi_register(bdi);
>>         bdi->dev = dev;
>>
>> -       bdi_debug_register(bdi, dev_name(dev));
>> +       if (bdi_debug_register(bdi, dev_name(dev)))
>> +               pr_warn("blkdev %s: creation of bdi debugfs entries failed.\n",
>> +                       dev_name(dev));
>>         set_bit(WB_registered, &bdi->wb.state);
>>
>>         spin_lock_bh(&bdi_lock);
>> --
> 
> Hi Jens,
> 
> madam has no permission to create debuts entry if SELINUX is enable at
> Fedora and Centos,
> and we have revert 6d0e4827b72 Revert "bdi: add error handle for
> bdi_debug_register", that is to say
> bdi debugfs is not the key component of block device, this patch just
> add warning log.

Have we fixed the case where we know it will trigger?

-- 
Jens Axboe

Re: [PATCH v2] bdi: show error log when fail to create bdi debugfs entry

[weiping zhang]

2018-01-20 3:54 GMT+08:00 Jens Axboe <axboe@kernel.dk>:
> On 1/19/18 10:36 AM, weiping zhang wrote:
>> 2018-01-11 0:36 GMT+08:00 weiping zhang <zhangweiping@didichuxing.com>:
>>> bdi debugfs dir/file may create fail, add error log here.
>>>
>>> Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
>>> ---
>>> V1->V2:
>>> fix indentation and make log message more clear
>>>
>>>  mm/backing-dev.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
>>> index b5f940c..0a49665 100644
>>> --- a/mm/backing-dev.c
>>> +++ b/mm/backing-dev.c
>>> @@ -885,7 +885,9 @@ int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args)
>>>         cgwb_bdi_register(bdi);
>>>         bdi->dev = dev;
>>>
>>> -       bdi_debug_register(bdi, dev_name(dev));
>>> +       if (bdi_debug_register(bdi, dev_name(dev)))
>>> +               pr_warn("blkdev %s: creation of bdi debugfs entries failed.\n",
>>> +                       dev_name(dev));
>>>         set_bit(WB_registered, &bdi->wb.state);
>>>
>>>         spin_lock_bh(&bdi_lock);
>>> --
>>
>> Hi Jens,
>>
>> madam has no permission to create debuts entry if SELINUX is enable at
>> Fedora and Centos,
>> and we have revert 6d0e4827b72 Revert "bdi: add error handle for
>> bdi_debug_register", that is to say
>> bdi debugfs is not the key component of block device, this patch just
>> add warning log.
>
> Have we fixed the case where we know it will trigger?

The reason is that mdadm has no permission to create dir/file under
/sys/kernel/debug/
, I think we can solve it in two possible ways.

1.Add proper SELINUX policy to allow mdadm create
/sys/kernel/debug/bdi/xxx, but not
every user add this allowance, so kernel show a warning for this case.

2.Split mdadm into 2 part, Firstly, user proccess mdadm trigger a
kwork and wait a event
done, secondly kwork will create gendisk)and then wake up event. But
it is more likely a
hack, and it may break SELINUX mechanism, so I give up this way.
https://marc.info/?l=linux-mm&m=151456540928231&w=2

> --
> Jens Axboe
>