[Ocfs2-devel] [PATCH 1/1] a fix of logging return value.

[Ocfs2-devel] [PATCH 1/1] a fix of logging return value.

[Wengang Wang]

	in ocfs2_file_aio_write(), log_exit() could don't log the value
which is really returned. this patch fixes it.

Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
---
 fs/ocfs2/file.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 62442e4..a49fa44 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1918,8 +1918,10 @@ out_sems:
 
 	mutex_unlock(&inode->i_mutex);
 
+	if (written)
+		ret = written;
 	mlog_exit(ret);
-	return written ? written : ret;
+	return ret;
 }
 
 static int ocfs2_splice_to_file(struct pipe_inode_info *pipe,
-- 
1.6.2.5

[Ocfs2-devel] [PATCH 1/1] a fix of logging return value.

[Sunil Mushran]

sob

Hi Wengang, functions is aops.c are missing mlogs. Do you have the  
bandwidth to help out?

On Jul 9, 2009, at 10:26 PM, Wengang Wang <wen.gang.wang@oracle.com>  
wrote:

>    in ocfs2_file_aio_write(), log_exit() could don't log the value
> which is really returned. this patch fixes it.
>
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
> ---
> fs/ocfs2/file.c |    4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 62442e4..a49fa44 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1918,8 +1918,10 @@ out_sems:
>
>    mutex_unlock(&inode->i_mutex);
>
> +    if (written)
> +        ret = written;
>    mlog_exit(ret);
> -    return written ? written : ret;
> +    return ret;
> }
>
> static int ocfs2_splice_to_file(struct pipe_inode_info *pipe,
> -- 
> 1.6.2.5
>
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel

[Ocfs2-devel] [PATCH 1/1] a fix of logging return value.

[Wengang Wang]

Hi Sunil,

Ok. I will post patche(s) for it later.

regards,
wengang.

Sunil Mushran wrote:
> sob
> 
> Hi Wengang, functions is aops.c are missing mlogs. Do you have the 
> bandwidth to help out?
> 
> On Jul 9, 2009, at 10:26 PM, Wengang Wang <wen.gang.wang@oracle.com> wrote:
> 
>>    in ocfs2_file_aio_write(), log_exit() could don't log the value
>> which is really returned. this patch fixes it.
>>
>> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
>> ---
>> fs/ocfs2/file.c |    4 +++-
>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
>> index 62442e4..a49fa44 100644
>> --- a/fs/ocfs2/file.c
>> +++ b/fs/ocfs2/file.c
>> @@ -1918,8 +1918,10 @@ out_sems:
>>
>>    mutex_unlock(&inode->i_mutex);
>>
>> +    if (written)
>> +        ret = written;
>>    mlog_exit(ret);
>> -    return written ? written : ret;
>> +    return ret;
>> }
>>
>> static int ocfs2_splice_to_file(struct pipe_inode_info *pipe,
>> -- 
>> 1.6.2.5
>>

[Ocfs2-devel] [PATCH 1/1] a fix of logging return value.

[Wengang Wang]

Sunil,

in the src, I see both mlog(ML_ERROR...)(finally printk(KERN_ERR...)) 
and ocfs2_error()(finally printk(KERN_CRIT...).
could you tell me in what case one should be used?

regards,
wengang.

Wengang Wang wrote:
> Hi Sunil,
> 
> Ok. I will post patche(s) for it later.
> 
> regards,
> wengang.
> 
> Sunil Mushran wrote:
>> sob
>>
>> Hi Wengang, functions is aops.c are missing mlogs. Do you have the 
>> bandwidth to help out?
>>
>> On Jul 9, 2009, at 10:26 PM, Wengang Wang <wen.gang.wang@oracle.com> wrote:
>>
>>>    in ocfs2_file_aio_write(), log_exit() could don't log the value
>>> which is really returned. this patch fixes it.
>>>
>>> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
>>> ---
>>> fs/ocfs2/file.c |    4 +++-
>>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
>>> index 62442e4..a49fa44 100644
>>> --- a/fs/ocfs2/file.c
>>> +++ b/fs/ocfs2/file.c
>>> @@ -1918,8 +1918,10 @@ out_sems:
>>>
>>>    mutex_unlock(&inode->i_mutex);
>>>
>>> +    if (written)
>>> +        ret = written;
>>>    mlog_exit(ret);
>>> -    return written ? written : ret;
>>> +    return ret;
>>> }
>>>
>>> static int ocfs2_splice_to_file(struct pipe_inode_info *pipe,
>>> -- 
>>> 1.6.2.5
>>>
> 
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel

-- 
--just begin to learn, you are never too late...

[Ocfs2-devel] [PATCH 1/1] a fix of logging return value.

[Tao Ma]

Hi wengang,

Wengang Wang wrote:
> Sunil,
> 
> in the src, I see both mlog(ML_ERROR...)(finally printk(KERN_ERR...)) 
> and ocfs2_error()(finally printk(KERN_CRIT...).
> could you tell me in what case one should be used?
mlog(ML_ERROR...) only print errors. But ocfs2_error does other things, 
sometimes make the volume readonly. Please check ocfs2_handle_error for 
details.

Regards,
Tao
> 
> regards,
> wengang.
> 
> Wengang Wang wrote:
>> Hi Sunil,
>>
>> Ok. I will post patche(s) for it later.
>>
>> regards,
>> wengang.
>>
>> Sunil Mushran wrote:
>>> sob
>>>
>>> Hi Wengang, functions is aops.c are missing mlogs. Do you have the 
>>> bandwidth to help out?
>>>
>>> On Jul 9, 2009, at 10:26 PM, Wengang Wang <wen.gang.wang@oracle.com> wrote:
>>>
>>>>    in ocfs2_file_aio_write(), log_exit() could don't log the value
>>>> which is really returned. this patch fixes it.
>>>>
>>>> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
>>>> ---
>>>> fs/ocfs2/file.c |    4 +++-
>>>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
>>>> index 62442e4..a49fa44 100644
>>>> --- a/fs/ocfs2/file.c
>>>> +++ b/fs/ocfs2/file.c
>>>> @@ -1918,8 +1918,10 @@ out_sems:
>>>>
>>>>    mutex_unlock(&inode->i_mutex);
>>>>
>>>> +    if (written)
>>>> +        ret = written;
>>>>    mlog_exit(ret);
>>>> -    return written ? written : ret;
>>>> +    return ret;
>>>> }
>>>>
>>>> static int ocfs2_splice_to_file(struct pipe_inode_info *pipe,
>>>> -- 
>>>> 1.6.2.5
>>>>
>>
>> _______________________________________________
>> Ocfs2-devel mailing list
>> Ocfs2-devel at oss.oracle.com
>> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
> 

[Ocfs2-devel] [PATCH 1/1] a fix of logging return value.

[Sunil Mushran]

Tao Ma wrote:
> Hi wengang,
>
> Wengang Wang wrote:
>> Sunil,
>>
>> in the src, I see both mlog(ML_ERROR...)(finally printk(KERN_ERR...)) 
>> and ocfs2_error()(finally printk(KERN_CRIT...).
>> could you tell me in what case one should be used?
> mlog(ML_ERROR...) only print errors. But ocfs2_error does other 
> things, sometimes make the volume readonly. Please check 
> ocfs2_handle_error for details.
>

Yes. ocfs2_error() is only called when an ondisk structure read reveals
a corrupted block. Depending on the mount option, it either panics or
remounts ro.

printk(KERN_ERR) is only called if we need to print messages before
mlog_sys_init() is called. So you'll see those printks during module
loads.

There are few instances of printk(KERN_CRIT). We call them because mlog
does not handle such messages. We could add it. But there are only
3 instances of this message. So why bother.

The mlogs that you can use are:

mlog_errno() Error if the only information is ret/status.

mlog(ML_ERROR, ...) Error with vararg.

mlog(ML_NOTICE, ...) Notice with vararg. Use very very selectively as NOTICE
is enabled by default.

mlog(0, ...) Like notice but is printed only when the user enables the 
mask bit.
Use liberally.
 
mlog_entry() Entry with varargs. Use carefully. We don't want mlog to 
dereference
null pointers. ;)

mlog_entry_void() Entry with no args. Preferably use mlog(0) somewhere 
in the
middle of the function with usable information.

mlog_exit() All mlog_entry*() should have an exit.
mlog_exit_ptr()
mlog_exit_void()

We use mlog() to narrow down the scope of an error. Say a read is returning
EIO or EINVAL. By enabling tracing selectively, we can get more information.

However, too much tracing is counterproductive because not only is it hard
to wade thru the logs, the printk ring buffer loses a lot of messages. So,
when you add, think whether it will be useful. This is subjective of course.

An easy example would be function ocfs2_unlock_and_free_pages(). It does
what it says. Now we could add an entry and an exit trace. But the exit is
clearly useless because it does not return any information. So a better
solution could be to have a mlog(0, ...).

mlog(0, "num_pages = %d\n", num_pages);

Do not forget the trailing newline.

Thanks for taking on this task.
Sunil

[Ocfs2-devel] [PATCH 1/1] a fix of logging return value.

[Joel Becker]

On Fri, Jul 10, 2009 at 01:26:04PM +0800, Wengang Wang wrote:
> 	in ocfs2_file_aio_write(), log_exit() could don't log the value
> which is really returned. this patch fixes it.
> 
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>

This patch is now in the fixes branch of ocfs2.git

Joel


-- 

Life's Little Instruction Book #207

	"Swing for the fence."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127