All of lore.kernel.org
 help / color / mirror / Atom feed
* Is there any reason for us to use EXT4_MAXQUOTAS_INIT_BLOCKS?
@ 2015-04-15 15:55 Theodore Ts'o
  2015-04-15 20:47 ` Jan Kara
  0 siblings, 1 reply; 3+ messages in thread
From: Theodore Ts'o @ 2015-04-15 15:55 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-ext4

It looks to me like EXT4_MAXQUOTAS_INIT_BLOCKS includes the credits
neede to set up the quota records.  So if we move the call to
dquot_initialize(inode) outside of the normal transaction (which we
should probably do in all cases), that means that we shouldn't ever
need to use EXT4_MAX_QUOTAS_INIT_BLOCKS.  Is that right?

The reason why I ask is the following is a easy way to trigger a file
system problem:

mke2fs -Fq -t ext4 -b 4096 /dev/vdc 50M
mount -t ext4 -o usrquota,grpquota /dev/vdc
l8=12345678
l16=$l8$l8
l32=$l16$l16
l64=$l32$l32
dmesg -n 7
ln -s $l64 /vdc/link

This will result in:

[    5.229165] JBD2: ln wants too many credits (156 > 128)
[    5.230194] EXT4-fs error (device vdc) in __ext4_new_inode:843: error 28

In other places where we are allocating a new inode (such as mknod),
we're doing the following:

	credits = EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
			EXT4_INDEX_EXTRA_TRANS_BLOCKS + 3;

Which is 37 blocks, and I suspect that's still too darned much.  But
if we don't need to use EXT4_MAXQUOTAS_INIT_BLOCKS in ext4_mknod(), we
shouldn't be needing it in ext4_symlink(), either.

Am I missing anything?

Thanks,

					- Ted

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Is there any reason for us to use EXT4_MAXQUOTAS_INIT_BLOCKS?
  2015-04-15 15:55 Is there any reason for us to use EXT4_MAXQUOTAS_INIT_BLOCKS? Theodore Ts'o
@ 2015-04-15 20:47 ` Jan Kara
  2015-04-15 23:27   ` Theodore Ts'o
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara @ 2015-04-15 20:47 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Jan Kara, linux-ext4

On Wed 15-04-15 11:55:37, Ted Tso wrote:
> It looks to me like EXT4_MAXQUOTAS_INIT_BLOCKS includes the credits
> neede to set up the quota records.  So if we move the call to
> dquot_initialize(inode) outside of the normal transaction (which we
> should probably do in all cases), that means that we shouldn't ever
> need to use EXT4_MAX_QUOTAS_INIT_BLOCKS.  Is that right?
  Correct. EXT4_MAX_QUOTAS_INIT_BLOCKS is only needed if we create the
first file for a user. So chown / chgrp may need this and inode creation
may need this. As you say, if dquot_init() is in a separate transaction,
then inode creation has to account only for a standard quota operation.

> The reason why I ask is the following is a easy way to trigger a file
> system problem:
> 
> mke2fs -Fq -t ext4 -b 4096 /dev/vdc 50M
> mount -t ext4 -o usrquota,grpquota /dev/vdc
> l8=12345678
> l16=$l8$l8
> l32=$l16$l16
> l64=$l32$l32
> dmesg -n 7
> ln -s $l64 /vdc/link
> 
> This will result in:
> 
> [    5.229165] JBD2: ln wants too many credits (156 > 128)
> [    5.230194] EXT4-fs error (device vdc) in __ext4_new_inode:843: error 28
> 
> In other places where we are allocating a new inode (such as mknod),
> we're doing the following:
> 
> 	credits = EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
> 			EXT4_INDEX_EXTRA_TRANS_BLOCKS + 3;
> 
> Which is 37 blocks, and I suspect that's still too darned much.  But
> if we don't need to use EXT4_MAXQUOTAS_INIT_BLOCKS in ext4_mknod(), we
> shouldn't be needing it in ext4_symlink(), either.
> 
> Am I missing anything?
  Yeah, the credit estimate in ext4_symlink():
credits = 4 + EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb) + EXT4_XATTR_TRANS_BLOCKS;
  is just too pessimistic. Actually what we need for long symlinks is only
credits for inode creation + addition to orphan list (so 4 +
EXT4_MAXQUOTAS_TRANS_BLOCKS -- sb, gdt, bitmap, inode). Then we stop the
transaction and add block to the symlink in a separate transaction. And
then we link symlink into a directory in yet another transaction.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Is there any reason for us to use EXT4_MAXQUOTAS_INIT_BLOCKS?
  2015-04-15 20:47 ` Jan Kara
@ 2015-04-15 23:27   ` Theodore Ts'o
  0 siblings, 0 replies; 3+ messages in thread
From: Theodore Ts'o @ 2015-04-15 23:27 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-ext4

On Wed, Apr 15, 2015 at 10:47:54PM +0200, Jan Kara wrote:
>   Yeah, the credit estimate in ext4_symlink():
> credits = 4 + EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb) + EXT4_XATTR_TRANS_BLOCKS;
>   is just too pessimistic. Actually what we need for long symlinks is only
> credits for inode creation + addition to orphan list (so 4 +
> EXT4_MAXQUOTAS_TRANS_BLOCKS -- sb, gdt, bitmap, inode).

Thanks, I just wanted to check with you before I fixed this, since we
didn't have anything explicitly documenting what
EXT4_MAXQUOAS_INIT_BLOCKS did (which I'll fix as well while I'm at
it), and I didn't want to make anything assumptions that might come
back and bite us later.  Of course, the fact that ext4_symlinks was
using way more credits than ext4_mknod made it clear something was
buggy, and I couldn't imagine any circumstances why creating a long
symlink (or more correctly, only the first part of creatinga long
symlink) would require modifying 156 metadata blocks!   :-)

	       	      	       	    	       - Ted

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-04-15 23:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-15 15:55 Is there any reason for us to use EXT4_MAXQUOTAS_INIT_BLOCKS? Theodore Ts'o
2015-04-15 20:47 ` Jan Kara
2015-04-15 23:27   ` Theodore Ts'o

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.