[Ocfs2-devel] [PATCH 1/1] OCFS2: Log -EIO errors just when hit them.

[Ocfs2-devel] [PATCH 1/1] OCFS2: Log -EIO errors just when hit them.

[wengang wang]

This patch logs(ERROR) -EIO errors just when they are hitted.

Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
--
diff -up ./linux-2.6.29.y.build/fs/ocfs2/buffer_head_io.c.orig ./linux-2.6.29.y.build/fs/ocfs2/buffer_head_io.c
--- ./linux-2.6.29.y.build/fs/ocfs2/buffer_head_io.c.orig	2009-04-17 13:55:52.000000000 +0800
+++ ./linux-2.6.29.y.build/fs/ocfs2/buffer_head_io.c	2009-04-17 14:19:54.000000000 +0800
@@ -91,6 +91,8 @@ int ocfs2_write_block(struct ocfs2_super
 		 * information for this bh as it's not marked locally
 		 * uptodate. */
 		ret = -EIO;
+		mlog(ML_ERROR, "writing block %llu failed with %d\n",
+		     (u64)bh->b_blocknr, ret);
 		put_bh(bh);
 	}
 
@@ -168,6 +170,8 @@ int ocfs2_read_blocks_sync(struct ocfs2_
 			 * so we can safely record this and loop back
 			 * to cleanup the other buffers. */
 			status = -EIO;
+			mlog(ML_ERROR, "reading block %llu failed with %d\n",
+			     (u64)bh->b_blocknr, status);
 			put_bh(bh);
 			bhs[i - 1] = NULL;
 		}
