Subject: [PATCH] update i_blocks on UFS

UFS code dosn't update i_blocks which leads to the problems ranging from wrong
stat/du to long symlinks (as opposed to inline ones) being damaged. This is
a fix.

Signed-off-by: Vladimir Serbinenko <[email protected]>

diff --git a/fs/ufs/balloc.c b/fs/ufs/balloc.c
index 42694e1..6f7c6ea 100644
--- a/fs/ufs/balloc.c
+++ b/fs/ufs/balloc.c
@@ -34,7 +34,8 @@ static void ufs_clusteracct(struct super_block *, struct ufs_cg_private_info *,
/*
* Free 'count' fragments from fragment number 'fragment'
*/
-void ufs_free_fragments(struct inode *inode, u64 fragment, unsigned count)
+void ufs_free_fragments(struct inode *inode, u64 fragment, unsigned count,
+ int account)
{
struct super_block * sb;
struct ufs_sb_private_info * uspi;
@@ -111,7 +112,10 @@ void ufs_free_fragments(struct inode *inode, u64 fragment, unsigned count)
fs32_add(sb, &ubh_cg_blktot(ucpi, cylno), 1);
}
}
-
+
+ if (account)
+ inode->i_blocks -= (u64) count * (u64) (sb->s_blocksize >> 9);
+
ubh_mark_buffer_dirty (USPI_UBH(uspi));
ubh_mark_buffer_dirty (UCPI_UBH(ucpi));
if (sb->s_flags & MS_SYNCHRONOUS)
@@ -131,7 +135,8 @@ failed:
/*
* Free 'count' fragments from fragment number 'fragment' (free whole blocks)
*/
-void ufs_free_blocks(struct inode *inode, u64 fragment, unsigned count)
+void ufs_free_blocks(struct inode *inode, u64 fragment, unsigned count,
+ int account)
{
struct super_block * sb;
struct ufs_sb_private_info * uspi;
@@ -140,6 +145,7 @@ void ufs_free_blocks(struct inode *inode, u64 fragment, unsigned count)
struct ufs_cylinder_group * ucg;
unsigned overflow, cgno, bit, end_bit, i;
u64 blkno;
+ unsigned free_count = account ? count : 0;

sb = inode->i_sb;
uspi = UFS_SB(sb)->s_uspi;
@@ -214,6 +220,8 @@ do_more:
goto do_more;
}

+ inode->i_blocks -= (u64) free_count * (u64) (sb->s_blocksize >> 9);
+
sb->s_dirt = 1;
unlock_super (sb);
UFSD("EXIT\n");
@@ -343,8 +351,8 @@ static void ufs_clear_frags(struct inode *inode, sector_t beg, unsigned int n,
}

