| Message ID | 1531123108-45918-1-git-send-email-junil0814.lee@lge.com |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | ext4: remove abnormal set for I_DATA_SEM subclass | expand |
On Mon, Jul 09, 2018 at 04:58:28PM +0900, Junil Lee wrote: > The -EBUSY return value of dquot_enable() function means that just > want to update flags. If some users make a duplicate request to update > flags, lockdep could catch the false positive casued by needing to > allocate a quota block from inside ext4_map_blocks(), while holding > i_data_sem for a data inode. This results in this complaint: > > CPU0 CPU1 > ---- ---- > lock(&s->s_dquot.dqio_mutex); > lock(&ei->i_data_sem); > lock(&s->s_dquot.dqio_mutex); > lock(&ei->i_data_sem); How does this happen in practice? The function ext4_quota_enable() is only called by ext4_enable_quotas(), and I don't see the code path where this would happen. And if it does it would be resulting an EXT4-fs warning message getting printing indicating that a failure to enable quotas with an error of EBUSY. So how does this happen that "users would make a duplicate request to update flags"? - Ted
On Mon, Jul 23, 2018 at 10:48:37AM +0900, 이준일/연구원/MC연구소 BSP실 BSP6팀(junil0814.lee@lge.com) wrote: > Then, I have a question. > quotactl() doesn't have case only to set limits flag, the routine to set > the DQUOT_LIMITS_ENABLED flag is under dquot_enable() function. > According to this logic, if users makes duplicate request to set > DQUOT_LIMITS_ENABLED flags, can lockdep make the false alarm with ext4 ? With the upstream kernel, if you call quotactl with the Q_QUOTAON command, it will fail with EEXIST before it ever gets to the file system specific quota code. This happens in dquot_quota_enable() in fs/quota/dquot.c. I'm going to guess you didn't try to reproduce the problem with the latest mainstream kernel, and then applied the patch, and verified the problem went away before you submitted it? Regards, - Ted
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 13d2706..0757c9a 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -5637,7 +5637,7 @@ static int ext4_quota_enable(struct super_block *sb, int type, int format_id, lockdep_set_quota_inode(qf_inode, I_DATA_SEM_QUOTA); err = dquot_enable(qf_inode, type, format_id, flags); iput(qf_inode); - if (err) + if (err && err != -EBUSY) lockdep_set_quota_inode(qf_inode, I_DATA_SEM_NORMAL); return err;
The -EBUSY return value of dquot_enable() function means that just want to update flags. If some users make a duplicate request to update flags, lockdep could catch the false positive casued by needing to allocate a quota block from inside ext4_map_blocks(), while holding i_data_sem for a data inode. This results in this complaint: CPU0 CPU1 ---- ---- lock(&s->s_dquot.dqio_mutex); lock(&ei->i_data_sem); lock(&s->s_dquot.dqio_mutex); lock(&ei->i_data_sem); Signed-off-by: Junil Lee <junil0814.lee@lge.com> --- fs/ext4/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)