* Can this happen ?
@ 2005-04-27 23:38 Badari Pulavarty
2005-04-27 23:53 ` [PATCH] drop_buffers() shouldn't de-ref page->mapping if its NULL Badari Pulavarty
2005-04-28 0:05 ` Can this happen ? Andrew Morton
0 siblings, 2 replies; 6+ messages in thread
From: Badari Pulavarty @ 2005-04-27 23:38 UTC (permalink / raw)
To: linux-mm, linux-fsdevel; +Cc: Andrew Morton, skodati
Hi Andrew,
We ran into a panic in drop_buffers() while running some networking
tests and I am wondering if this a valid case. try_to_free_buffers()
seems to call drop_buffers() even if the mapping is NULL. drop_buffers()
seems to de-ref the mapping. This is causing NULL pointer deref.
But, is "mapping == NULL" still valid case here ? Can we be in the
code to drop buffers and have mapping NULL ? We would be in this
code only if PagePrivate() is set. Can we have page private with
out a valid mapping ?
Thanks,
Badari
int try_to_free_buffers(struct page *page)
{
struct address_space * const mapping = page->mapping;
....
if (mapping == NULL) { /* can this still happen? */
ret = drop_buffers(page, &buffers_to_free);
goto out;
}
}
drop_buffers(struct page *page, struct buffer_head **buffers_to_free)
{
....
if (buffer_write_io_error(bh))
set_bit(AS_EIO, &page->mapping->flags); <<<<<<
...
}
1:mon> e
cpu 0x1: Vector: 300 (Data Access) at [c00000007ff4b620]
pc: c0000000000bd524: .drop_buffers+0x40/0xcc
lr: c0000000000bd614: .try_to_free_buffers+0x64/0xf4
sp: c00000007ff4b8a0
msr: 8000000000009032
dar: 60
dsisr: 40000000
current = 0xc00000000fe7e040
paca = 0xc0000000003da800
pid = 40, comm = kswapd1
1:mon> t
[c00000007ff4b920] c0000000000bd614 .try_to_free_buffers+0x64/0xf4
[c00000007ff4b9c0] c0000000000baadc .try_to_release_page+0x88/0x9c
[c00000007ff4ba40] c000000000099418 .shrink_list+0x3a0/0x608
[c00000007ff4bb90] c000000000099a04 .shrink_cache+0x384/0x610
[c00000007ff4bcd0] c00000000009a4d4 .shrink_zone+0x104/0x140
[c00000007ff4bd70] c00000000009aaf0 .balance_pgdat+0x270/0x448
[c00000007ff4be90] c00000000009ade4 .kswapd+0x11c/0x120
[c00000007ff4bf90] c000000000018ad0 .kernel_thread+0x4c/0x6c
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] drop_buffers() shouldn't de-ref page->mapping if its NULL
2005-04-27 23:38 Can this happen ? Badari Pulavarty
@ 2005-04-27 23:53 ` Badari Pulavarty
2005-04-28 3:46 ` OGAWA Hirofumi
2005-04-28 0:05 ` Can this happen ? Andrew Morton
1 sibling, 1 reply; 6+ messages in thread
From: Badari Pulavarty @ 2005-04-27 23:53 UTC (permalink / raw)
To: linux-mm, Linux Kernel Mailing List; +Cc: linux-fsdevel, Andrew Morton, skodati
[-- Attachment #1: Type: text/plain, Size: 2599 bytes --]
Hi,
I answered my own question. It looks like we could have pages
with buffers without page->mapping. In such cases, we shouldn't
de-ref page->mapping in drop_buffers(). Here is the trivial
patch to fix it.
Thanks,
Badari
On Wed, 2005-04-27 at 16:38, Badari Pulavarty wrote:
> Hi Andrew,
>
> We ran into a panic in drop_buffers() while running some networking
> tests and I am wondering if this a valid case. try_to_free_buffers()
> seems to call drop_buffers() even if the mapping is NULL. drop_buffers()
> seems to de-ref the mapping. This is causing NULL pointer deref.
>
> But, is "mapping == NULL" still valid case here ? Can we be in the
> code to drop buffers and have mapping NULL ? We would be in this
> code only if PagePrivate() is set. Can we have page private with
> out a valid mapping ?
>
> Thanks,
> Badari
>
> int try_to_free_buffers(struct page *page)
> {
> struct address_space * const mapping = page->mapping;
> ....
>
> if (mapping == NULL) { /* can this still happen? */
> ret = drop_buffers(page, &buffers_to_free);
> goto out;
> }
> }
>
> drop_buffers(struct page *page, struct buffer_head **buffers_to_free)
> {
> ....
> if (buffer_write_io_error(bh))
> set_bit(AS_EIO, &page->mapping->flags); <<<<<<
> ...
> }
>
> 1:mon> e
> cpu 0x1: Vector: 300 (Data Access) at [c00000007ff4b620]
> pc: c0000000000bd524: .drop_buffers+0x40/0xcc
> lr: c0000000000bd614: .try_to_free_buffers+0x64/0xf4
> sp: c00000007ff4b8a0
> msr: 8000000000009032
> dar: 60
> dsisr: 40000000
> current = 0xc00000000fe7e040
> paca = 0xc0000000003da800
> pid = 40, comm = kswapd1
>
> 1:mon> t
> [c00000007ff4b920] c0000000000bd614 .try_to_free_buffers+0x64/0xf4
> [c00000007ff4b9c0] c0000000000baadc .try_to_release_page+0x88/0x9c
> [c00000007ff4ba40] c000000000099418 .shrink_list+0x3a0/0x608
> [c00000007ff4bb90] c000000000099a04 .shrink_cache+0x384/0x610
> [c00000007ff4bcd0] c00000000009a4d4 .shrink_zone+0x104/0x140
> [c00000007ff4bd70] c00000000009aaf0 .balance_pgdat+0x270/0x448
> [c00000007ff4be90] c00000000009ade4 .kswapd+0x11c/0x120
> [c00000007ff4bf90] c000000000018ad0 .kernel_thread+0x4c/0x6c
>
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
[-- Attachment #2: drop_buffer_fix.patch --]
[-- Type: text/plain, Size: 450 bytes --]
Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
--- linux-2.6.12-rc2.org/fs/buffer.c 2005-04-27 07:19:44.000000000 -0700
+++ linux-2.6.12-rc2/fs/buffer.c 2005-04-27 07:20:34.000000000 -0700
@@ -2917,7 +2917,7 @@ drop_buffers(struct page *page, struct b
bh = head;
do {
- if (buffer_write_io_error(bh))
+ if (buffer_write_io_error(bh) && page->mapping)
set_bit(AS_EIO, &page->mapping->flags);
if (buffer_busy(bh))
goto failed;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drop_buffers() shouldn't de-ref page->mapping if its NULL
2005-04-27 23:53 ` [PATCH] drop_buffers() shouldn't de-ref page->mapping if its NULL Badari Pulavarty
@ 2005-04-28 3:46 ` OGAWA Hirofumi
2005-04-28 15:12 ` Badari Pulavarty
0 siblings, 1 reply; 6+ messages in thread
From: OGAWA Hirofumi @ 2005-04-28 3:46 UTC (permalink / raw)
To: Badari Pulavarty
Cc: linux-mm, Linux Kernel Mailing List, linux-fsdevel,
Andrew Morton, skodati
Badari Pulavarty <pbadari@us.ibm.com> writes:
> Hi,
>
> I answered my own question. It looks like we could have pages
> with buffers without page->mapping. In such cases, we shouldn't
> de-ref page->mapping in drop_buffers(). Here is the trivial
> patch to fix it.
>
> Thanks,
> Badari
[...]
>
> Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
> --- linux-2.6.12-rc2.org/fs/buffer.c 2005-04-27 07:19:44.000000000 -0700
> +++ linux-2.6.12-rc2/fs/buffer.c 2005-04-27 07:20:34.000000000 -0700
> @@ -2917,7 +2917,7 @@ drop_buffers(struct page *page, struct b
>
> bh = head;
> do {
> - if (buffer_write_io_error(bh))
> + if (buffer_write_io_error(bh) && page->mapping)
> set_bit(AS_EIO, &page->mapping->flags);
> if (buffer_busy(bh))
> goto failed;
On my experience, this happened the bh leak case only.
If you are not sure whether this is valid state or not, I worry this
patch hides real bug. How about adding the warning, not just remove
de-ref?
Thanks.
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drop_buffers() shouldn't de-ref page->mapping if its NULL
2005-04-28 3:46 ` OGAWA Hirofumi
@ 2005-04-28 15:12 ` Badari Pulavarty
2005-04-28 16:26 ` OGAWA Hirofumi
0 siblings, 1 reply; 6+ messages in thread
From: Badari Pulavarty @ 2005-04-28 15:12 UTC (permalink / raw)
To: OGAWA Hirofumi
Cc: linux-mm, Linux Kernel Mailing List, linux-fsdevel,
Andrew Morton, skodati
On Wed, 2005-04-27 at 20:46, OGAWA Hirofumi wrote:
> Badari Pulavarty <pbadari@us.ibm.com> writes:
>
> > Hi,
> >
> > I answered my own question. It looks like we could have pages
> > with buffers without page->mapping. In such cases, we shouldn't
> > de-ref page->mapping in drop_buffers(). Here is the trivial
> > patch to fix it.
> >
> > Thanks,
> > Badari
>
> [...]
>
> >
> > Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
> > --- linux-2.6.12-rc2.org/fs/buffer.c 2005-04-27 07:19:44.000000000 -0700
> > +++ linux-2.6.12-rc2/fs/buffer.c 2005-04-27 07:20:34.000000000 -0700
> > @@ -2917,7 +2917,7 @@ drop_buffers(struct page *page, struct b
> >
> > bh = head;
> > do {
> > - if (buffer_write_io_error(bh))
> > + if (buffer_write_io_error(bh) && page->mapping)
> > set_bit(AS_EIO, &page->mapping->flags);
> > if (buffer_busy(bh))
> > goto failed;
>
> On my experience, this happened the bh leak case only.
Could you explain more on bh leak ? Is there one in the current code ?
>
> If you are not sure whether this is valid state or not, I worry this
> patch hides real bug. How about adding the warning, not just remove
> de-ref?
Andrew confirmed that this is a valid case.
I don't understand what you want to do here ? If the mapping is NULL,
we can't de-ref it. Whats the point in putting a warning and de-refing
it. Its going to cause NULL pointer de-ref anyway.
Thanks,
Badari
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drop_buffers() shouldn't de-ref page->mapping if its NULL
2005-04-28 15:12 ` Badari Pulavarty
@ 2005-04-28 16:26 ` OGAWA Hirofumi
0 siblings, 0 replies; 6+ messages in thread
From: OGAWA Hirofumi @ 2005-04-28 16:26 UTC (permalink / raw)
To: Badari Pulavarty
Cc: linux-mm, Linux Kernel Mailing List, linux-fsdevel,
Andrew Morton, skodati
Badari Pulavarty <pbadari@us.ibm.com> writes:
> Andrew confirmed that this is a valid case.
>
> I don't understand what you want to do here ? If the mapping is NULL,
> we can't de-ref it. Whats the point in putting a warning and de-refing
> it. Its going to cause NULL pointer de-ref anyway.
I meant your patch + warning. If it is just bh leak, not valid state,
I thought we can notice the leak of bh by warning.
I wanted above things. If it's valid state, of course warning is just
crap.
Sorry for noise.
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Can this happen ?
2005-04-27 23:38 Can this happen ? Badari Pulavarty
2005-04-27 23:53 ` [PATCH] drop_buffers() shouldn't de-ref page->mapping if its NULL Badari Pulavarty
@ 2005-04-28 0:05 ` Andrew Morton
1 sibling, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2005-04-28 0:05 UTC (permalink / raw)
To: Badari Pulavarty; +Cc: linux-mm, linux-fsdevel, skodati
Badari Pulavarty <pbadari@us.ibm.com> wrote:
>
> We ran into a panic in drop_buffers()
erk.
In rare situations, drop_buffers() can be called for a page which has buffers,
but no ->mapping (it was truncated, but the buffers were left behind because
ext3 was still fiddling with them).
But if there was an I/O error in a buffer_head, drop_buffers() will try to get
at the address_space and will oops.
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
fs/buffer.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -puN fs/buffer.c~drop-buffers-oops-fix fs/buffer.c
--- 25/fs/buffer.c~drop-buffers-oops-fix Wed Apr 27 17:02:02 2005
+++ 25-akpm/fs/buffer.c Wed Apr 27 17:02:44 2005
@@ -2924,7 +2924,7 @@ drop_buffers(struct page *page, struct b
bh = head;
do {
- if (buffer_write_io_error(bh))
+ if (buffer_write_io_error(bh) && page->mapping)
set_bit(AS_EIO, &page->mapping->flags);
if (buffer_busy(bh))
goto failed;
_
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2005-04-28 16:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-04-27 23:38 Can this happen ? Badari Pulavarty
2005-04-27 23:53 ` [PATCH] drop_buffers() shouldn't de-ref page->mapping if its NULL Badari Pulavarty
2005-04-28 3:46 ` OGAWA Hirofumi
2005-04-28 15:12 ` Badari Pulavarty
2005-04-28 16:26 ` OGAWA Hirofumi
2005-04-28 0:05 ` Can this happen ? Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox