linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Revert "f2fs: use printk_ratelimited for f2fs_msg"
@ 2018-08-13  6:28 Chao Yu
  2018-08-13  9:50 ` Joe Perches
  2018-08-14 17:09 ` Jaegeuk Kim
  0 siblings, 2 replies; 9+ messages in thread
From: Chao Yu @ 2018-08-13  6:28 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

Don't limit printing log, so that we will not miss any key messages.

This reverts commit a36c106dffb616250117efb1cab271c19a8f94ff.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index bf4c919fe418..3d89d94191e7 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -209,7 +209,7 @@ void f2fs_msg(struct super_block *sb, const char *level, const char *fmt, ...)
 	va_start(args, fmt);
 	vaf.fmt = fmt;
 	vaf.va = &args;
-	printk_ratelimited("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
+	printk("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
 	va_end(args);
 }
 
-- 
2.18.0.rc1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "f2fs: use printk_ratelimited for f2fs_msg"
  2018-08-13  6:28 [PATCH] Revert "f2fs: use printk_ratelimited for f2fs_msg" Chao Yu
@ 2018-08-13  9:50 ` Joe Perches
  2018-08-14  7:53   ` Chao Yu
  2018-08-14 17:09 ` Jaegeuk Kim
  1 sibling, 1 reply; 9+ messages in thread
From: Joe Perches @ 2018-08-13  9:50 UTC (permalink / raw)
  To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao

On Mon, 2018-08-13 at 14:28 +0800, Chao Yu wrote:
> Don't limit printing log, so that we will not miss any key messages.
> 
> This reverts commit a36c106dffb616250117efb1cab271c19a8f94ff.
[]
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
[]
> @@ -209,7 +209,7 @@ void f2fs_msg(struct super_block *sb, const char *level, const char *fmt, ...)
>  	va_start(args, fmt);
>  	vaf.fmt = fmt;
>  	vaf.va = &args;
> -	printk_ratelimited("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
> +	printk("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
>  	va_end(args);
>  }

If there was value in ratelimiting these messages at all,
perhaps there is value in using a specific ratelimit_state
other than

	static DEFINE_RATELIMIT_STATE(_rs,				\
				      DEFAULT_RATELIMIT_INTERVAL,	\
				      DEFAULT_RATELIMIT_BURST);		\




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "f2fs: use printk_ratelimited for f2fs_msg"
  2018-08-13  9:50 ` Joe Perches
@ 2018-08-14  7:53   ` Chao Yu
  0 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2018-08-14  7:53 UTC (permalink / raw)
  To: Joe Perches, jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao

Hi Joe, thanks for pointing out this, I didn't see any cases need to be limited now.

Thanks,

On 2018/8/13 17:50, Joe Perches wrote:
> On Mon, 2018-08-13 at 14:28 +0800, Chao Yu wrote:
>> Don't limit printing log, so that we will not miss any key messages.
>>
>> This reverts commit a36c106dffb616250117efb1cab271c19a8f94ff.
> []
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> []
>> @@ -209,7 +209,7 @@ void f2fs_msg(struct super_block *sb, const char *level, const char *fmt, ...)
>>  	va_start(args, fmt);
>>  	vaf.fmt = fmt;
>>  	vaf.va = &args;
>> -	printk_ratelimited("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
>> +	printk("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
>>  	va_end(args);
>>  }
> 
> If there was value in ratelimiting these messages at all,
> perhaps there is value in using a specific ratelimit_state
> other than
> 
> 	static DEFINE_RATELIMIT_STATE(_rs,				\
> 				      DEFAULT_RATELIMIT_INTERVAL,	\
> 				      DEFAULT_RATELIMIT_BURST);		\
> 
> 
> 
> 
> .
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "f2fs: use printk_ratelimited for f2fs_msg"
  2018-08-13  6:28 [PATCH] Revert "f2fs: use printk_ratelimited for f2fs_msg" Chao Yu
  2018-08-13  9:50 ` Joe Perches
@ 2018-08-14 17:09 ` Jaegeuk Kim
  2018-08-15  1:19   ` Chao Yu
  1 sibling, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2018-08-14 17:09 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao

On 08/13, Chao Yu wrote:
> Don't limit printing log, so that we will not miss any key messages.

For example, this can avoid lots of messages during roll-forward recovery.

> 
> This reverts commit a36c106dffb616250117efb1cab271c19a8f94ff.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/super.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index bf4c919fe418..3d89d94191e7 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -209,7 +209,7 @@ void f2fs_msg(struct super_block *sb, const char *level, const char *fmt, ...)
>  	va_start(args, fmt);
>  	vaf.fmt = fmt;
>  	vaf.va = &args;
> -	printk_ratelimited("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
> +	printk("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
>  	va_end(args);
>  }
>  
> -- 
> 2.18.0.rc1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "f2fs: use printk_ratelimited for f2fs_msg"
  2018-08-14 17:09 ` Jaegeuk Kim
@ 2018-08-15  1:19   ` Chao Yu
  2018-08-15  3:01     ` Jaegeuk Kim
  0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2018-08-15  1:19 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao

On 2018/8/15 1:09, Jaegeuk Kim wrote:
> On 08/13, Chao Yu wrote:
>> Don't limit printing log, so that we will not miss any key messages.
> 
> For example, this can avoid lots of messages during roll-forward recovery.

That's really important evidence that can prove filesystem didn't lose last
fsynced data, when file/data is missing after SPO, otherwise, filesystem will
still be treated as suspect... :(

Thanks,

> 
>>
>> This reverts commit a36c106dffb616250117efb1cab271c19a8f94ff.
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  fs/f2fs/super.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index bf4c919fe418..3d89d94191e7 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -209,7 +209,7 @@ void f2fs_msg(struct super_block *sb, const char *level, const char *fmt, ...)
>>  	va_start(args, fmt);
>>  	vaf.fmt = fmt;
>>  	vaf.va = &args;
>> -	printk_ratelimited("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
>> +	printk("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
>>  	va_end(args);
>>  }
>>  
>> -- 
>> 2.18.0.rc1
> 
> .
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "f2fs: use printk_ratelimited for f2fs_msg"
  2018-08-15  1:19   ` Chao Yu
@ 2018-08-15  3:01     ` Jaegeuk Kim
  2018-08-15  3:11       ` Chao Yu
  0 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2018-08-15  3:01 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao

On 08/15, Chao Yu wrote:
> On 2018/8/15 1:09, Jaegeuk Kim wrote:
> > On 08/13, Chao Yu wrote:
> >> Don't limit printing log, so that we will not miss any key messages.
> > 
> > For example, this can avoid lots of messages during roll-forward recovery.
> 
> That's really important evidence that can prove filesystem didn't lose last
> fsynced data, when file/data is missing after SPO, otherwise, filesystem will
> still be treated as suspect... :(

It's too bad that we need to rely on kernel messages for that purpose. Anyway,
if so, we may need to use printk() for essential messages only.

> 
> Thanks,
> 
> > 
> >>
> >> This reverts commit a36c106dffb616250117efb1cab271c19a8f94ff.
> >>
> >> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >> ---
> >>  fs/f2fs/super.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> >> index bf4c919fe418..3d89d94191e7 100644
> >> --- a/fs/f2fs/super.c
> >> +++ b/fs/f2fs/super.c
> >> @@ -209,7 +209,7 @@ void f2fs_msg(struct super_block *sb, const char *level, const char *fmt, ...)
> >>  	va_start(args, fmt);
> >>  	vaf.fmt = fmt;
> >>  	vaf.va = &args;
> >> -	printk_ratelimited("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
> >> +	printk("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
> >>  	va_end(args);
> >>  }
> >>  
> >> -- 
> >> 2.18.0.rc1
> > 
> > .
> > 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "f2fs: use printk_ratelimited for f2fs_msg"
  2018-08-15  3:01     ` Jaegeuk Kim
@ 2018-08-15  3:11       ` Chao Yu
  2018-08-15  3:25         ` Jaegeuk Kim
  0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2018-08-15  3:11 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao

On 2018/8/15 11:01, Jaegeuk Kim wrote:
> On 08/15, Chao Yu wrote:
>> On 2018/8/15 1:09, Jaegeuk Kim wrote:
>>> On 08/13, Chao Yu wrote:
>>>> Don't limit printing log, so that we will not miss any key messages.
>>>
>>> For example, this can avoid lots of messages during roll-forward recovery.
>>
>> That's really important evidence that can prove filesystem didn't lose last
>> fsynced data, when file/data is missing after SPO, otherwise, filesystem will
>> still be treated as suspect... :(
> 
> It's too bad that we need to rely on kernel messages for that purpose. Anyway,

Yes, it's very hard to make sure the problem is not caused by filesystem's bug
when trouble shooting, unless there is explicit kernel message.

Also, without message, trouble shooting becomes harder.

> if so, we may need to use printk() for essential messages only.

Now, during our debugging, we have to change to use printk to avoid potential
message missing...

Thanks,

> 
>>
>> Thanks,
>>
>>>
>>>>
>>>> This reverts commit a36c106dffb616250117efb1cab271c19a8f94ff.
>>>>
>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>> ---
>>>>  fs/f2fs/super.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>> index bf4c919fe418..3d89d94191e7 100644
>>>> --- a/fs/f2fs/super.c
>>>> +++ b/fs/f2fs/super.c
>>>> @@ -209,7 +209,7 @@ void f2fs_msg(struct super_block *sb, const char *level, const char *fmt, ...)
>>>>  	va_start(args, fmt);
>>>>  	vaf.fmt = fmt;
>>>>  	vaf.va = &args;
>>>> -	printk_ratelimited("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
>>>> +	printk("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
>>>>  	va_end(args);
>>>>  }
>>>>  
>>>> -- 
>>>> 2.18.0.rc1
>>>
>>> .
>>>
> 
> .
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "f2fs: use printk_ratelimited for f2fs_msg"
  2018-08-15  3:11       ` Chao Yu
@ 2018-08-15  3:25         ` Jaegeuk Kim
  2018-08-15  3:42           ` Chao Yu
  0 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2018-08-15  3:25 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao

On 08/15, Chao Yu wrote:
> On 2018/8/15 11:01, Jaegeuk Kim wrote:
> > On 08/15, Chao Yu wrote:
> >> On 2018/8/15 1:09, Jaegeuk Kim wrote:
> >>> On 08/13, Chao Yu wrote:
> >>>> Don't limit printing log, so that we will not miss any key messages.
> >>>
> >>> For example, this can avoid lots of messages during roll-forward recovery.
> >>
> >> That's really important evidence that can prove filesystem didn't lose last
> >> fsynced data, when file/data is missing after SPO, otherwise, filesystem will
> >> still be treated as suspect... :(
> > 
> > It's too bad that we need to rely on kernel messages for that purpose. Anyway,
> 
> Yes, it's very hard to make sure the problem is not caused by filesystem's bug
> when trouble shooting, unless there is explicit kernel message.
> 
> Also, without message, trouble shooting becomes harder.
> 
> > if so, we may need to use printk() for essential messages only.
> 
> Now, during our debugging, we have to change to use printk to avoid potential
> message missing...

Let me understand what messages are really important for your debugging purpose.
I believe it'd not require all of them.

> 
> Thanks,
> 
> > 
> >>
> >> Thanks,
> >>
> >>>
> >>>>
> >>>> This reverts commit a36c106dffb616250117efb1cab271c19a8f94ff.
> >>>>
> >>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >>>> ---
> >>>>  fs/f2fs/super.c | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> >>>> index bf4c919fe418..3d89d94191e7 100644
> >>>> --- a/fs/f2fs/super.c
> >>>> +++ b/fs/f2fs/super.c
> >>>> @@ -209,7 +209,7 @@ void f2fs_msg(struct super_block *sb, const char *level, const char *fmt, ...)
> >>>>  	va_start(args, fmt);
> >>>>  	vaf.fmt = fmt;
> >>>>  	vaf.va = &args;
> >>>> -	printk_ratelimited("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
> >>>> +	printk("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
> >>>>  	va_end(args);
> >>>>  }
> >>>>  
> >>>> -- 
> >>>> 2.18.0.rc1
> >>>
> >>> .
> >>>
> > 
> > .
> > 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "f2fs: use printk_ratelimited for f2fs_msg"
  2018-08-15  3:25         ` Jaegeuk Kim
@ 2018-08-15  3:42           ` Chao Yu
  0 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2018-08-15  3:42 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao

On 2018/8/15 11:25, Jaegeuk Kim wrote:
> On 08/15, Chao Yu wrote:
>> On 2018/8/15 11:01, Jaegeuk Kim wrote:
>>> On 08/15, Chao Yu wrote:
>>>> On 2018/8/15 1:09, Jaegeuk Kim wrote:
>>>>> On 08/13, Chao Yu wrote:
>>>>>> Don't limit printing log, so that we will not miss any key messages.
>>>>>
>>>>> For example, this can avoid lots of messages during roll-forward recovery.
>>>>
>>>> That's really important evidence that can prove filesystem didn't lose last
>>>> fsynced data, when file/data is missing after SPO, otherwise, filesystem will
>>>> still be treated as suspect... :(
>>>
>>> It's too bad that we need to rely on kernel messages for that purpose. Anyway,
>>
>> Yes, it's very hard to make sure the problem is not caused by filesystem's bug
>> when trouble shooting, unless there is explicit kernel message.
>>
>> Also, without message, trouble shooting becomes harder.
>>
>>> if so, we may need to use printk() for essential messages only.
>>
>> Now, during our debugging, we have to change to use printk to avoid potential
>> message missing...
> 
> Let me understand what messages are really important for your debugging purpose.

The most important message is about filesystem/data consistence.

> I believe it'd not require all of them.

Yeah, IMO, error injection prints too many messages.

Thanks,

> 
>>
>> Thanks,
>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>>>
>>>>>> This reverts commit a36c106dffb616250117efb1cab271c19a8f94ff.
>>>>>>
>>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>>>> ---
>>>>>>  fs/f2fs/super.c | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>>>> index bf4c919fe418..3d89d94191e7 100644
>>>>>> --- a/fs/f2fs/super.c
>>>>>> +++ b/fs/f2fs/super.c
>>>>>> @@ -209,7 +209,7 @@ void f2fs_msg(struct super_block *sb, const char *level, const char *fmt, ...)
>>>>>>  	va_start(args, fmt);
>>>>>>  	vaf.fmt = fmt;
>>>>>>  	vaf.va = &args;
>>>>>> -	printk_ratelimited("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
>>>>>> +	printk("%sF2FS-fs (%s): %pV\n", level, sb->s_id, &vaf);
>>>>>>  	va_end(args);
>>>>>>  }
>>>>>>  
>>>>>> -- 
>>>>>> 2.18.0.rc1
>>>>>
>>>>> .
>>>>>
>>>
>>> .
>>>
> 
> .
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-08-15  3:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-13  6:28 [PATCH] Revert "f2fs: use printk_ratelimited for f2fs_msg" Chao Yu
2018-08-13  9:50 ` Joe Perches
2018-08-14  7:53   ` Chao Yu
2018-08-14 17:09 ` Jaegeuk Kim
2018-08-15  1:19   ` Chao Yu
2018-08-15  3:01     ` Jaegeuk Kim
2018-08-15  3:11       ` Chao Yu
2018-08-15  3:25         ` Jaegeuk Kim
2018-08-15  3:42           ` Chao Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).