Mark Fasheh wrote: > On Sun, Jan 30, 2011 at 02:26:00PM +0800, Tristan Ye wrote: >> The new code is dedicated to calculate free inodes number of all >> inode_allocs, >> then return the info to userpace in terms of an array. >> >> Specially, flag 'OCFS2_INFO_FL_NON_COHERENT', manipulated by >> '--cluster-coherent' >> from userspace, is now going to be involved. setting the flag on means no >> cluster >> coherency considered, usually, userspace tools choose none-coherency >> strategy by >> default for the sake of performace. >> >> Signed-off-by: Tristan Ye <tristan...@oracle.com> >> --- >> fs/ocfs2/ioctl.c | 122 >> ++++++++++++++++++++++++++++++++++++++++++++++++ >> fs/ocfs2/ocfs2_ioctl.h | 11 ++++ >> 2 files changed, 133 insertions(+), 0 deletions(-) >> >> diff --git a/fs/ocfs2/ioctl.c b/fs/ocfs2/ioctl.c >> index 731cf46..18812be 100644 >> --- a/fs/ocfs2/ioctl.c >> +++ b/fs/ocfs2/ioctl.c >> @@ -23,6 +23,9 @@ >> #include "ioctl.h" >> #include "resize.h" >> #include "refcounttree.h" >> +#include "sysfile.h" >> +#include "dir.h" >> +#include "buffer_head_io.h" >> >> #include <linux/ext2_fs.h> >> >> @@ -62,6 +65,13 @@ static inline void __o2info_clear_request_filled(struct >> ocfs2_info_request *req) >> #define o2info_clear_request_filled(a) \ >> __o2info_clear_request_filled((struct ocfs2_info_request *)&(a)) >> >> +static inline int __o2info_coherent(struct ocfs2_info_request *req) >> +{ >> + return (!(req->ir_flags & OCFS2_INFO_FL_NON_COHERENT)); >> +} >> + >> +#define o2info_coherent(a) __o2info_coherent((struct ocfs2_info_request >> *)&(a)) > > Same comment about typechecking as before. > > >> static int ocfs2_get_inode_attr(struct inode *inode, unsigned *flags) >> { >> int status; >> @@ -321,6 +331,114 @@ bail: >> return status; >> } >> >> +int ocfs2_info_scan_inode_alloc(struct ocfs2_super *osb, >> + struct inode *inode_alloc, u64 blkno, >> + struct ocfs2_info_freeinode *fi, u32 slot) >> +{ >> + int status = 0, unlock = 0; >> + >> + struct buffer_head *bh = NULL; >> + struct ocfs2_dinode *dinode_alloc = NULL; >> + >> + if (inode_alloc) >> + mutex_lock(&inode_alloc->i_mutex); >> + >> + if (o2info_coherent(*fi)) { >> + status = ocfs2_inode_lock(inode_alloc, &bh, 0); >> + if (status < 0) { >> + mlog_errno(status); >> + goto bail; >> + } >> + unlock = 1; >> + } else { >> + status = ocfs2_read_blocks_sync(osb, blkno, 1, &bh); >> + if (status < 0) { >> + mlog_errno(status); >> + goto bail; >> + } >> + } >> + >> + dinode_alloc = (struct ocfs2_dinode *)bh->b_data; >> + >> + fi->ifi_stat[slot].lfi_total = >> + le32_to_cpu(dinode_alloc->id1.bitmap1.i_total); >> + fi->ifi_stat[slot].lfi_free = >> + le32_to_cpu(dinode_alloc->id1.bitmap1.i_total) - >> + le32_to_cpu(dinode_alloc->id1.bitmap1.i_used); >> + >> +bail: >> + if (unlock) >> + ocfs2_inode_unlock(inode_alloc, 0); >> + >> + if (inode_alloc) >> + mutex_unlock(&inode_alloc->i_mutex); >> + >> + if (inode_alloc) >> + iput(inode_alloc); > ^^^^^^^^^^^^^^^^^^ > > Is this iput necessary? I don't see a balancing ref in the function.
Good catch, better to iput the inode in the same place where it was got. > >> + >> + brelse(bh); >> + >> + mlog_exit(status); >> + return status; >> +} >> + >> +int ocfs2_info_handle_freeinode(struct inode *inode, >> + struct ocfs2_info_request __user *req) >> +{ >> + u32 i; >> + u64 blkno = -1; >> + char namebuf[40]; >> + int status = -EFAULT, type = INODE_ALLOC_SYSTEM_INODE; >> + struct ocfs2_info_freeinode oifi; >> + struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); >> + struct inode *inode_alloc = NULL; >> + >> + if (o2info_from_user(oifi, req)) >> + goto bail; >> + >> + oifi.ifi_slotnum = osb->max_slots; >> + >> + for (i = 0; i < oifi.ifi_slotnum; i++) { >> + if (o2info_coherent(oifi)) { >> + inode_alloc = ocfs2_get_system_file_inode(osb, type, i); >> + if (!inode_alloc) { >> + mlog(ML_ERROR, "unable to get alloc inode in " >> + "slot %u\n", i); >> + status = -EIO; >> + goto bail; >> + } >> + } else { >> + ocfs2_sprintf_system_inode_name(namebuf, >> + sizeof(namebuf), >> + type, i); >> + status = ocfs2_lookup_ino_from_name(osb->sys_root_inode, >> + namebuf, >> + strlen(namebuf), >> + &blkno); >> + if (status < 0) { >> + status = -ENOENT; >> + goto bail; >> + } >> + } >> + >> + status = ocfs2_info_scan_inode_alloc(osb, inode_alloc, blkno, >> &oifi, i); > > You need the following here: > > iput(inode_alloc); > inode_alloc = NULL; > >> + if (status < 0) >> + goto bail; >> + } >> + >> + o2info_set_request_filled(oifi); >> + >> + if (o2info_to_user(oifi, req)) >> + goto bail; >> + >> + status = 0; >> +bail: >> + if (status) >> + o2info_set_request_error(oifi, req); >> + >> + return status; >> +} >> + >> int ocfs2_info_handle_unknown(struct inode *inode, >> struct ocfs2_info_request __user *req) >> { >> @@ -392,6 +510,10 @@ int ocfs2_info_handle_request(struct inode *inode, >> if (oir.ir_size == sizeof(struct ocfs2_info_journal_size)) >> status = ocfs2_info_handle_journal_size(inode, req); >> break; >> + case OCFS2_INFO_FREEINODE: >> + if (oir.ir_size == sizeof(struct ocfs2_info_freeinode)) >> + status = ocfs2_info_handle_freeinode(inode, req); >> + break; >> default: >> status = ocfs2_info_handle_unknown(inode, req); >> break; >> diff --git a/fs/ocfs2/ocfs2_ioctl.h b/fs/ocfs2/ocfs2_ioctl.h >> index b46f39b..6b4b39a 100644 >> --- a/fs/ocfs2/ocfs2_ioctl.h >> +++ b/fs/ocfs2/ocfs2_ioctl.h >> @@ -142,6 +142,16 @@ struct ocfs2_info_journal_size { >> __u64 ij_journal_size; >> }; >> >> +struct ocfs2_info_freeinode { >> + struct ocfs2_info_request ifi_req; >> + struct ocfs2_info_local_freeinode { >> + __u64 lfi_total; >> + __u64 lfi_free; >> + } ifi_stat[OCFS2_MAX_SLOTS]; >> + __u32 ifi_slotnum; /* out */ >> + __u32 ifi_pad; >> +}; > > Any reason why we don't keep the array (ocfs2_info_local_freeinode) at the > end of this structure like this: > > struct ocfs2_info_freeinode { > struct ocfs2_info_request ifi_req; > __u32 ifi_slotnum; /* out */ > __u32 ifi_pad; > struct ocfs2_info_local_freeinode { > __u64 lfi_total; > __u64 lfi_free; > } ifi_stat[OCFS2_MAX_SLOTS]; > }; Yep, placing the struct in the end didn't hurt any alignment problem, while I'm wondering how it will be hurting when It wasn't;-) > --Mark > > -- > Mark Fasheh _______________________________________________ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com http://oss.oracle.com/mailman/listinfo/ocfs2-devel