2006-01-31 23:28:28

by Alexey Dobriyan

[permalink] [raw]
Subject: [PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

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 <[email protected]>
---

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.


2006-02-01 15:52:20

by Evgeniy Dushistov

[permalink] [raw]
Subject: Re: [PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

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 <[email protected]>

---

--- 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

2006-02-01 20:16:13

by Evgeniy Dushistov

[permalink] [raw]
Subject: Re [2]: [PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

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 <[email protected]>

---

--- 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

2006-02-02 23:38:25

by Andrew Morton

[permalink] [raw]
Subject: Re: Re [2]: [PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

Evgeniy Dushistov <[email protected]> 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.

2006-02-03 17:28:13

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: Re [2]: [PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

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).

2006-02-03 22:26:46

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: Re [2]: [PATCH] Mark CONFIG_UFS_FS_WRITE as BROKEN

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

2006-02-04 01:00:10

by Alexey Dobriyan

[permalink] [raw]
Subject: [PATCH] ufs: fill i_size at directory creation

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 <[email protected]>

--- 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);

2006-02-04 06:30:33

by Evgeniy Dushistov

[permalink] [raw]
Subject: Re: [PATCH] ufs: fill i_size at directory creation

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