Hi Mingming,

Thanks for pointing out the problem and making the required changes. I
have tested the changes and it works properly for all cases. The only
downside being that if a filesystem was using EAs and now is compiled
without XATTR support, inode expansion will not work on some inodes. But
the "-E expand_extra_isize" option patch for e2fsck should solve this
problem.

Acked-by: Kalpak Shah <[EMAIL PROTECTED]>

Thanks,
Kalpak.

P.S.: Maybe the "allow more than 32000 subdirectories" should also be
included in the 2.6.21-ext4 patchset (or 2.6.22-ext4 rather).

On Tue, 2007-05-15 at 18:15 -0700, Mingming Cao wrote:
> On Wed, 2007-04-11 at 18:47 +0530, Kalpak Shah wrote: 
> > Hi,
> > 
> > This patch is on top of the nanosecond timestamp and i_version_hi
> > patches. 
> > 
> > This patch adds 64-bit inode version support to ext4. The lower 32 bits
> > are stored in the osd1.linux1.l_i_version field while the high 32 bits
> > are stored in the i_version_hi field newly created in the ext4_inode.
> > 
> > We need to make sure that existing filesystems can also avail the new
> > fields that have been added to the inode.
> 
> Hi Kalpak,
> 
> Failed to build ext4 as module. It is because CONFIG_EXT4DEV_FS_XATTR is
> not configed but ext4_expand_extra_isize() assumes it's on.
> 
> > @@ -3173,10 +3186,32 @@ ext4_reserve_inode_write(handle_t *handl
> >  int ext4_mark_inode_dirty(handle_t *handle, struct inode *inode)
> >  {
> >     struct ext4_iloc iloc;
> > -   int err;
> > +   int err, ret;
> > +   static int expand_message;
> > 
> >     might_sleep();
> >     err = ext4_reserve_inode_write(handle, inode, &iloc);
> > +   if (EXT4_I(inode)->i_extra_isize <
> > +       EXT4_SB(inode->i_sb)->s_want_extra_isize &&
> > +       !(EXT4_I(inode)->i_state & EXT4_STATE_NO_EXPAND)) {
> > +           /* We need extra buffer credits since we may write into EA block
> > +            * with this same handle */
> > +           if ((jbd2_journal_extend(handle,
> > +                        EXT4_DATA_TRANS_BLOCKS(inode->i_sb))) == 0) {
> > +                   ret = ext4_expand_extra_isize(inode,
> > +                                   
> > EXT4_SB(inode->i_sb)->s_want_extra_isize,
> > +                                   iloc, handle);
> 
> Here is the place where ext4_expand_extra_isize can be called without
> xattrs turned on.
> 
> > Index: linux-2.6.20/fs/ext4/xattr.c
> > ===================================================================
> > --- linux-2.6.20.orig/fs/ext4/xattr.c
> > +++ linux-2.6.20/fs/ext4/xattr.c
> > @@ -502,6 +502,20 @@ ext4_xattr_release_block(handle_t *handl
> >     }
> >  }
> > 
> > +static inline size_t ext3_xattr_free_space(struct ext4_xattr_entry *last,
> > +                               size_t *min_offs, void *base, int *total)
> 
> should renamed to ext4_xattr_free_space()
> 
> > +static void ext3_xattr_shift_entries(struct ext4_xattr_entry *entry,
> > +                                int value_offs_shift, void *to,
> > +                                void *from, size_t n, int blocksize)
> 
> Should rename to ext4_xxx_xxx().
> 
> > +/* Expand an inode by new_extra_isize bytes.
> > + * Returns 0 on success or negative error number on failure.
> > + */
> > +int ext4_expand_extra_isize(struct inode *inode, int new_extra_isize,
> > +                       struct ext4_iloc iloc, handle_t *handle)
> > +{
> 
> ....
> 
> > Index: linux-2.6.20/fs/ext4/xattr.h
> > ===================================================================
> > --- linux-2.6.20.orig/fs/ext4/xattr.h
> > +++ linux-2.6.20/fs/ext4/xattr.h
> > @@ -74,6 +74,9 @@ extern int ext4_xattr_set_handle(handle_
> >  extern void ext4_xattr_delete_inode(handle_t *, struct inode *);
> >  extern void ext4_xattr_put_super(struct super_block *);
> > 
> > +int ext4_expand_extra_isize(struct inode *inode, int new_extra_isize,
> > +                       struct ext4_iloc iloc, handle_t *handle);
> > +
> >  extern int init_ext4_xattr(void);
> >  extern void exit_ext4_xattr(void);
> > 
> > 
> 
> The following patch moved the ext4_expand_extra_isize() function to
> inode.c and provide proper defines in xattr.h. Renamed the ext3
> functions to ext4_xxx_xxx().
> 
> Compile tested. Can you Ack the changes. Appreciate if you can let me
> know it passes your tests.
> 
> Signed-Off-By: Mingming Cao <[EMAIL PROTECTED]>



> Index: linux-2.6.22-rc1/fs/ext4/inode.c
> ===================================================================
> --- linux-2.6.22-rc1.orig/fs/ext4/inode.c     2007-05-15 17:44:25.000000000 
> -0700
> +++ linux-2.6.22-rc1/fs/ext4/inode.c  2007-05-15 17:46:23.000000000 -0700
> @@ -3097,6 +3097,40 @@
>  }
>  
>  /*
> + * Expand an inode by new_extra_isize bytes.
> + * Returns 0 on success or negative error number on failure.
> + */
> +int ext4_expand_extra_isize(struct inode *inode, unsigned int 
> new_extra_isize,
> +                        struct ext4_iloc iloc, handle_t *handle)
> +{
> +     struct ext4_inode *raw_inode;
> +     struct ext4_xattr_ibody_header *header;
> +     struct ext4_xattr_entry *entry;
> +
> +     if (EXT4_I(inode)->i_extra_isize >= new_extra_isize) {
> +             return 0;
> +     }
> +
> +     raw_inode = ext4_raw_inode(&iloc);
> +
> +     header = IHDR(inode, raw_inode);
> +        entry = IFIRST(header);
> +
> +     /* No extended attributes present */
> +     if (!(EXT4_I(inode)->i_state & EXT4_STATE_XATTR) ||
> +             header->h_magic != cpu_to_le32(EXT4_XATTR_MAGIC)) {
> +             memset((void *)raw_inode + EXT4_GOOD_OLD_INODE_SIZE, 0,
> +                     new_extra_isize);
> +             EXT4_I(inode)->i_extra_isize = new_extra_isize;
> +             return 0;
> +     }
> +
> +     /* try to expand with EA present */
> +     return ext4_expand_extra_isize_ea(inode, new_extra_isize,
> +                                             raw_inode, handle);
> +}
> +
> +/*
>   * What we do here is to mark the in-core inode as clean with respect to 
> inode
>   * dirtiness (it may still be data-dirty).
>   * This means that the in-core inode may be reaped by prune_icache
> Index: linux-2.6.22-rc1/fs/ext4/xattr.c
> ===================================================================
> --- linux-2.6.22-rc1.orig/fs/ext4/xattr.c     2007-05-15 17:44:25.000000000 
> -0700
> +++ linux-2.6.22-rc1/fs/ext4/xattr.c  2007-05-15 17:46:23.000000000 -0700
> @@ -66,13 +66,6 @@
>  #define BFIRST(bh) ENTRY(BHDR(bh)+1)
>  #define IS_LAST_ENTRY(entry) (*(__u32 *)(entry) == 0)
>  
> -#define IHDR(inode, raw_inode) \
> -     ((struct ext4_xattr_ibody_header *) \
> -             ((void *)raw_inode + \
> -              EXT4_GOOD_OLD_INODE_SIZE + \
> -              EXT4_I(inode)->i_extra_isize))
> -#define IFIRST(hdr) ((struct ext4_xattr_entry *)((hdr)+1))
> -
>  #ifdef EXT4_XATTR_DEBUG
>  # define ea_idebug(inode, f...) do { \
>               printk(KERN_DEBUG "inode %s:%lu: ", \
> @@ -508,7 +501,7 @@
>       return;
>  }
>  
> -static inline size_t ext3_xattr_free_space(struct ext4_xattr_entry *last,
> +static inline size_t ext4_xattr_free_space(struct ext4_xattr_entry *last,
>                                   size_t *min_offs, void *base, int *total)
>  {
>       for (; !IS_LAST_ENTRY(last); last = EXT4_XATTR_NEXT(last)) {
> @@ -1083,7 +1076,7 @@
>       return error;
>  }
>  
> -static void ext3_xattr_shift_entries(struct ext4_xattr_entry *entry,
> +static void ext4_xattr_shift_entries(struct ext4_xattr_entry *entry,
>                                    int value_offs_shift, void *to,
>                                    void *from, size_t n, int blocksize)
>  {
> @@ -1103,13 +1096,14 @@
>       memmove(to, from, n);
>  }
>  
> -/* Expand an inode by new_extra_isize bytes.
> +/*
> + * Expand an inode by new_extra_isize bytes when EA presents.
>   * Returns 0 on success or negative error number on failure.
> + *
>   */
>  int ext4_expand_extra_isize(struct inode *inode, int new_extra_isize,
> -                         struct ext4_iloc iloc, handle_t *handle)
> +                         struct ext4_inode *raw_inode, handle_t *handle)
>  {
> -     struct ext4_inode *raw_inode;
>       struct ext4_xattr_ibody_header *header;
>       struct ext4_xattr_entry *entry, *last, *first;
>       struct buffer_head *bh = NULL;
> @@ -1123,27 +1117,15 @@
>       int s_min_extra_isize = EXT4_SB(inode->i_sb)->s_es->s_min_extra_isize;
>  
>       down_write(&EXT4_I(inode)->xattr_sem);
> -
>  retry:
>       if (EXT4_I(inode)->i_extra_isize >= new_extra_isize) {
>               up_write(&EXT4_I(inode)->xattr_sem);
>               return 0;
>       }
>  
> -     raw_inode = ext4_raw_inode(&iloc);
> -
>       header = IHDR(inode, raw_inode);
>       entry = IFIRST(header);
>  
> -     /* No extended attributes present */
> -     if (!(EXT4_I(inode)->i_state & EXT4_STATE_XATTR) ||
> -         header->h_magic != cpu_to_le32(EXT4_XATTR_MAGIC)) {
> -             memset((void *)raw_inode + EXT4_GOOD_OLD_INODE_SIZE, 0,
> -                    new_extra_isize);
> -             EXT4_I(inode)->i_extra_isize = new_extra_isize;
> -             goto cleanup;
> -     }
> -
>       /*
>        * Check if enough free space is available in the inode to shift the
>        * entries ahead by new_extra_isize.
> @@ -1155,10 +1137,10 @@
>       last = entry;
>       total_ino = sizeof(struct ext4_xattr_ibody_header);
>  
> -     free = ext3_xattr_free_space(last, &min_offs, base, &total_ino);
> +     free = ext4_xattr_free_space(last, &min_offs, base, &total_ino);
>       if (free >= new_extra_isize) {
>               entry = IFIRST(header);
> -             ext3_xattr_shift_entries(entry, EXT4_I(inode)->i_extra_isize
> +             ext4_xattr_shift_entries(entry, EXT4_I(inode)->i_extra_isize
>                               - new_extra_isize, (void *)raw_inode +
>                               EXT4_GOOD_OLD_INODE_SIZE + new_extra_isize,
>                               (void *)header, total_ino,
> @@ -1188,7 +1170,7 @@
>               first = BFIRST(bh);
>               end = bh->b_data + bh->b_size;
>               min_offs = end - base;
> -             free = ext3_xattr_free_space(first, &min_offs, base,
> +             free = ext4_xattr_free_space(first, &min_offs, base,
>                                            &total_blk);
>               if (free < new_extra_isize) {
>                       if (!tried_min_extra_isize && s_min_extra_isize) {
> @@ -1287,7 +1269,7 @@
>               else
>                       shift_bytes = entry_size + size;
>               /* Adjust the offsets and shift the remaining entries ahead */
> -             ext3_xattr_shift_entries(entry, EXT4_I(inode)->i_extra_isize -
> +             ext4_xattr_shift_entries(entry, EXT4_I(inode)->i_extra_isize -
>                       shift_bytes, (void *)raw_inode +
>                       EXT4_GOOD_OLD_INODE_SIZE + extra_isize + shift_bytes,
>                       (void *)header, total_ino - entry_size,
> Index: linux-2.6.22-rc1/fs/ext4/xattr.h
> ===================================================================
> --- linux-2.6.22-rc1.orig/fs/ext4/xattr.h     2007-05-15 17:44:25.000000000 
> -0700
> +++ linux-2.6.22-rc1/fs/ext4/xattr.h  2007-05-15 17:48:26.000000000 -0700
> @@ -56,6 +56,13 @@
>  #define EXT4_XATTR_SIZE(size) \
>       (((size) + EXT4_XATTR_ROUND) & ~EXT4_XATTR_ROUND)
>  
> +#define IHDR(inode, raw_inode) \
> +     ((struct ext4_xattr_ibody_header *) \
> +             ((void *)raw_inode + \
> +             EXT4_GOOD_OLD_INODE_SIZE + \
> +             EXT4_I(inode)->i_extra_isize))
> +#define IFIRST(hdr) ((struct ext4_xattr_entry *)((hdr)+1))
> +
>  # ifdef CONFIG_EXT4DEV_FS_XATTR
>  
>  extern struct xattr_handler ext4_xattr_user_handler;
> @@ -74,8 +81,8 @@
>  extern void ext4_xattr_delete_inode(handle_t *, struct inode *);
>  extern void ext4_xattr_put_super(struct super_block *);
>  
> -int ext4_expand_extra_isize(struct inode *inode, int new_extra_isize,
> -                         struct ext4_iloc iloc, handle_t *handle);
> +extern int ext4_expand_extra_isize_ea(struct inode *inode, int 
> new_extra_isize,
> +                         struct ext4_inode *raw_inode, handle_t *handle);
>  
>  extern int init_ext4_xattr(void);
>  extern void exit_ext4_xattr(void);
> @@ -132,6 +139,13 @@
>  {
>  }
>  
> +static int
> +ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
> +                         struct ext4_inode *raw_inode, handle_t *handle)
> +{
> +     return -EOPNOTSUPP;
> +}
> +
>  #define ext4_xattr_handlers  NULL
>  
>  # endif  /* CONFIG_EXT4DEV_FS_XATTR */
> 
> 

-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to