@@ -340,6 +344,9 @@ int ocfs2_read_blocks(struct inode *inod
 				 * for this bh as it's not marked locally
 				 * uptodate. */
 				status = -EIO;
+				mlog(ML_ERROR, "reading block %llu failed with"
+				     " %d\n",
+				     (u64)bh->b_blocknr, status);
 				put_bh(bh);
 				bhs[i] = NULL;
 				continue;
@@ -431,6 +438,8 @@ int ocfs2_write_super_or_backup(struct o
 
 	if (!buffer_uptodate(bh)) {
 		ret = -EIO;
+		mlog(ML_ERROR, "writing block %llu failed with %d\n",
+		     (u64)bh->b_blocknr, ret);
 		put_bh(bh);
 	}
 

[Ocfs2-devel] [PATCH 1/1] OCFS2: Log -EIO errors just when hit them.

[Sunil Mushran]

I imagine this is for the unlogged EIOs that have been reported.

 From my scan, at least in mainline, all these EIOs are being logged
by the caller. So this patch is not adding any value. Can you double
check that please?

Now it could be that the reported EIOs are on 1.2/1.4 and that those
trees are missing the mlogs. In that case, the patch should be specific
to the tree.

BTW, the (u64) should be (unsigned long long). This ensures that
it compiles warning free on all arches.

Sunil


wengang wang wrote:
> This patch logs(ERROR) -EIO errors just when they are hitted.
>
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
> --
> diff -up ./linux-2.6.29.y.build/fs/ocfs2/buffer_head_io.c.orig ./linux-2.6.29.y.build/fs/ocfs2/buffer_head_io.c
> --- ./linux-2.6.29.y.build/fs/ocfs2/buffer_head_io.c.orig	2009-04-17 13:55:52.000000000 +0800
> +++ ./linux-2.6.29.y.build/fs/ocfs2/buffer_head_io.c	2009-04-17 14:19:54.000000000 +0800
> @@ -91,6 +91,8 @@ int ocfs2_write_block(struct ocfs2_super
>  		 * information for this bh as it's not marked locally
>  		 * uptodate. */
>  		ret = -EIO;
> +		mlog(ML_ERROR, "writing block %llu failed with %d\n",
> +		     (u64)bh->b_blocknr, ret);
>  		put_bh(bh);
>  	}
>  
> @@ -168,6 +170,8 @@ int ocfs2_read_blocks_sync(struct ocfs2_
>  			 * so we can safely record this and loop back
>  			 * to cleanup the other buffers. */
>  			status = -EIO;
> +			mlog(ML_ERROR, "reading block %llu failed with %d\n",
> +			     (u64)bh->b_blocknr, status);
>  			put_bh(bh);
>  			bhs[i - 1] = NULL;
>  		}
> @@ -340,6 +344,9 @@ int ocfs2_read_blocks(struct inode *inod
>  				 * for this bh as it's not marked locally
>  				 * uptodate. */
>  				status = -EIO;
> +				mlog(ML_ERROR, "reading block %llu failed with"
> +				     " %d\n",
> +				     (u64)bh->b_blocknr, status);
>  				put_bh(bh);
>  				bhs[i] = NULL;
>  				continue;
> @@ -431,6 +438,8 @@ int ocfs2_write_super_or_backup(struct o
>  
>  	if (!buffer_uptodate(bh)) {
>  		ret = -EIO;
> +		mlog(ML_ERROR, "writing block %llu failed with %d\n",
> +		     (u64)bh->b_blocknr, ret);
>  		put_bh(bh);
>  	}
>   

[Ocfs2-devel] [PATCH 1/1] OCFS2: Log -EIO errors just when hit them.

[Tao Ma]

Hi wengang,
	I just went through the thread. It looks that Sunil said that "From my 
scan, at least in mainline, all these EIOs are being logged by the 
caller. So this patch is not adding any value. "

I guess his meaning is that since all the callers has already logged the 
-EIO error, your patch does't add any value for it. So you may go in the 
wrong direction.

Regards,
Tao

Wengang Wang wrote:
> Hi,
> 
> I noticed ocfs2_end_buffer_io_sync() is only for 1.2
> need to do something for 1.4
> 
> regards,
> wengang.
> 
> Wengang Wang wrote:
>> Hi Sunil and Joel,
>>
>> For the EIO log problem, ocfs2_end_buffer_io_sync() is used as the 
>> callback function b_end_io
>> for both READ and WRITE.
>> I noticed that in this function,
>>
>> if (!uptodate)
>>     mlog_errno(-EIO);
>>
>> Isn't the 2 lines enough to log EIO errors?
>>
>> regards,
>> wengang.
>>
>> Sunil Mushran wrote:
>>   
>>> I imagine this is for the unlogged EIOs that have been reported.
>>>
>>> From my scan, at least in mainline, all these EIOs are being logged
>>> by the caller. So this patch is not adding any value. Can you double
>>> check that please?
>>>
>>> Now it could be that the reported EIOs are on 1.2/1.4 and that those
>>> trees are missing the mlogs. In that case, the patch should be specific
>>> to the tree.
>>>
>>> BTW, the (u64) should be (unsigned long long). This ensures that
>>> it compiles warning free on all arches.
>>>
>>>     
>>
>> _______________________________________________
>> Ocfs2-devel mailing list
>> Ocfs2-devel at oss.oracle.com
>> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
>>   
> 
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel

[Ocfs2-devel] [PATCH 1/1] OCFS2: Log -EIO errors just when hit them.

[Wengang Wang]

Hi Sunil and Joel,

For the EIO log problem, ocfs2_end_buffer_io_sync() is used as the 
callback function b_end_io
for both READ and WRITE.
I noticed that in this function,

if (!uptodate)
    mlog_errno(-EIO);

Isn't the 2 lines enough to log EIO errors?

regards,
wengang.

Sunil Mushran wrote:
> I imagine this is for the unlogged EIOs that have been reported.
>
> From my scan, at least in mainline, all these EIOs are being logged
> by the caller. So this patch is not adding any value. Can you double
> check that please?
>
> Now it could be that the reported EIOs are on 1.2/1.4 and that those
> trees are missing the mlogs. In that case, the patch should be specific
> to the tree.
>
> BTW, the (u64) should be (unsigned long long). This ensures that
> it compiles warning free on all arches.
>

[Ocfs2-devel] [PATCH 1/1] OCFS2: Log -EIO errors just when hit them.

[Wengang Wang]

Hi,

I noticed ocfs2_end_buffer_io_sync() is only for 1.2
need to do something for 1.4

regards,
wengang.

Wengang Wang wrote:
> Hi Sunil and Joel,
>
> For the EIO log problem, ocfs2_end_buffer_io_sync() is used as the 
> callback function b_end_io
> for both READ and WRITE.
> I noticed that in this function,
>
> if (!uptodate)
>     mlog_errno(-EIO);
>
> Isn't the 2 lines enough to log EIO errors?
>
> regards,
> wengang.
>
> Sunil Mushran wrote:
>   
>> I imagine this is for the unlogged EIOs that have been reported.
>>
>> From my scan, at least in mainline, all these EIOs are being logged
>> by the caller. So this patch is not adding any value. Can you double
>> check that please?
>>
>> Now it could be that the reported EIOs are on 1.2/1.4 and that those
>> trees are missing the mlogs. In that case, the patch should be specific
>> to the tree.
>>
>> BTW, the (u64) should be (unsigned long long). This ensures that
>> it compiles warning free on all arches.
>>
>>     
>
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
>   

[Ocfs2-devel] [PATCH 1/1] OCFS2: Log -EIO errors just when hit them.

[Wengang Wang]

Hi Tao,

Maybe my post is misleading :)
First thing is that I know the callers has the log. why I post the patch 
is to log more detailed info such block number which callers don't.
Second, my replies are based on 1.2 and 1.4, not mainline. --I didn't 
clarify it, sorry :)

Sunil and Joel,
That bug report is against 1.2.  I think ocfs2_end_buffer_io_sync() is 
able to log the EIOs.
By later bug update, looks like the IO is READA, but 
ocfs2_end_buffer_io_sync() can't dealwith readahead(low lever didn't 
call it or didn't call it with correct param)?

thanks,
wengang.

Tao Ma wrote:
> Hi wengang,
>     I just went through the thread. It looks that Sunil said that 
> "From my scan, at least in mainline, all these EIOs are being logged 
> by the caller. So this patch is not adding any value. "
>
> I guess his meaning is that since all the callers has already logged 
> the -EIO error, your patch does't add any value for it. So you may go 
> in the wrong direction.
>
> Regards,
> Tao
>
> Wengang Wang wrote:
>> Hi,
>>
>> I noticed ocfs2_end_buffer_io_sync() is only for 1.2
>> need to do something for 1.4
>>
>> regards,
>> wengang.
>>
>> Wengang Wang wrote:
>>> Hi Sunil and Joel,
>>>
>>> For the EIO log problem, ocfs2_end_buffer_io_sync() is used as the 
>>> callback function b_end_io
>>> for both READ and WRITE.
>>> I noticed that in this function,
>>>
>>> if (!uptodate)
>>>     mlog_errno(-EIO);
>>>
>>> Isn't the 2 lines enough to log EIO errors?
>>>
>>> regards,
>>> wengang.
>>>
>>> Sunil Mushran wrote:
>>>  
>>>> I imagine this is for the unlogged EIOs that have been reported.
>>>>
>>>> From my scan, at least in mainline, all these EIOs are being logged
>>>> by the caller. So this patch is not adding any value. Can you double
>>>> check that please?
>>>>
>>>> Now it could be that the reported EIOs are on 1.2/1.4 and that those
>>>> trees are missing the mlogs. In that case, the patch should be 
>>>> specific
>>>> to the tree.
>>>>
>>>> BTW, the (u64) should be (unsigned long long). This ensures that
>>>> it compiles warning free on all arches.
>>>>
>>>>     
>>>
>>> _______________________________________________
>>> Ocfs2-devel mailing list
>>> Ocfs2-devel at oss.oracle.com
>>> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
>>>   
>>
>>
>> _______________________________________________
>> Ocfs2-devel mailing list
>> Ocfs2-devel at oss.oracle.com
>> http://oss.oracle.com/mailman/listinfo/ocfs2-devel