u64 ufs_new_fragments(struct inode *inode, void *p, u64 fragment,
- u64 goal, unsigned count, int *err,
- struct page *locked_page)
+ u64 goal, unsigned count, int *err,
+ struct page *locked_page, int account)
{
struct super_block * sb;
struct ufs_sb_private_info * uspi;
@@ -425,6 +433,10 @@ u64 ufs_new_fragments(struct inode *inode, void *p, u64 fragment,
*err = 0;
UFS_I(inode)->i_lastfrag =
max(UFS_I(inode)->i_lastfrag, fragment + count);
+ if (account)
+ inode->i_blocks += (u64) count
+ * (u64) (sb->s_blocksize >> 9);
+
ufs_clear_frags(inode, result + oldcount,
newcount - oldcount, locked_page != NULL);
}
@@ -441,6 +453,9 @@ u64 ufs_new_fragments(struct inode *inode, void *p, u64 fragment,
*err = 0;
UFS_I(inode)->i_lastfrag = max(UFS_I(inode)->i_lastfrag,
fragment + count);
+ if (account)
+ inode->i_blocks += (u64) count
+ * (u64) (sb->s_blocksize >> 9);
ufs_clear_frags(inode, result + oldcount, newcount - oldcount,
locked_page != NULL);
unlock_super(sb);
@@ -481,10 +496,15 @@ u64 ufs_new_fragments(struct inode *inode, void *p, u64 fragment,
*err = 0;
UFS_I(inode)->i_lastfrag = max(UFS_I(inode)->i_lastfrag,
fragment + count);
+ if (account)
+ inode->i_blocks += (u64) count
+ * (u64) (sb->s_blocksize >> 9);
+
unlock_super(sb);
if (newcount < request)
- ufs_free_fragments (inode, result + newcount, request - newcount);
- ufs_free_fragments (inode, tmp, oldcount);
+ ufs_free_fragments(inode, result + newcount,
+ request - newcount, 0);
+ ufs_free_fragments(inode, tmp, oldcount, 0);
UFSD("EXIT, result %llu\n", (unsigned long long)result);
return result;
}
diff --git a/fs/ufs/inode.c b/fs/ufs/inode.c
index 7cdd395..4d75897 100644
--- a/fs/ufs/inode.c
+++ b/fs/ufs/inode.c
@@ -172,7 +172,8 @@ out:
static struct buffer_head *
ufs_inode_getfrag(struct inode *inode, u64 fragment,
sector_t new_fragment, unsigned int required, int *err,
- long *phys, int *new, struct page *locked_page)
+ long *phys, int *new, struct page *locked_page,
+ int account)
{
struct ufs_inode_info *ufsi = UFS_I(inode);
struct super_block *sb = inode->i_sb;
@@ -231,7 +232,7 @@ repeat:
tmp = ufs_new_fragments(inode, p2, lastfrag,
ufs_data_ptr_to_cpu(sb, p2),
uspi->s_fpb - lastblockoff,
- err, locked_page);
+ err, locked_page, account);
if (!tmp) {
if (lastfrag != ufsi->i_lastfrag)
goto repeat;
@@ -249,7 +250,8 @@ repeat:
tmp = ufs_new_fragments (inode, p, fragment - blockoff,
goal, required + blockoff,
err,
- phys != NULL ? locked_page : NULL);
+ phys != NULL ? locked_page : NULL,
+ account);
} else if (lastblock == block) {
/*
* We will extend last allocated block
@@ -258,7 +260,8 @@ repeat:
(blockoff - lastblockoff),
ufs_data_ptr_to_cpu(sb, p),
required + (blockoff - lastblockoff),
- err, phys != NULL ? locked_page : NULL);
+ err, phys != NULL ? locked_page : NULL,
+ account);
} else /* (lastblock > block) */ {
/*
* We will allocate new block before last allocated block
@@ -271,7 +274,8 @@ repeat:
}
tmp = ufs_new_fragments(inode, p, fragment - blockoff,
goal, uspi->s_fpb, err,
- phys != NULL ? locked_page : NULL);
+ phys != NULL ? locked_page : NULL,
+ account);
}
if (!tmp) {
if ((!blockoff && ufs_data_ptr_to_cpu(sb, p)) ||
@@ -328,8 +332,9 @@ repeat2:
*/
static struct buffer_head *
ufs_inode_getblock(struct inode *inode, struct buffer_head *bh,
- u64 fragment, sector_t new_fragment, int *err,
- long *phys, int *new, struct page *locked_page)
+ u64 fragment, sector_t new_fragment, int *err,
+ long *phys, int *new, struct page *locked_page,
+ int account)
{
struct super_block *sb = inode->i_sb;
struct ufs_sb_private_info *uspi = UFS_SB(sb)->s_uspi;
@@ -380,7 +385,7 @@ repeat:
else
goal = bh->b_blocknr + uspi->s_fpb;
tmp = ufs_new_fragments(inode, p, ufs_blknum(new_fragment), goal,
- uspi->s_fpb, err, locked_page);
+ uspi->s_fpb, err, locked_page, account);
if (!tmp) {
if (ufs_data_ptr_to_cpu(sb, p))
goto repeat;
@@ -456,16 +461,16 @@ int ufs_getfrag_block(struct inode *inode, sector_t fragment, struct buffer_head
*/
#define GET_INODE_DATABLOCK(x) \
ufs_inode_getfrag(inode, x, fragment, 1, &err, &phys, &new,\
- bh_result->b_page)
+ bh_result->b_page, 1)
#define GET_INODE_PTR(x) \
ufs_inode_getfrag(inode, x, fragment, uspi->s_fpb, &err, NULL, NULL,\
- bh_result->b_page)
+ bh_result->b_page, 0)
#define GET_INDIRECT_DATABLOCK(x) \
ufs_inode_getblock(inode, bh, x, fragment, \
- &err, &phys, &new, bh_result->b_page)
+ &err, &phys, &new, bh_result->b_page, 1)
#define GET_INDIRECT_PTR(x) \
ufs_inode_getblock(inode, bh, x, fragment, \
- &err, NULL, NULL, NULL)
+ &err, NULL, NULL, NULL, 0)

