[PATCH 1/3] ufs: ufs_trunc_indirect: infinite cycle

[PATCH 1/3] ufs: ufs_trunc_indirect: infinite cycle

[Evgeniy Dushistov]

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 <dushistov@mail.ru>

---

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

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

[Andrew Morton]

Evgeniy Dushistov <dushistov@mail.ru> 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.

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

[Evgeniy Dushistov]

On Sun, May 14, 2006 at 02:47:52AM -0700, Andrew Morton wrote:
> Evgeniy Dushistov <dushistov@mail.ru> 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