* [patch] xfsprogs: unhandled error check in libxfs_trans_read_buf
@ 2011-02-03 6:17 Ajeet Yadav
2011-02-07 1:08 ` Ajeet Yadav
2011-02-08 1:17 ` Alex Elder
0 siblings, 2 replies; 5+ messages in thread
From: Ajeet Yadav @ 2011-02-03 6:17 UTC (permalink / raw)
To: xfs, Eric Sandeen
xfsprogs: unhandled error check in libxfs_trans_read_buf
libxfs_trans_read_buf() is used in both mkfs.xfs & xfs_repair.
During stability testing we found some time occur pagefault in mkfs.xfs,
code inspection shows that if libxfs_readbuf() fails then occurs a
page fault in xfs_buf_item_init() called in libxfs_trans_read_buf().
mkfs.xfs: unhandled page fault (11) at 0x00000070, code 0x017
Added NULL check and errno handling.
Signed-off-by: Ajeet Yadav <ajeet.yadav.77@gmail.com>
diff -Nurp xfsprogs/libxfs/rdwr.c xfsprogs-dirty/libxfs/rdwr.c
--- xfsprogs/libxfs/rdwr.c 2011-01-28 20:22:11.000000000 +0900
+++ xfsprogs-dirty/libxfs/rdwr.c 2011-02-02 16:59:16.000000000 +0900
@@ -207,10 +207,11 @@ libxfs_trace_readbuf(const char *func, c
{
xfs_buf_t *bp = libxfs_readbuf(dev, blkno, len, flags);
- bp->b_func = func;
- bp->b_file = file;
- bp->b_line = line;
-
+ if (bp){
+ bp->b_func = func;
+ bp->b_file = file;
+ bp->b_line = line;
+ }
return bp;
}
@@ -485,6 +486,7 @@ libxfs_readbuf(dev_t dev, xfs_daddr_t bl
error = libxfs_readbufr(dev, blkno, bp, len, flags);
if (error) {
libxfs_putbuf(bp);
+ errno = error;
return NULL;
}
}
diff -Nurp xfsprogs/libxfs/trans.c xfsprogs-dirty/libxfs/trans.c
--- xfsprogs/libxfs/trans.c 2011-01-28 20:22:11.000000000 +0900
+++ xfsprogs-dirty/libxfs/trans.c 2011-02-02 17:00:42.000000000 +0900
@@ -508,6 +508,10 @@ libxfs_trans_read_buf(
}
bp = libxfs_readbuf(dev, blkno, len, flags);
+ if (!bp){
+ *bpp = NULL;
+ return errno;
+ }
#ifdef XACT_DEBUG
fprintf(stderr, "trans_read_buf buffer %p, transaction %p\n", bp, tp);
#endif
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch] xfsprogs: unhandled error check in libxfs_trans_read_buf
2011-02-03 6:17 [patch] xfsprogs: unhandled error check in libxfs_trans_read_buf Ajeet Yadav
@ 2011-02-07 1:08 ` Ajeet Yadav
2011-02-08 1:17 ` Alex Elder
1 sibling, 0 replies; 5+ messages in thread
From: Ajeet Yadav @ 2011-02-07 1:08 UTC (permalink / raw)
To: xfs
Dear Community,
Our testing team reported one issue, we found it was related to libxfs
and resolved it.
I thought to share my experience with you, but did not receive any comment.
On Thu, Feb 3, 2011 at 3:17 PM, Ajeet Yadav <ajeet.yadav.77@gmail.com> wrote:
> xfsprogs: unhandled error check in libxfs_trans_read_buf
>
> libxfs_trans_read_buf() is used in both mkfs.xfs & xfs_repair.
> During stability testing we found some time occur pagefault in mkfs.xfs,
> code inspection shows that if libxfs_readbuf() fails then occurs a
> page fault in xfs_buf_item_init() called in libxfs_trans_read_buf().
>
> mkfs.xfs: unhandled page fault (11) at 0x00000070, code 0x017
>
> Added NULL check and errno handling.
>
> Signed-off-by: Ajeet Yadav <ajeet.yadav.77@gmail.com>
>
> diff -Nurp xfsprogs/libxfs/rdwr.c xfsprogs-dirty/libxfs/rdwr.c
> --- xfsprogs/libxfs/rdwr.c 2011-01-28 20:22:11.000000000 +0900
> +++ xfsprogs-dirty/libxfs/rdwr.c 2011-02-02 16:59:16.000000000 +0900
> @@ -207,10 +207,11 @@ libxfs_trace_readbuf(const char *func, c
> {
> xfs_buf_t *bp = libxfs_readbuf(dev, blkno, len, flags);
>
> - bp->b_func = func;
> - bp->b_file = file;
> - bp->b_line = line;
> -
> + if (bp){
> + bp->b_func = func;
> + bp->b_file = file;
> + bp->b_line = line;
> + }
> return bp;
> }
>
> @@ -485,6 +486,7 @@ libxfs_readbuf(dev_t dev, xfs_daddr_t bl
> error = libxfs_readbufr(dev, blkno, bp, len, flags);
> if (error) {
> libxfs_putbuf(bp);
> + errno = error;
> return NULL;
> }
> }
> diff -Nurp xfsprogs/libxfs/trans.c xfsprogs-dirty/libxfs/trans.c
> --- xfsprogs/libxfs/trans.c 2011-01-28 20:22:11.000000000 +0900
> +++ xfsprogs-dirty/libxfs/trans.c 2011-02-02 17:00:42.000000000 +0900
> @@ -508,6 +508,10 @@ libxfs_trans_read_buf(
> }
>
> bp = libxfs_readbuf(dev, blkno, len, flags);
> + if (!bp){
> + *bpp = NULL;
> + return errno;
> + }
> #ifdef XACT_DEBUG
> fprintf(stderr, "trans_read_buf buffer %p, transaction %p\n", bp, tp);
> #endif
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch] xfsprogs: unhandled error check in libxfs_trans_read_buf
2011-02-03 6:17 [patch] xfsprogs: unhandled error check in libxfs_trans_read_buf Ajeet Yadav
2011-02-07 1:08 ` Ajeet Yadav
@ 2011-02-08 1:17 ` Alex Elder
2011-02-08 2:22 ` Ajeet Yadav
1 sibling, 1 reply; 5+ messages in thread
From: Alex Elder @ 2011-02-08 1:17 UTC (permalink / raw)
To: Ajeet Yadav; +Cc: Eric Sandeen, xfs
On Thu, 2011-02-03 at 15:17 +0900, Ajeet Yadav wrote:
> xfsprogs: unhandled error check in libxfs_trans_read_buf
Sorry you didn't get a response earlier. Reporting
problems like this--especially if they come with a
proposed fix--is always appreciated.
> libxfs_trans_read_buf() is used in both mkfs.xfs & xfs_repair.
> During stability testing we found some time occur pagefault in mkfs.xfs,
> code inspection shows that if libxfs_readbuf() fails then occurs a
> page fault in xfs_buf_item_init() called in libxfs_trans_read_buf().
>
> mkfs.xfs: unhandled page fault (11) at 0x00000070, code 0x017
>
> Added NULL check and errno handling.
I expect there are other similar problems lurking in
the libxfs code.
I think your change looks good, with one exception,
noted below. I will gladly adjust your patch for
you if you consent to the change I suggest.
-Alex
> Signed-off-by: Ajeet Yadav <ajeet.yadav.77@gmail.com>
>
> diff -Nurp xfsprogs/libxfs/rdwr.c xfsprogs-dirty/libxfs/rdwr.c
> --- xfsprogs/libxfs/rdwr.c 2011-01-28 20:22:11.000000000 +0900
> +++ xfsprogs-dirty/libxfs/rdwr.c 2011-02-02 16:59:16.000000000 +0900
> @@ -207,10 +207,11 @@ libxfs_trace_readbuf(const char *func, c
> {
> xfs_buf_t *bp = libxfs_readbuf(dev, blkno, len, flags);
>
> - bp->b_func = func;
> - bp->b_file = file;
> - bp->b_line = line;
> -
> + if (bp){
> + bp->b_func = func;
> + bp->b_file = file;
> + bp->b_line = line;
> + }
> return bp;
> }
>
> @@ -485,6 +486,7 @@ libxfs_readbuf(dev_t dev, xfs_daddr_t bl
> error = libxfs_readbufr(dev, blkno, bp, len, flags);
> if (error) {
> libxfs_putbuf(bp);
> + errno = error;
I think that libxfs_readbuf() simply returns the
value of the special global "errno" if there is
an error. And I believe that at this point it
will not have been changed, so there's no need
to assign it here.
In other words, I'd like to remove this one piece
of your patch.
> return NULL;
> }
> }
> diff -Nurp xfsprogs/libxfs/trans.c xfsprogs-dirty/libxfs/trans.c
> --- xfsprogs/libxfs/trans.c 2011-01-28 20:22:11.000000000 +0900
> +++ xfsprogs-dirty/libxfs/trans.c 2011-02-02 17:00:42.000000000 +0900
> @@ -508,6 +508,10 @@ libxfs_trans_read_buf(
> }
>
> bp = libxfs_readbuf(dev, blkno, len, flags);
> + if (!bp){
> + *bpp = NULL;
> + return errno;
> + }
> #ifdef XACT_DEBUG
> fprintf(stderr, "trans_read_buf buffer %p, transaction %p\n", bp, tp);
> #endif
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch] xfsprogs: unhandled error check in libxfs_trans_read_buf
2011-02-08 1:17 ` Alex Elder
@ 2011-02-08 2:22 ` Ajeet Yadav
2011-02-08 2:22 ` Ajeet Yadav
0 siblings, 1 reply; 5+ messages in thread
From: Ajeet Yadav @ 2011-02-08 2:22 UTC (permalink / raw)
To: aelder; +Cc: Eric Sandeen, xfs
Thank you for providing review comment.
On Tue, Feb 8, 2011 at 10:17 AM, Alex Elder <aelder@sgi.com> wrote:
> On Thu, 2011-02-03 at 15:17 +0900, Ajeet Yadav wrote:
>> xfsprogs: unhandled error check in libxfs_trans_read_buf
>
> Sorry you didn't get a response earlier. Reporting
> problems like this--especially if they come with a
> proposed fix--is always appreciated.
>
>> libxfs_trans_read_buf() is used in both mkfs.xfs & xfs_repair.
>> During stability testing we found some time occur pagefault in mkfs.xfs,
>> code inspection shows that if libxfs_readbuf() fails then occurs a
>> page fault in xfs_buf_item_init() called in libxfs_trans_read_buf().
>>
>> mkfs.xfs: unhandled page fault (11) at 0x00000070, code 0x017
>>
>> Added NULL check and errno handling.
>
> I expect there are other similar problems lurking in
> the libxfs code.
>
> I think your change looks good, with one exception,
> noted below. I will gladly adjust your patch for
> you if you consent to the change I suggest.
>
> -Alex
>
>> Signed-off-by: Ajeet Yadav <ajeet.yadav.77@gmail.com>
>>
>> diff -Nurp xfsprogs/libxfs/rdwr.c xfsprogs-dirty/libxfs/rdwr.c
>> --- xfsprogs/libxfs/rdwr.c 2011-01-28 20:22:11.000000000 +0900
>> +++ xfsprogs-dirty/libxfs/rdwr.c 2011-02-02 16:59:16.000000000 +0900
>> @@ -207,10 +207,11 @@ libxfs_trace_readbuf(const char *func, c
>> {
>> xfs_buf_t *bp = libxfs_readbuf(dev, blkno, len, flags);
>>
>> - bp->b_func = func;
>> - bp->b_file = file;
>> - bp->b_line = line;
>> -
>> + if (bp){
>> + bp->b_func = func;
>> + bp->b_file = file;
>> + bp->b_line = line;
>> + }
>> return bp;
>> }
>>
>> @@ -485,6 +486,7 @@ libxfs_readbuf(dev_t dev, xfs_daddr_t bl
>> error = libxfs_readbufr(dev, blkno, bp, len, flags);
>> if (error) {
>> libxfs_putbuf(bp);
>> + errno = error;
>
> I think that libxfs_readbuf() simply returns the
> value of the special global "errno" if there is
> an error. And I believe that at this point it
> will not have been changed, so there's no need
> to assign it here.
>
> In other words, I'd like to remove this one piece
> of your patch.
>
>> return NULL;
>> }
>> }
>> diff -Nurp xfsprogs/libxfs/trans.c xfsprogs-dirty/libxfs/trans.c
>> --- xfsprogs/libxfs/trans.c 2011-01-28 20:22:11.000000000 +0900
>> +++ xfsprogs-dirty/libxfs/trans.c 2011-02-02 17:00:42.000000000 +0900
>> @@ -508,6 +508,10 @@ libxfs_trans_read_buf(
>> }
>>
>> bp = libxfs_readbuf(dev, blkno, len, flags);
>> + if (!bp){
>> + *bpp = NULL;
>> + return errno;
>> + }
>> #ifdef XACT_DEBUG
>> fprintf(stderr, "trans_read_buf buffer %p, transaction %p\n", bp, tp);
>> #endif
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@oss.sgi.com
>> http://oss.sgi.com/mailman/listinfo/xfs
>
>
>
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch] xfsprogs: unhandled error check in libxfs_trans_read_buf
2011-02-08 2:22 ` Ajeet Yadav
@ 2011-02-08 2:22 ` Ajeet Yadav
0 siblings, 0 replies; 5+ messages in thread
From: Ajeet Yadav @ 2011-02-08 2:22 UTC (permalink / raw)
To: aelder; +Cc: Eric Sandeen, xfs
xfsprogs: unhandled error check in libxfs_trans_read_buf
libxfs_trans_read_buf() is used in both mkfs.xfs & xfs_repair.
During stability testing we found some time occur pagefault in mkfs.xfs,
code inspection shows that if libxfs_readbuf() fails then occurs a
page fault in xfs_buf_item_init() called in libxfs_trans_read_buf().
mkfs.xfs: unhandled page fault (11) at 0x00000070, code 0x017
Added NULL check and errno handling.
diff -Nurp xfsprogs/libxfs/rdwr.c xfsprogs-dirty/libxfs/rdwr.c
--- xfsprogs/libxfs/rdwr.c 2011-01-28 20:22:11.000000000 +0900
+++ xfsprogs-dirty/libxfs/rdwr.c 2011-02-02 16:59:16.000000000 +0900
@@ -207,10 +207,11 @@ libxfs_trace_readbuf(const char *func, c
{
xfs_buf_t *bp = libxfs_readbuf(dev, blkno, len, flags);
- bp->b_func = func;
- bp->b_file = file;
- bp->b_line = line;
-
+ if (bp){
+ bp->b_func = func;
+ bp->b_file = file;
+ bp->b_line = line;
+ }
return bp;
}
diff -Nurp xfsprogs/libxfs/trans.c xfsprogs-dirty/libxfs/trans.c
--- xfsprogs/libxfs/trans.c 2011-01-28 20:22:11.000000000 +0900
+++ xfsprogs-dirty/libxfs/trans.c 2011-02-02 17:00:42.000000000 +0900
@@ -508,6 +508,10 @@ libxfs_trans_read_buf(
}
bp = libxfs_readbuf(dev, blkno, len, flags);
+ if (!bp){
+ *bpp = NULL;
+ return errno;
+ }
#ifdef XACT_DEBUG
fprintf(stderr, "trans_read_buf buffer %p, transaction %p\n", bp, tp);
#endif
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-02-08 2:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-03 6:17 [patch] xfsprogs: unhandled error check in libxfs_trans_read_buf Ajeet Yadav
2011-02-07 1:08 ` Ajeet Yadav
2011-02-08 1:17 ` Alex Elder
2011-02-08 2:22 ` Ajeet Yadav
2011-02-08 2:22 ` Ajeet Yadav
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.