ocfs2: alloc orphaned inode in ocfs2_symlink

alloc the inode orphaned in ocfs2_symlink so we can delete the inode in the iput if we meet errors with the inode, e.g. ocfs2_add_inode_data. or ocfs2_query_inode_wipe will complain the inode is not orphaned on disk.

---
 fs/ocfs2/namei.c |   60 +++++++++++++++++++++++++++++------------------------
 1 files changed, 33 insertions(+), 27 deletions(-)

index f010b22..daf1fdf 100644
@@ -94,6 +94,8 @@ static int ocfs2_create_symlink_data(struct ocfs2_super 

*osb,
struct inode *inode, const char *symname);

+static int ocfs2_blkno_stringify(u64 blkno, char *name); +
/* An orphan dir name is an 8 byte value, printed as a hex string */ #define OCFS2_ORPHAN_NAMELEN ((int)(2 * sizeof(u64)))

@@ -1579,6 +1581,7 @@ static int ocfs2_symlink(struct inode *dir,
u64 newsize;
struct ocfs2_super *osb = NULL;
struct inode *inode = NULL;
+ struct inode *orphan_dir = NULL;
struct super_block *sb;
struct buffer_head *new_fe_bh = NULL;
struct buffer_head *parent_fe_bh = NULL;
@@ -1594,7 +1597,9 @@ static int ocfs2_symlink(struct inode *dir,
.enable = 1,
};
int did_quota = 0, did_quota_inode = 0;
+ char orphan_name[OCFS2_ORPHAN_NAMELEN + 1];
struct ocfs2_dir_lookup_result lookup = { NULL, };
+ struct ocfs2_dir_lookup_result orphan_insert = { NULL, };

mlog_entry("(0x%p, 0x%p, symname='%s' actual='%.*s')\n", dir,
dentry, symname, dentry->d_name.len, dentry->d_name.name);
@@ -1621,14 +1626,9 @@ static int ocfs2_symlink(struct inode *dir,
goto bail;
}

- status = ocfs2_check_dir_for_entry(dir, dentry->d_name.name,
- dentry->d_name.len);
- if (status)
- goto bail;
-
- status = ocfs2_prepare_dir_for_insert(osb, dir, parent_fe_bh,
- dentry->d_name.name,
- dentry->d_name.len, &lookup);
+ status = ocfs2_prepare_orphan_dir(osb, &orphan_dir,
+ osb->root_blkno,
+ orphan_name, &orphan_insert);
if (status < 0) {
mlog_errno(status);
goto bail;
@@ -1709,7 +1709,20 @@ static int ocfs2_symlink(struct inode *dir,
goto bail;
}

+ status = ocfs2_blkno_stringify(OCFS2_I(inode)->ip_blkno, orphan_name);
+ if (status < 0) {
+ mlog_errno(status);
+ goto bail;
+ }
+
fe = (struct ocfs2_dinode *) new_fe_bh->b_data;
+ status = ocfs2_orphan_add(osb, handle, inode, fe, orphan_name,
+ &orphan_insert, orphan_dir);
+ if (status < 0) {
+ mlog_errno(status);
+ goto bail;
+ }
+
inode->i_rdev = 0;
newsize = l - 1;
if (l > ocfs2_fast_symlink_chars(sb)) {
@@ -1768,24 +1781,6 @@ static int ocfs2_symlink(struct inode *dir,
goto bail;
}
}
-
- status = ocfs2_add_entry(handle, dentry, inode,
- le64_to_cpu(fe->i_blkno), parent_fe_bh,
- &lookup);
- if (status < 0) {
- mlog_errno(status);
- goto bail;
- }
-
- status = ocfs2_dentry_attach_lock(dentry, inode, OCFS2_I(dir)->ip_blkno);
- if (status) {
- mlog_errno(status);
- goto bail;
- }
-
- insert_inode_hash(inode);
- dentry->d_op = &ocfs2_dentry_ops;
- d_instantiate(dentry, inode);
bail:
if (status < 0 && did_quota)
vfs_dq_free_space_nodirty(inode,
@@ -1802,13 +1797,24 @@ bail:
kfree(si.name);
kfree(si.value); ocfs2_free_dir_lookup_result(&lookup);
+ ocfs2_free_dir_lookup_result(&orphan_insert);
if (inode_ac)
ocfs2_free_alloc_context(inode_ac);
if (data_ac)
ocfs2_free_alloc_context(data_ac);
if (xattr_ac)
ocfs2_free_alloc_context(xattr_ac);
- if ((status < 0) && inode) {
+ if (orphan_dir) {
+ ocfs2_inode_unlock(orphan_dir, 1);
+ mutex_unlock(&orphan_dir->i_mutex);
+ iput(orphan_dir);
+ }
+ if (inode && !status) {
+ status = ocfs2_mv_orphaned_inode_to_new(dir, inode, dentry);
+ if (status)
+ mlog_errno(status);
+ }
+ if (inode && status < 0) {
clear_nlink(inode);
iput(inode);
}

1.7.0.3

Huh. At first I was sure this was ok, as the iput is sanely
killing the inode. But you're right, the wipe code expects it to be orphaned.
Do you have a testcase for this?

Joel

Life's Little Instruction Book #497

	"Go down swinging."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

I found it while trying to reproduce the bug fixed by the patch ocfs2: avoid direct write if we fall back to buffered.
In my case, just run fsstress on a single node, and when the ocfs2 partition is out of space, ocfs2_add_inode_data will return -ENOSPC and then will trigger the error in ocfs2_query_inode_wipe

Hi Li,

Thanks for the patch. Your solution is too heavy for such a special case - we can't lock the orphan dir and orphan / de-orphan an inode every time a symlink is made.

The good news is that I believe there's an easier way to handle this. You can add a flag to ocfs2_inode_info->ip_flags (see fs/ocfs2/inode.h). I would call this flag something like OCFS2_INODE_SKIP_ORPHAN_DIR. If the allocation fails, we could mark the inode with that flag. From a quick look at the code, the only places that need additional checking for the flag would be:
ocfs2_query_inode_wipe() and ocfs2_wipe_inode().

Ultimately, I think the best solution is for us to delete the inode "in-place". We actually already have all the locks and journal credits we need at that point. It's a bit tricky though because we have to be careful what state the inode gets set in, which is why for a short-term fix, the above solution is ok.

	--Mark

--
Mark Fasheh

Hi, Mark,
Thanks for reviewing, Jan Kara and I also think this is expensive.

Looking into that.

Glad to know this way, maybe I'll post an updated patch.