Hello,
Here are two fixes as incrementals to the xattr/acl patches:
fix-xattr.diff: The bad_block bug Andreas Dilger has reported
fix-acl.diff: Make change in ext[23]_new_inode() less intrusive.
On Tue, Oct 15, 2002 at 04:40:15PM +0200, Andreas Gruenbacher wrote:
> Hello,
>
> Here are two fixes as incrementals to the xattr/acl patches:
>
> fix-xattr.diff: The bad_block bug Andreas Dilger has reported
I've fixed this already in my patches, thanks.
> fix-acl.diff: Make change in ext[23]_new_inode() less intrusive.
Uh, I must be missing something.
It looks like the ext3 change in fix-acl.diff was to revert a change
that I never had; it's not in the 2.4 0.8.50 patches, and it wasn't in
my patches. So I'm not sure what's going on there.
The ext2 change in fix-acl.diff looks *wrong*. It removes a call to
mark_inode_dirty which was there in the original, and which is
necessary.
Are you sure you sent me the right diff, or that you diffed the
correct set of trees/files?
- Ted
On Tuesday 15 October 2002 20:29, Theodore Ts'o wrote:
> It looks like the ext3 change in fix-acl.diff was to revert a change
> that I never had; it's not in the 2.4 0.8.50 patches, and it wasn't in
> my patches. So I'm not sure what's going on there.
Utter confusion has arisen. Can you put the current versions of your patches
at a well known location (web/ftp/cvs), so we can more easily check and patch
against them? That would be great.
The fix-acl patch is on top of either 0.8.50 or 8.0.51. The x_init_acl() dirty
the inode themselves (at least they should). Initially the x_dirty_inode()
calls had been moved below the x_init_acl() calls, but this is no longer
necessary, and so the patch moved them up again.
BUG: I have overlooked the dummy implementation of ext[23]_init_acl(). Please
find attached a corrected version.
> The ext2 change in fix-acl.diff looks *wrong*. It removes a call to
> mark_inode_dirty which was there in the original, and which is
> necessary.
The original ext2_new_inode with no xattr/acl patches calls mark_inode_dirty
before unlock_super. This call is not removed in 0.8.50 or 0.8.51, but a
second call is added below ext2_init_acl. Since ext2_init_acl takes care of
dirtying the inode itself this second call is no longer needed (I hope!)
--Andreas.
On Oct 15, 2002 23:00 +0200, Andreas Gruenbacher wrote:
> The original ext2_new_inode with no xattr/acl patches calls mark_inode_dirty
> before unlock_super. This call is not removed in 0.8.50 or 0.8.51, but a
> second call is added below ext2_init_acl. Since ext2_init_acl takes care of
> dirtying the inode itself this second call is no longer needed (I hope!)
Just as an FYI - marking ext3 inodes dirty is an expensive operation,
and should be done only once if at all possible (not sure if the same
code applies to ext3 as you are discussing ext2, but I thought I should
mention it).
Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/
On Wednesday 16 October 2002 00:01, Andreas Dilger wrote:
> On Oct 15, 2002 23:00 +0200, Andreas Gruenbacher wrote:
> > The original ext2_new_inode with no xattr/acl patches calls
> > mark_inode_dirty before unlock_super. This call is not removed in 0.8.50
> > or 0.8.51, but a second call is added below ext2_init_acl. Since
> > ext2_init_acl takes care of dirtying the inode itself this second call is
> > no longer needed (I hope!)
>
> Just as an FYI - marking ext3 inodes dirty is an expensive operation,
> and should be done only once if at all possible (not sure if the same
> code applies to ext3 as you are discussing ext2, but I thought I should
> mention it).
Then I should really think out something to improve that. Thanks.
--Andreas.
On Wednesday 16 October 2002 00:09, Andreas Gruenbacher wrote:
> > Just as an FYI - marking ext3 inodes dirty is an expensive operation,
> > and should be done only once if at all possible (not sure if the same
> > code applies to ext3 as you are discussing ext2, but I thought I should
> > mention it).
>
> Then I should really think out something to improve that. Thanks.
Can I be sure that it's safe to:
- move mark_inode_dirty() below unlock_super() in ext2
- move ext3_mark_inode_dirty() below unlock_super() ext3
in ext[23]_new_inode()?
Thanks.
On Tue, 2002-10-15 at 23:01, Andreas Dilger wrote:
> Just as an FYI - marking ext3 inodes dirty is an expensive operation,
> and should be done only once if at all possible (not sure if the same
> code applies to ext3 as you are discussing ext2, but I thought I should
> mention it).
Yes its noticable that 2.4 makes ext3 inodes dirty unneccessarily in
some cases. Its one of the things Pete Zaitsev of mysql pointed out to
me.