if (ptr < UFS_NDIR_FRAGMENT) {
bh = GET_INODE_DATABLOCK(ptr);
diff --git a/fs/ufs/truncate.c b/fs/ufs/truncate.c
index f04f89f..cb9f06c 100644
--- a/fs/ufs/truncate.c
+++ b/fs/ufs/truncate.c
@@ -115,7 +115,8 @@ static int ufs_trunc_direct(struct inode *inode)
frag2 -= frag1;
frag1 = ufs_fragnum (frag1);

- ufs_free_fragments(inode, tmp + frag1, frag2);
+ ufs_free_fragments(inode, tmp + frag1, frag2, 1);
+
mark_inode_dirty(inode);
frag_to_free = tmp + frag1;

@@ -136,7 +137,7 @@ next1:
} else if (free_count > 0 && frag_to_free == tmp - free_count)
free_count += uspi->s_fpb;
else {
- ufs_free_blocks (inode, frag_to_free, free_count);
+ ufs_free_blocks(inode, frag_to_free, free_count, 1);
frag_to_free = tmp;
free_count = uspi->s_fpb;
}
@@ -144,7 +145,7 @@ next1:
}

if (free_count > 0)
- ufs_free_blocks (inode, frag_to_free, free_count);
+ ufs_free_blocks(inode, frag_to_free, free_count, 1);

if (frag3 >= frag4)
goto next3;
@@ -159,7 +160,8 @@ next1:
frag4 = ufs_fragnum (frag4);
ufs_data_ptr_clear(uspi, p);

- ufs_free_fragments (inode, tmp, frag4);
+ ufs_free_fragments(inode, tmp, frag4, 1);
+
mark_inode_dirty(inode);
next3:

@@ -218,7 +220,7 @@ static int ufs_trunc_indirect(struct inode *inode, u64 offset, void *p)
} else if (free_count > 0 && frag_to_free == tmp - free_count)
free_count += uspi->s_fpb;
else {
- ufs_free_blocks (inode, frag_to_free, free_count);
+ ufs_free_blocks(inode, frag_to_free, free_count, 1);
frag_to_free = tmp;
free_count = uspi->s_fpb;
}
@@ -227,7 +229,7 @@ static int ufs_trunc_indirect(struct inode *inode, u64 offset, void *p)
}

