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

Reply via email to