On 05/30/2016 04:45 AM, Gang He wrote: > Hello Goldwyn, > > Please see my comments inline. > > > Thanks > Gang > > >>>> >> From: Goldwyn Rodrigues <[email protected]> >> >> Check that the entriy exists and has been filed for check. >> Also perform some code cleanup >> >> Signed-off-by: Goldwyn Rodrigues <[email protected]> >> --- >> fs/ocfs2/filecheck.c | 41 +++++++++++++++++++++++++---------------- >> fs/ocfs2/filecheck.h | 1 + >> fs/ocfs2/sysfs.c | 2 +- >> 3 files changed, 27 insertions(+), 17 deletions(-) >> >> diff --git a/fs/ocfs2/filecheck.c b/fs/ocfs2/filecheck.c >> index 006d521..fc6e183 100644 >> --- a/fs/ocfs2/filecheck.c >> +++ b/fs/ocfs2/filecheck.c >> @@ -198,22 +198,6 @@ ocfs2_filecheck_handle(struct ocfs2_super *osb, >> return ret; >> } >> >> -static void >> -ocfs2_filecheck_handle_entry(struct ocfs2_super *osb, >> - struct ocfs2_filecheck_entry *entry) >> -{ >> - if (entry->fe_type == OCFS2_FILECHECK_TYPE_CHK) >> - entry->fe_status = ocfs2_filecheck_handle(osb, >> - entry->fe_ino, OCFS2_FI_FLAG_FILECHECK_CHK); >> - else if (entry->fe_type == OCFS2_FILECHECK_TYPE_FIX) >> - entry->fe_status = ocfs2_filecheck_handle(osb, >> - entry->fe_ino, OCFS2_FI_FLAG_FILECHECK_FIX); >> - else >> - entry->fe_status = OCFS2_FILECHECK_ERR_UNSUPPORTED; >> - >> - ocfs2_filecheck_done_entry(osb, entry); >> -} >> - >> int ocfs2_filecheck_add_inode(struct ocfs2_super *osb, >> unsigned long ino) >> { >> @@ -268,3 +252,28 @@ unlock: >> ocfs2_filecheck_done_entry(osb, entry); >> return 0; >> } >> + >> +int ocfs2_filefix_add_inode(struct ocfs2_super *osb, >> + unsigned long ino) >> +{ >> + struct ocfs2_filecheck_entry *entry; >> + int ret = -ENOENT; >> + >> + spin_lock(&osb->fc_lock); >> + list_for_each_entry(entry, &osb->file_check_entries, fe_list) >> + if (entry->fe_ino == ino) { >> + entry->fe_type = OCFS2_FILECHECK_TYPE_FIX; > Gang: It looks that we can not do it directly, why? because the entry pointer > can be freed by the function ocfs2_filecheck_erase_entries(). > We can not use the same entry pointer within two user processes. > The simple solution is to return -EBUSY error in case there is the same ino > number entry in the list, then the user can try again after the previous user > process is returned.
How? is it not protected under spinlock? Anyways, I plan to make a separate list for this so we can do away with more macros. Besides, any I/O and check operation should be done in a separate thread/work queue. > >> + ret = 0; >> + break; >> + } >> + spin_unlock(&osb->fc_lock); >> + if (ret) >> + return ret; >> + >> + entry->fe_status = ocfs2_filecheck_handle(osb, >> + entry->fe_ino, OCFS2_FI_FLAG_FILECHECK_FIX); >> + >> + ocfs2_filecheck_done_entry(osb, entry); >> + return 0; >> +} >> + >> diff --git a/fs/ocfs2/filecheck.h b/fs/ocfs2/filecheck.h >> index b1a0d8c..d5f81a7 100644 >> --- a/fs/ocfs2/filecheck.h >> +++ b/fs/ocfs2/filecheck.h >> @@ -53,6 +53,7 @@ int ocfs2_filecheck_create_sysfs(struct super_block *sb); >> int ocfs2_filecheck_remove_sysfs(struct super_block *sb); >> int ocfs2_filefix_inode(struct ocfs2_super *osb, unsigned long ino); >> int ocfs2_filecheck_add_inode(struct ocfs2_super *osb, unsigned long ino); >> +int ocfs2_filefix_add_inode(struct ocfs2_super *osb, unsigned long ino); >> int ocfs2_filecheck_set_max_entries(struct ocfs2_super *osb, int num); >> int ocfs2_filecheck_show(struct ocfs2_super *osb, unsigned int type, char >> *buf); >> >> diff --git a/fs/ocfs2/sysfs.c b/fs/ocfs2/sysfs.c >> index acc4834..ac149fb 100644 >> --- a/fs/ocfs2/sysfs.c >> +++ b/fs/ocfs2/sysfs.c >> @@ -91,7 +91,7 @@ static ssize_t file_fix_store(struct ocfs2_super *osb, >> ret = kstrtoul(skip_spaces(buf), 0, &t); >> if (ret) >> return ret; >> - ret = ocfs2_filecheck_add_inode(osb, t); >> + ret = ocfs2_filefix_add_inode(osb, t); >> if (ret) >> return ret; >> return count; >> -- >> 2.6.6 >> >> >> _______________________________________________ >> Ocfs2-devel mailing list >> [email protected] >> https://oss.oracle.com/mailman/listinfo/ocfs2-devel -- Goldwyn _______________________________________________ Ocfs2-devel mailing list [email protected] https://oss.oracle.com/mailman/listinfo/ocfs2-devel