if (free_count > 0) {
- ufs_free_blocks (inode, frag_to_free, free_count);
+ ufs_free_blocks(inode, frag_to_free, free_count, 1);
}
for (i = 0; i < uspi->s_apb; i++)
if (!ufs_is_data_ptr_zero(uspi,
@@ -237,7 +239,7 @@ static int ufs_trunc_indirect(struct inode *inode, u64 offset, void *p)
tmp = ufs_data_ptr_to_cpu(sb, p);
ufs_data_ptr_clear(uspi, p);

- ufs_free_blocks (inode, tmp, uspi->s_fpb);
+ ufs_free_blocks(inode, tmp, uspi->s_fpb, 0);
mark_inode_dirty(inode);
ubh_bforget(ind_ubh);
ind_ubh = NULL;
@@ -299,7 +301,7 @@ static int ufs_trunc_dindirect(struct inode *inode, u64 offset, void *p)
tmp = ufs_data_ptr_to_cpu(sb, p);
ufs_data_ptr_clear(uspi, p);

- ufs_free_blocks(inode, tmp, uspi->s_fpb);
+ ufs_free_blocks(inode, tmp, uspi->s_fpb, 0);
mark_inode_dirty(inode);
ubh_bforget(dind_bh);
dind_bh = NULL;
@@ -357,7 +359,7 @@ static int ufs_trunc_tindirect(struct inode *inode)
tmp = ufs_data_ptr_to_cpu(sb, p);
ufs_data_ptr_clear(uspi, p);

- ufs_free_blocks(inode, tmp, uspi->s_fpb);
+ ufs_free_blocks(inode, tmp, uspi->s_fpb, 0);
mark_inode_dirty(inode);
ubh_bforget(tind_bh);
tind_bh = NULL;
diff --git a/fs/ufs/ufs.h b/fs/ufs/ufs.h
index 528750b..88144a0 100644
--- a/fs/ufs/ufs.h
+++ b/fs/ufs/ufs.h
@@ -76,10 +76,10 @@ struct ufs_inode_info {
#endif

/* balloc.c */
-extern void ufs_free_fragments (struct inode *, u64, unsigned);
-extern void ufs_free_blocks (struct inode *, u64, unsigned);
+extern void ufs_free_fragments(struct inode *, u64, unsigned, int);
+extern void ufs_free_blocks(struct inode *, u64, unsigned, int);
extern u64 ufs_new_fragments(struct inode *, void *, u64, u64,
- unsigned, int *, struct page *);
+ unsigned, int *, struct page *, int);

/* cylinder.c */
extern struct ufs_cg_private_info * ufs_load_cylinder (struct super_block *, unsigned);


Attachments:
signature.asc (294.00 B)
OpenPGP digital signature

Subject: Re: [PATCH] update i_blocks on UFS

On 13.05.2012 23:59, Evgeniy Dushistov wrote:

> On Sun, May 13, 2012 at 03:52:46PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> UFS code dosn't update i_blocks which leads to the problems ranging from wrong
>> stat/du to long symlinks (as opposed to inline ones) being damaged. This is
>> a fix.
>>
>
> Yes, when quota support was removed, code that update statistic
> about usage of blocks also was removed. But why so big change to fix this?
> I found this problem some time ago and fixed it with code bellow,
> but may be I missed something?
>

Hm, I don't know why this code wasn't in my copy. Was it already
propagated to Linus' tree? When will it be? Which tree I should merge
from to get your latest fixes?
My code avoided counting metadata blocks (the ones holding the pointers
to actual blocks in indirect, double-indirect and triple-indirect
chains). I thought that i_blocks doesn't take into account the
metablocks but quick experiment with NetBSD image proved me wrong. So I
retract this patch altogether.
My UFS file size limit patch remains though.

--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko


Attachments:
signature.asc (294.00 B)
OpenPGP digital signature

2012-05-13 22:16:42

by Evgeniy Dushistov

[permalink] [raw]
Subject: Re: [PATCH] update i_blocks on UFS

On Sun, May 13, 2012 at 03:52:46PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> UFS code dosn't update i_blocks which leads to the problems ranging from wrong
> stat/du to long symlinks (as opposed to inline ones) being damaged. This is
> a fix.
>

Yes, when quota support was removed, code that update statistic
about usage of blocks also was removed. But why so big change to fix this?
I found this problem some time ago and fixed it with code bellow,
but may be I missed something?

>From 3bba2473d2d09c0ebde5e287d694346ba0e4b4c0 Mon Sep 17 00:00:00 2001
From: Evgeniy Dushistov <[email protected]>
Date: Fri, 18 Mar 2011 06:42:57 +0300
Subject: [PATCH] Looks like with quote support code was removed code
that update statistic about usage blocks per inode.
So fix this.

---
fs/ufs/balloc.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/ufs/balloc.c b/fs/ufs/balloc.c
index 100d2ee..e7bf2fd 100644
--- a/fs/ufs/balloc.c
+++ b/fs/ufs/balloc.c
@@ -83,7 +83,7 @@ void ufs_free_fragments(struct inode *inode, u64 fragment, unsigned count)
ufs_error (sb, "ufs_free_fragments",
"bit already cleared for fragment %u", i);
}
-
+ inode_sub_bytes(inode, count << uspi->s_fshift);
fs32_add(sb, &ucg->cg_cs.cs_nffree, count);
uspi->cs_total.cs_nffree += count;
fs32_add(sb, &UFS_SB(sb)->fs_cs(cgno).cs_nffree, count);
@@ -189,6 +189,7 @@ do_more:
ubh_setblock(UCPI_UBH(ucpi), ucpi->c_freeoff, blkno);
if ((UFS_SB(sb)->s_flags & UFS_CG_MASK) == UFS_CG_44BSD)
ufs_clusteracct (sb, ucpi, blkno, 1);
+ inode_sub_bytes(inode, uspi->s_bsize);

fs32_add(sb, &ucg->cg_cs.cs_nbfree, 1);
uspi->cs_total.cs_nbfree++;
@@ -547,7 +548,7 @@ static u64 ufs_add_fragments(struct inode *inode, u64 fragment,
fs32_add(sb, &ucg->cg_frsum[fragsize - count], 1);
for (i = oldcount; i < newcount; i++)
ubh_clrbit (UCPI_UBH(ucpi), ucpi->c_freeoff, fragno + i);
-
+ inode_add_bytes(inode, count << uspi->s_fshift);
fs32_sub(sb, &ucg->cg_cs.cs_nffree, count);
fs32_sub(sb, &UFS_SB(sb)->fs_cs(cgno).cs_nffree, count);
uspi->cs_total.cs_nffree -= count;
@@ -649,6 +650,7 @@ cg_found:
for (i = count; i < uspi->s_fpb; i++)
ubh_setbit (UCPI_UBH(ucpi), ucpi->c_freeoff, goal + i);
i = uspi->s_fpb - count;
+ inode_sub_bytes(inode, i << uspi->s_fshift);

fs32_add(sb, &ucg->cg_cs.cs_nffree, i);
uspi->cs_total.cs_nffree += i;
@@ -660,6 +662,7 @@ cg_found:
result = ufs_bitmap_search (sb, ucpi, goal, allocsize);
if (result == INVBLOCK)
return 0;
+ inode_add_bytes(inode, count << uspi->s_fshift);
for (i = 0; i < count; i++)
ubh_clrbit (UCPI_UBH(ucpi), ucpi->c_freeoff, result + i);

@@ -725,7 +728,7 @@ gotit:
ubh_clrblock (UCPI_UBH(ucpi), ucpi->c_freeoff, blkno);
if ((UFS_SB(sb)->s_flags & UFS_CG_MASK) == UFS_CG_44BSD)
ufs_clusteracct (sb, ucpi, blkno, -1);
-
+ inode_add_bytes(inode, uspi->s_bsize);
fs32_sub(sb, &ucg->cg_cs.cs_nbfree, 1);
uspi->cs_total.cs_nbfree--;
fs32_sub(sb, &UFS_SB(sb)->fs_cs(ucpi->c_cgx).cs_nbfree, 1);
--
1.7.3.4

--
/Evgeniy

2012-05-14 11:56:59

by Evgeniy Dushistov

[permalink] [raw]
Subject: Re: [PATCH] update i_blocks on UFS

On Mon, May 14, 2012 at 12:16:27AM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 13.05.2012 23:59, Evgeniy Dushistov wrote:
>
> > On Sun, May 13, 2012 at 03:52:46PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> >> UFS code dosn't update i_blocks which leads to the problems ranging from wrong
> >> stat/du to long symlinks (as opposed to inline ones) being damaged. This is
> >> a fix.
> >>
> >
> > Yes, when quota support was removed, code that update statistic
> > about usage of blocks also was removed. But why so big change to fix this?
> > I found this problem some time ago and fixed it with code bellow,
> > but may be I missed something?
> >
>
> Hm, I don't know why this code wasn't in my copy. Was it already
> propagated to Linus' tree? When will it be? Which tree I should merge
> from to get your latest fixes?

I created this patch some time ago, but forgot to merge it to main tree.
You email remind me about this problem.

> My code avoided counting metadata blocks (the ones holding the pointers
> to actual blocks in indirect, double-indirect and triple-indirect
> chains). I thought that i_blocks doesn't take into account the
> metablocks but quick experiment with NetBSD image proved me wrong. So I
> retract this patch altogether.
> My UFS file size limit patch remains though.
>

ok, so I will test your patch and my and then take care about merging
them into mainline.


--
/Evgeniy