[PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

[PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

[Alexey Dobriyan]

OpenBSD doesn't see "." correctly in directories created by Linux.
Copying files over several KB will buy you infinite loop in
__getblk_slow(). Copying files smaller than 1 KB seems to be OK.
Sometimes files will be filled with zeros. Sometimes incorrectly copied
file will reappear after next file with truncated size.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 fs/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -1327,7 +1327,7 @@ config UFS_FS
 
 config UFS_FS_WRITE
 	bool "UFS file system write support (DANGEROUS)"
-	depends on UFS_FS && EXPERIMENTAL
+	depends on UFS_FS && EXPERIMENTAL && BROKEN
 	help
 	  Say Y here if you want to try writing to UFS partitions. This is
 	  experimental, so you should back up your UFS partitions beforehand.

Re: [PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

[Evgeniy Dushistov]

On Wed, Feb 01, 2006 at 02:46:34AM +0300, Alexey Dobriyan wrote:
> OpenBSD doesn't see "." correctly in directories created by Linux.
The problem is in dir.c:ufs_make_empty, which create "." and ".."
entires, in this function i_size isn't updated, 
so result directory has zero size.
This patch should solve the problem, can you try it?

Signed-off-by: Evgeniy Dushistov <dushistov@mail.ru>

---

--- linux-2.6.16-rc1-mm4/fs/ufs/dir.c.orig	2006-02-01 18:29:28.943878250 +0300
+++ linux-2.6.16-rc1-mm4/fs/ufs/dir.c	2006-02-01 18:12:24.043826000 +0300
@@ -539,6 +539,7 @@ int ufs_make_empty(struct inode * inode,
 		return err;
 
 	inode->i_blocks = sb->s_blocksize / UFS_SECTOR_SIZE;
+	inode->i_size = sb->s_blocksize;
 	de = (struct ufs_dir_entry *) dir_block->b_data;
 	de->d_ino = cpu_to_fs32(sb, inode->i_ino);
 	ufs_set_de_type(sb, de, inode->i_mode);


-- 
/Evgeniy

Re [2]: [PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

[Evgeniy Dushistov]

On Wed, Feb 01, 2006 at 02:46:34AM +0300, Alexey Dobriyan wrote:
> Copying files over several KB will buy you infinite loop in
> __getblk_slow(). Copying files smaller than 1 KB seems to be OK.
> Sometimes files will be filled with zeros. Sometimes incorrectly copied
> file will reappear after next file with truncated size.
The problem as can I see, in very strange code in
balloc.c:ufs_new_fragments. b_blocknr is changed without "restraint".

This patch just "workaround", not a clear solution. But it helps me
copy files more than 4K. Can you try it and say is it really help?

Signed-off-by: Evgeniy Dushistov <dushistov@mail.ru>

---

--- linux-2.6.16-rc1-mm4/fs/ufs/balloc.c.orig	2006-02-01 22:55:28.245272250 +0300
+++ linux-2.6.16-rc1-mm4/fs/ufs/balloc.c	2006-02-01 22:47:33.455599750 +0300
@@ -241,7 +241,7 @@ unsigned ufs_new_fragments (struct inode
 	struct super_block * sb;
 	struct ufs_sb_private_info * uspi;
 	struct ufs_super_block_first * usb1;
-	struct buffer_head * bh;
+	struct buffer_head * bh, *bh1;
 	unsigned cgno, oldcount, newcount, tmp, request, i, result;
 	
 	UFSD(("ENTER, ino %lu, fragment %u, goal %u, count %u\n", inode->i_ino, fragment, goal, count))
@@ -359,17 +359,23 @@ unsigned ufs_new_fragments (struct inode
 	if (result) {
 		for (i = 0; i < oldcount; i++) {
 			bh = sb_bread(sb, tmp + i);
-			if(bh)
-			{
-				clear_buffer_dirty(bh);
-				bh->b_blocknr = result + i;
+			bh1 = sb_getblk(sb, result+i);
+			if (bh && bh1)	{
+				memcpy(bh1->b_data, bh->b_data, bh->b_size);
+				
 				mark_buffer_dirty (bh);
-				if (IS_SYNC(inode))
+				mark_buffer_dirty(bh1);
+				if (IS_SYNC(inode)) {
 					sync_dirty_buffer(bh);
+					sync_dirty_buffer(bh1);
+				}
 				brelse (bh);
-			}
-			else
-			{
+				brelse(bh1);
+			} else {
+				if (bh)
+					brelse(bh);
+				if (bh1)
+					brelse(bh1);
 				printk(KERN_ERR "ufs_new_fragments: bread fail\n");
 				unlock_super(sb);
 				return 0;


-- 
/Evgeniy

Re: Re [2]: [PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

[Andrew Morton]

Evgeniy Dushistov <dushistov@mail.ru> wrote:
>
> On Wed, Feb 01, 2006 at 02:46:34AM +0300, Alexey Dobriyan wrote:
> > Copying files over several KB will buy you infinite loop in
> > __getblk_slow(). Copying files smaller than 1 KB seems to be OK.
> > Sometimes files will be filled with zeros. Sometimes incorrectly copied
> > file will reappear after next file with truncated size.
> The problem as can I see, in very strange code in
> balloc.c:ufs_new_fragments. b_blocknr is changed without "restraint".
> 
> This patch just "workaround", not a clear solution. But it helps me
> copy files more than 4K. Can you try it and say is it really help?

Thanks for working on this.  I won't apply these two patches at this stage
as things don't seem to be finalised.  But if you and Alexey could come up
with some final thing which resurrects UFS write support, that'd be great.

Re: Re [2]: [PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

[Alexey Dobriyan]

Thanks, these two patches makes things better but not much better.

1.

        inode->i_blocks = sb->s_blocksize / UFS_SECTOR_SIZE;
+       inode->i_size = sb->s_blocksize;
        de = (struct ufs_dir_entry *) dir_block->b_data;

This creates directories which are 2048 bytes in size. Native ones are
512 bytes.

	inode->i_size = 512;

makes mkdir and rm reliable for me both on linux and OpenBSD.

2. Second patch indeed makes hangs disaapear. However, data corruption
   is still in place:

Try

	for i in $(seq 1 10000); do echo $i; done >10000-linux.txt
	cp 10000-linux.txt >/mnt/openbsd/10000-openbsd.txt

Now corruption structure is following:

00000000  39 0a 33 30 39 30 0a 33  30 39 31 0a 33 30 39 32  |9.3090.3091.3092|
	[snip]
000007f0  33 34 39 36 0a 33 34 39  37 0a 33 34 39 38 0a 33  |3496.3497.3498.3|
00000800  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00004000  36 36 0a 36 33 36 37 0a  36 33 36 38 0a 36 33 36  |66.6367.6368.636|
	[snip]
000047f0  0a 36 37 37 33 0a 36 37  37 34 0a 36 37 37 35 0a  |.6773.6774.6775.|
00004800  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00008000  36 34 33 0a 39 36 34 34  0a 39 36 34 35 0a 39 36  |643.9644.9645.96|
	[snip]
000086f0  39 38 0a 39 39 39 39 0a  31 30 30 30 30 0a 00 00  |98.9999.10000...|
00008700  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
0000bef0  00 00 00 00 00 00 00 00  00 00 00 00 00 00        |..............|
0000befe

There are all zeros in 0800-4000 and 4800-8000 range. 0000-3800 from
original file is dropped (that 9 it's from 3089).

Re: Re [2]: [PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

[Alexey Dobriyan]

On Fri, Feb 03, 2006 at 08:46:13PM +0300, Alexey Dobriyan wrote:
> 2. Second patch indeed makes hangs disaapear. However, data corruption
>    is still in place:
> 
> Try
> 
> 	for i in $(seq 1 10000); do echo $i; done >10000-linux.txt
> 	cp 10000-linux.txt >/mnt/openbsd/10000-openbsd.txt

Aha! Cutoff is 2048 bytes. This and less is fine. 2049 and more is not.

00000000  0a 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000800  00                                                |.|
00000801

[PATCH] ufs: fill i_size at directory creation

[Alexey Dobriyan]

On Fri, Feb 03, 2006 at 08:46:13PM +0300, Alexey Dobriyan wrote:
> Thanks, these two patches makes things better but not much better.
> 
> 1.
> 
>         inode->i_blocks = sb->s_blocksize / UFS_SECTOR_SIZE;
> +       inode->i_size = sb->s_blocksize;
>         de = (struct ufs_dir_entry *) dir_block->b_data;
> 
> This creates directories which are 2048 bytes in size. Native ones are
> 512 bytes.
> 
> 	inode->i_size = 512;
> 
> makes mkdir and rm reliable for me both on linux and OpenBSD.

I take "reliably" back.

	for i in $(seq 1 100); do mkdir $i; done

barfs after 42-nd.

How about this as a first small step?

[PATCH] ufs: fill i_size at directory creation

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>

--- a/fs/ufs/dir.c
+++ b/fs/ufs/dir.c
@@ -539,6 +539,7 @@ int ufs_make_empty(struct inode * inode,
 		return err;
 
 	inode->i_blocks = sb->s_blocksize / UFS_SECTOR_SIZE;
+	inode->i_size = UFS_SB(sb)->s_uspi->s_fsize;
 	de = (struct ufs_dir_entry *) dir_block->b_data;
 	de->d_ino = cpu_to_fs32(sb, inode->i_ino);
 	ufs_set_de_type(sb, de, inode->i_mode);

Re: [PATCH] ufs: fill i_size at directory creation

[Evgeniy Dushistov]

On Sat, Feb 04, 2006 at 04:18:15AM +0300, Alexey Dobriyan wrote:
> How about this as a first small step?
> +	inode->i_size = UFS_SB(sb)->s_uspi->s_fsize;

It looks very strange for me.

During "fill super" we set block size of device to fragment size,
so sb->s_blocksize and UFS_SB(sb)->s_uspi->s_fsize should be the 
same size on your system: 2048, hence question:
what difference between your and my patch?

-- 
/Evgeniy