2006-05-14 08:15:05

by Evgeniy Dushistov

[permalink] [raw]
Subject: [PATCH 1/3] ufs: ufs_trunc_indirect: infinite cycle

Currently, ufs write support have two sets of problems:
work with files and work with directories.

This series of patches should solve the first problem.

This patch similar to
http://lkml.org/lkml/2006/1/17/61
this patch complements it.

The situation the same: in ufs_trunc_(not direct),
we read block,
check if count of links to it is equal to one,
if so we finish cycle, if not continue.
Because of "count of links" always >=2 this operation cause
infinite cycle and hang up the kernel.

Signed-off-by: Evgeniy Dushistov <[email protected]>

---

diff -upr -X linux-2.6.17-rc4/Documentation/dontdiff linux-2.6.17-rc4-vanilla/fs/ufs/truncate.c linux-2.6.17-rc4/fs/ufs/truncate.c
--- linux-2.6.17-rc4-vanilla/fs/ufs/truncate.c 2006-05-13 23:50:53.000000000 +0400
+++ linux-2.6.17-rc4/fs/ufs/truncate.c 2006-05-14 00:21:03.696891750 +0400
@@ -237,19 +237,14 @@ static int ufs_trunc_indirect (struct in
for (i = 0; i < uspi->s_apb; i++)
if (*ubh_get_addr32(ind_ubh,i))
break;
- if (i >= uspi->s_apb) {
- if (ubh_max_bcount(ind_ubh) != 1) {
- retry = 1;
- }
- else {
- tmp = fs32_to_cpu(sb, *p);
- *p = 0;
- inode->i_blocks -= uspi->s_nspb;
- mark_inode_dirty(inode);
- ufs_free_blocks (inode, tmp, uspi->s_fpb);
- ubh_bforget(ind_ubh);
- ind_ubh = NULL;
- }
+ if (i >= uspi->s_apb) {
+ tmp = fs32_to_cpu(sb, *p);
+ *p = 0;
+ inode->i_blocks -= uspi->s_nspb;
+ mark_inode_dirty(inode);
+ ufs_free_blocks (inode, tmp, uspi->s_fpb);
+ ubh_bforget(ind_ubh);
+ ind_ubh = NULL;
}
if (IS_SYNC(inode) && ind_ubh && ubh_buffer_dirty(ind_ubh)) {
ubh_ll_rw_block (SWRITE, 1, &ind_ubh);
@@ -305,18 +300,14 @@ static int ufs_trunc_dindirect (struct i
for (i = 0; i < uspi->s_apb; i++)
if (*ubh_get_addr32 (dind_bh, i))
break;
- if (i >= uspi->s_apb) {
- if (ubh_max_bcount(dind_bh) != 1)
- retry = 1;
- else {
- tmp = fs32_to_cpu(sb, *p);
- *p = 0;
- inode->i_blocks -= uspi->s_nspb;
- mark_inode_dirty(inode);
- ufs_free_blocks (inode, tmp, uspi->s_fpb);
- ubh_bforget(dind_bh);
- dind_bh = NULL;
- }
+ if (i >= uspi->s_apb) {
+ tmp = fs32_to_cpu(sb, *p);
+ *p = 0;
+ inode->i_blocks -= uspi->s_nspb;
+ mark_inode_dirty(inode);
+ ufs_free_blocks (inode, tmp, uspi->s_fpb);
+ ubh_bforget(dind_bh);
+ dind_bh = NULL;
}
if (IS_SYNC(inode) && dind_bh && ubh_buffer_dirty(dind_bh)) {
ubh_ll_rw_block (SWRITE, 1, &dind_bh);
@@ -369,18 +360,14 @@ static int ufs_trunc_tindirect (struct i
for (i = 0; i < uspi->s_apb; i++)
if (*ubh_get_addr32 (tind_bh, i))
break;
- if (i >= uspi->s_apb) {
- if (ubh_max_bcount(tind_bh) != 1)
- retry = 1;
- else {
- tmp = fs32_to_cpu(sb, *p);
- *p = 0;
- inode->i_blocks -= uspi->s_nspb;
- mark_inode_dirty(inode);
- ufs_free_blocks (inode, tmp, uspi->s_fpb);
- ubh_bforget(tind_bh);
- tind_bh = NULL;
- }
+ if (i >= uspi->s_apb) {
+ tmp = fs32_to_cpu(sb, *p);
+ *p = 0;
+ inode->i_blocks -= uspi->s_nspb;
+ mark_inode_dirty(inode);
+ ufs_free_blocks (inode, tmp, uspi->s_fpb);
+ ubh_bforget(tind_bh);
+ tind_bh = NULL;
}
if (IS_SYNC(inode) && tind_bh && ubh_buffer_dirty(tind_bh)) {
ubh_ll_rw_block (SWRITE, 1, &tind_bh);
diff -upr -X linux-2.6.17-rc4/Documentation/dontdiff linux-2.6.17-rc4-vanilla/fs/ufs/util.c linux-2.6.17-rc4/fs/ufs/util.c
--- linux-2.6.17-rc4-vanilla/fs/ufs/util.c 2006-05-13 23:50:53.000000000 +0400
+++ linux-2.6.17-rc4/fs/ufs/util.c 2006-05-14 00:19:07.757646000 +0400
@@ -139,18 +139,6 @@ void ubh_wait_on_buffer (struct ufs_buff
wait_on_buffer (ubh->bh[i]);
}

-unsigned ubh_max_bcount (struct ufs_buffer_head * ubh)
-{
- unsigned i;
- unsigned max = 0;
- if (!ubh)
- return 0;
- for ( i = 0; i < ubh->count; i++ )
- if ( atomic_read(&ubh->bh[i]->b_count) > max )
- max = atomic_read(&ubh->bh[i]->b_count);
- return max;
-}
-
void ubh_bforget (struct ufs_buffer_head * ubh)
{
unsigned i;
diff -upr -X linux-2.6.17-rc4/Documentation/dontdiff linux-2.6.17-rc4-vanilla/fs/ufs/util.h linux-2.6.17-rc4/fs/ufs/util.h
--- linux-2.6.17-rc4-vanilla/fs/ufs/util.h 2006-05-13 23:50:53.000000000 +0400
+++ linux-2.6.17-rc4/fs/ufs/util.h 2006-05-14 00:19:14.186047750 +0400
@@ -238,7 +238,6 @@ extern void ubh_mark_buffer_dirty (struc
extern void ubh_mark_buffer_uptodate (struct ufs_buffer_head *, int);
extern void ubh_ll_rw_block (int, unsigned, struct ufs_buffer_head **);
extern void ubh_wait_on_buffer (struct ufs_buffer_head *);
-extern unsigned ubh_max_bcount (struct ufs_buffer_head *);
extern void ubh_bforget (struct ufs_buffer_head *);
extern int ubh_buffer_dirty (struct ufs_buffer_head *);
#define ubh_ubhcpymem(mem,ubh,size) _ubh_ubhcpymem_(uspi,mem,ubh,size)

--
/Evgeniy


2006-05-14 09:51:09

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 1/3] ufs: ufs_trunc_indirect: infinite cycle

Evgeniy Dushistov <[email protected]> wrote:
>
> The situation the same: in ufs_trunc_(not direct),
> we read block,
> check if count of links to it is equal to one,
> if so we finish cycle, if not continue.
> Because of "count of links" always >=2 this operation cause
> infinite cycle and hang up the kernel.

okay, but do we know what that code which you removed was actually intended
to do?

Do you know whether 2.4 kernels exhibit the same bug? If not, we might have
accidentally broken UFS at some stage.

2006-05-14 11:44:47

by Evgeniy Dushistov

[permalink] [raw]
Subject: Re: [PATCH 1/3] ufs: ufs_trunc_indirect: infinite cycle

On Sun, May 14, 2006 at 02:47:52AM -0700, Andrew Morton wrote:
> Evgeniy Dushistov <[email protected]> wrote:
> >
> > The situation the same: in ufs_trunc_(not direct),
> > we read block,
> > check if count of links to it is equal to one,
> > if so we finish cycle, if not continue.
> > Because of "count of links" always >=2 this operation cause
> > infinite cycle and hang up the kernel.
>
> okay, but do we know what that code which you removed was actually intended
> to do?
>
I suppose it wait untill all range of buffers, which used by
"inode" will be freed. "vmtruncate" should care about
this.

> Do you know whether 2.4 kernels exhibit the same bug?
I suppose 2.4 doesn't affected, because of "sb_get_hash_table"
(analog of sb_find_get_block in 2.6) use "get_bh" only once,
so b_count equal to 1 if nobody used it.


--
/Evgeniy