On Mon, Oct 17, 2016 at 05:01:46PM +0800, Wang Xiaoguang wrote:
> > [..]
> >>   int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end,
> >> -                        struct extent_state **cached_state);
> >> +                        struct extent_state **cached_state, int flag);
> >>   int btrfs_set_extent_defrag(struct inode *inode, u64 start, u64 end,
> >> -                      struct extent_state **cached_state);
> >> +                      struct extent_state **cached_state, int flag);
> > [..]
> >>   int btrfs_dirty_pages(struct btrfs_root *root, struct inode *inode,
> >>                  struct page **pages, size_t num_pages,
> >>                  loff_t pos, size_t write_bytes,
> >> -                struct extent_state **cached);
> >> +                struct extent_state **cached, int flag);
> > Instead of adding "int flag" why not use the already defined
> > btrfs_metadata_reserve_type enum? I know it's just an int at the end of
> > the day, but the dedupe support already added another "int dedupe" argument
> > and it's probably easy to cause confusion.
> > Maybe later it would be beneficial to consolidate the flags into a 
> > consistent
> > set of enum values to prevent more "int flag" inflation and better declare 
> > the
> > intent of the extent state change. Not sure if that makes sense.
> Yes, agree.
> I'll rebase them later, thanks.

Would be great. I won't manually merge the patch now as it's not a
conflict against the current state, btrfs_set_extent_delalloc has the
extra parameter already. Please consolidate them before this patch is
supposed to be merged. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to