Message ID | 20181030131823.29040-18-cmaiolino@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | New ->fiemap infrastructure and ->bmap removal | expand |
On Oct 30, 2018, at 7:18 AM, Carlos Maiolino <cmaiolino@redhat.com> wrote: > > With the goal of using FIEMAP infra-structure for FIBMAP ioctl, and with > the updates being done previously into the FIEMAP interface, the > fiemap_extent_info structure is no longer necessary, so, move > fi_extents_mapped and fi_extents_max up to fiemap_ctx. and use > fiemap_ctx.fc_data to hold a pointer to struct fiemap_extent. Having the "void *fc_data" pointer point to fiemap_extent is not necessarily intuitive. Why not just make it directly point to it: struct fiemap_extent *fc_extent; Cheers, Andreas > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > --- > fs/btrfs/extent_io.c | 3 +-- > fs/ioctl.c | 29 +++++++++++++---------------- > include/linux/fs.h | 9 ++------- > include/linux/iomap.h | 1 - > 4 files changed, 16 insertions(+), 26 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 6a8bc502bc04..88b8a4416dfc 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -4436,7 +4436,6 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx) > struct btrfs_path *path; > struct btrfs_root *root = BTRFS_I(inode)->root; > struct fiemap_cache cache = { 0 }; > - struct fiemap_extent_info *fieinfo = f_ctx->fc_data; > int end = 0; > u64 em_start = 0; > u64 em_len = 0; > @@ -4561,7 +4560,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx) > } else if (em->block_start == EXTENT_MAP_DELALLOC) { > flags |= (FIEMAP_EXTENT_DELALLOC | > FIEMAP_EXTENT_UNKNOWN); > - } else if (fieinfo->fi_extents_max) { > + } else if (f_ctx->fc_extents_max) { > u64 bytenr = em->block_start - > (em->start - em->orig_start); > > diff --git a/fs/ioctl.c b/fs/ioctl.c > index cbc1b21c4f7c..71d11201a06b 100644 > --- a/fs/ioctl.c > +++ b/fs/ioctl.c > @@ -78,17 +78,16 @@ static int ioctl_fibmap(struct file *filp, int __user *p) > int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical, > u64 phys, u64 len, u32 flags) > { > - struct fiemap_extent_info *fieinfo = f_ctx->fc_data; > struct fiemap_extent extent; > - struct fiemap_extent __user *dest = fieinfo->fi_extents_start; > + struct fiemap_extent __user *dest = f_ctx->fc_data; > > /* only count the extents */ > - if (fieinfo->fi_extents_max == 0) { > - fieinfo->fi_extents_mapped++; > + if (f_ctx->fc_extents_max == 0) { > + f_ctx->fc_extents_mapped++; > return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0; > } > > - if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max) > + if (f_ctx->fc_extents_mapped >= f_ctx->fc_extents_max) > return 1; > > if (flags & SET_UNKNOWN_FLAGS) > @@ -104,12 +103,12 @@ int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical, > extent.fe_length = len; > extent.fe_flags = flags; > > - dest += fieinfo->fi_extents_mapped; > + dest += f_ctx->fc_extents_mapped; > if (copy_to_user(dest, &extent, sizeof(extent))) > return -EFAULT; > > - fieinfo->fi_extents_mapped++; > - if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max) > + f_ctx->fc_extents_mapped++; > + if (f_ctx->fc_extents_mapped == f_ctx->fc_extents_max) > return 1; > return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0; > } > @@ -189,7 +188,6 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg) > { > struct fiemap fiemap; > struct fiemap __user *ufiemap = (struct fiemap __user *) arg; > - struct fiemap_extent_info fieinfo = { 0, }; > struct inode *inode = file_inode(filp); > struct super_block *sb = inode->i_sb; > struct fiemap_ctx f_ctx; > @@ -210,19 +208,18 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg) > if (error) > return error; > > - fieinfo.fi_extents_max = fiemap.fm_extent_count; > - fieinfo.fi_extents_start = ufiemap->fm_extents; > - > + f_ctx.fc_extents_max = fiemap.fm_extent_count; > + f_ctx.fc_data = ufiemap->fm_extents; > + f_ctx.fc_extents_mapped = 0; > f_ctx.fc_flags = fiemap.fm_flags; > - f_ctx.fc_data = &fieinfo; > f_ctx.fc_start = fiemap.fm_start; > f_ctx.fc_len = len; > > f_ctx.fc_cb = fiemap_fill_usr_extent; > > if (fiemap.fm_extent_count != 0 && > - !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start, > - fieinfo.fi_extents_max * sizeof(struct fiemap_extent))) > + !access_ok(VERIFY_WRITE, f_ctx.fc_data, > + f_ctx.fc_extents_max * sizeof(struct fiemap_extent))) > return -EFAULT; > > if (f_ctx.fc_flags & FIEMAP_FLAG_SYNC) > @@ -230,7 +227,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg) > > error = inode->i_op->fiemap(inode, &f_ctx); > fiemap.fm_flags = f_ctx.fc_flags; > - fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped; > + fiemap.fm_mapped_extents = f_ctx.fc_extents_mapped; > if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap))) > error = -EFAULT; > > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 9a538bc0dac2..4c6dee908a38 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -1691,18 +1691,13 @@ extern bool may_open_dev(const struct path *path); > > struct fiemap_ctx; > > -struct fiemap_extent_info { > - unsigned int fi_extents_mapped; /* Number of mapped extents */ > - unsigned int fi_extents_max; /* Size of fiemap_extent array */ > - struct fiemap_extent __user *fi_extents_start; /* Start of > - fiemap_extent array */ > -}; > - > typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical, > u64 phys, u64 len, u32 flags); > > struct fiemap_ctx { > unsigned int fc_flags; /* Flags as passed from user */ > + unsigned int fc_extents_mapped; /* Number of mapped extents */ > + unsigned int fc_extents_max; /* Size of fiemap_extent array */ > void *fc_data; > fiemap_fill_cb fc_cb; > u64 fc_start; > diff --git a/include/linux/iomap.h b/include/linux/iomap.h > index 7ce84be499d6..21643bcf6104 100644 > --- a/include/linux/iomap.h > +++ b/include/linux/iomap.h > @@ -9,7 +9,6 @@ > #include <linux/mm_types.h> > > struct address_space; > -struct fiemap_extent_info; > struct inode; > struct iov_iter; > struct kiocb; > -- > 2.17.1 > Cheers, Andreas
On Mon, Nov 05, 2018 at 03:14:38PM -0700, Andreas Dilger wrote: > On Oct 30, 2018, at 7:18 AM, Carlos Maiolino <cmaiolino@redhat.com> wrote: > > > > With the goal of using FIEMAP infra-structure for FIBMAP ioctl, and with > > the updates being done previously into the FIEMAP interface, the > > fiemap_extent_info structure is no longer necessary, so, move > > fi_extents_mapped and fi_extents_max up to fiemap_ctx. and use > > fiemap_ctx.fc_data to hold a pointer to struct fiemap_extent. > > Having the "void *fc_data" pointer point to fiemap_extent is not > necessarily intuitive. Why not just make it directly point to it: > > struct fiemap_extent *fc_extent; Both data structures pointed by *fc_data contains a fiemap_extent struct, but one is a __user annotated address while other is a kernel space address, I don't know if I can use a non __user pointer as you suggested to store the address of a __user pointer, I actually don't think I can without getting warnings, but I honestly don't remember. > > Cheers, Andreas > > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > > --- > > fs/btrfs/extent_io.c | 3 +-- > > fs/ioctl.c | 29 +++++++++++++---------------- > > include/linux/fs.h | 9 ++------- > > include/linux/iomap.h | 1 - > > 4 files changed, 16 insertions(+), 26 deletions(-) > > > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > > index 6a8bc502bc04..88b8a4416dfc 100644 > > --- a/fs/btrfs/extent_io.c > > +++ b/fs/btrfs/extent_io.c > > @@ -4436,7 +4436,6 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx) > > struct btrfs_path *path; > > struct btrfs_root *root = BTRFS_I(inode)->root; > > struct fiemap_cache cache = { 0 }; > > - struct fiemap_extent_info *fieinfo = f_ctx->fc_data; > > int end = 0; > > u64 em_start = 0; > > u64 em_len = 0; > > @@ -4561,7 +4560,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx) > > } else if (em->block_start == EXTENT_MAP_DELALLOC) { > > flags |= (FIEMAP_EXTENT_DELALLOC | > > FIEMAP_EXTENT_UNKNOWN); > > - } else if (fieinfo->fi_extents_max) { > > + } else if (f_ctx->fc_extents_max) { > > u64 bytenr = em->block_start - > > (em->start - em->orig_start); > > > > diff --git a/fs/ioctl.c b/fs/ioctl.c > > index cbc1b21c4f7c..71d11201a06b 100644 > > --- a/fs/ioctl.c > > +++ b/fs/ioctl.c > > @@ -78,17 +78,16 @@ static int ioctl_fibmap(struct file *filp, int __user *p) > > int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical, > > u64 phys, u64 len, u32 flags) > > { > > - struct fiemap_extent_info *fieinfo = f_ctx->fc_data; > > struct fiemap_extent extent; > > - struct fiemap_extent __user *dest = fieinfo->fi_extents_start; > > + struct fiemap_extent __user *dest = f_ctx->fc_data; > > > > /* only count the extents */ > > - if (fieinfo->fi_extents_max == 0) { > > - fieinfo->fi_extents_mapped++; > > + if (f_ctx->fc_extents_max == 0) { > > + f_ctx->fc_extents_mapped++; > > return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0; > > } > > > > - if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max) > > + if (f_ctx->fc_extents_mapped >= f_ctx->fc_extents_max) > > return 1; > > > > if (flags & SET_UNKNOWN_FLAGS) > > @@ -104,12 +103,12 @@ int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical, > > extent.fe_length = len; > > extent.fe_flags = flags; > > > > - dest += fieinfo->fi_extents_mapped; > > + dest += f_ctx->fc_extents_mapped; > > if (copy_to_user(dest, &extent, sizeof(extent))) > > return -EFAULT; > > > > - fieinfo->fi_extents_mapped++; > > - if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max) > > + f_ctx->fc_extents_mapped++; > > + if (f_ctx->fc_extents_mapped == f_ctx->fc_extents_max) > > return 1; > > return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0; > > } > > @@ -189,7 +188,6 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg) > > { > > struct fiemap fiemap; > > struct fiemap __user *ufiemap = (struct fiemap __user *) arg; > > - struct fiemap_extent_info fieinfo = { 0, }; > > struct inode *inode = file_inode(filp); > > struct super_block *sb = inode->i_sb; > > struct fiemap_ctx f_ctx; > > @@ -210,19 +208,18 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg) > > if (error) > > return error; > > > > - fieinfo.fi_extents_max = fiemap.fm_extent_count; > > - fieinfo.fi_extents_start = ufiemap->fm_extents; > > - > > + f_ctx.fc_extents_max = fiemap.fm_extent_count; > > + f_ctx.fc_data = ufiemap->fm_extents; > > + f_ctx.fc_extents_mapped = 0; > > f_ctx.fc_flags = fiemap.fm_flags; > > - f_ctx.fc_data = &fieinfo; > > f_ctx.fc_start = fiemap.fm_start; > > f_ctx.fc_len = len; > > > > f_ctx.fc_cb = fiemap_fill_usr_extent; > > > > if (fiemap.fm_extent_count != 0 && > > - !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start, > > - fieinfo.fi_extents_max * sizeof(struct fiemap_extent))) > > + !access_ok(VERIFY_WRITE, f_ctx.fc_data, > > + f_ctx.fc_extents_max * sizeof(struct fiemap_extent))) > > return -EFAULT; > > > > if (f_ctx.fc_flags & FIEMAP_FLAG_SYNC) > > @@ -230,7 +227,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg) > > > > error = inode->i_op->fiemap(inode, &f_ctx); > > fiemap.fm_flags = f_ctx.fc_flags; > > - fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped; > > + fiemap.fm_mapped_extents = f_ctx.fc_extents_mapped; > > if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap))) > > error = -EFAULT; > > > > diff --git a/include/linux/fs.h b/include/linux/fs.h > > index 9a538bc0dac2..4c6dee908a38 100644 > > --- a/include/linux/fs.h > > +++ b/include/linux/fs.h > > @@ -1691,18 +1691,13 @@ extern bool may_open_dev(const struct path *path); > > > > struct fiemap_ctx; > > > > -struct fiemap_extent_info { > > - unsigned int fi_extents_mapped; /* Number of mapped extents */ > > - unsigned int fi_extents_max; /* Size of fiemap_extent array */ > > - struct fiemap_extent __user *fi_extents_start; /* Start of > > - fiemap_extent array */ > > -}; > > - > > typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical, > > u64 phys, u64 len, u32 flags); > > > > struct fiemap_ctx { > > unsigned int fc_flags; /* Flags as passed from user */ > > + unsigned int fc_extents_mapped; /* Number of mapped extents */ > > + unsigned int fc_extents_max; /* Size of fiemap_extent array */ > > void *fc_data; > > fiemap_fill_cb fc_cb; > > u64 fc_start; > > diff --git a/include/linux/iomap.h b/include/linux/iomap.h > > index 7ce84be499d6..21643bcf6104 100644 > > --- a/include/linux/iomap.h > > +++ b/include/linux/iomap.h > > @@ -9,7 +9,6 @@ > > #include <linux/mm_types.h> > > > > struct address_space; > > -struct fiemap_extent_info; > > struct inode; > > struct iov_iter; > > struct kiocb; > > -- > > 2.17.1 > > > > > Cheers, Andreas > > > > >
On Tue, Oct 30, 2018 at 02:18:20PM +0100, Carlos Maiolino wrote: > With the goal of using FIEMAP infra-structure for FIBMAP ioctl, and with > the updates being done previously into the FIEMAP interface, the > fiemap_extent_info structure is no longer necessary, so, move > fi_extents_mapped and fi_extents_max up to fiemap_ctx. and use > fiemap_ctx.fc_data to hold a pointer to struct fiemap_extent. > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > --- > fs/btrfs/extent_io.c | 3 +-- > fs/ioctl.c | 29 +++++++++++++---------------- > include/linux/fs.h | 9 ++------- > include/linux/iomap.h | 1 - > 4 files changed, 16 insertions(+), 26 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 6a8bc502bc04..88b8a4416dfc 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -4436,7 +4436,6 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx) > struct btrfs_path *path; > struct btrfs_root *root = BTRFS_I(inode)->root; > struct fiemap_cache cache = { 0 }; > - struct fiemap_extent_info *fieinfo = f_ctx->fc_data; > int end = 0; > u64 em_start = 0; > u64 em_len = 0; > @@ -4561,7 +4560,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx) > } else if (em->block_start == EXTENT_MAP_DELALLOC) { > flags |= (FIEMAP_EXTENT_DELALLOC | > FIEMAP_EXTENT_UNKNOWN); > - } else if (fieinfo->fi_extents_max) { > + } else if (f_ctx->fc_extents_max) { > u64 bytenr = em->block_start - > (em->start - em->orig_start); Shouldn't this be earlier, when or just after when we introduce f_ctx->fc_extents_max? > return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0; Can you use a plain old if statment here to make the code a little more readable? if (flags & FIEMAP_EXTENT_LAST) return 1; return 0; > struct fiemap_ctx { > unsigned int fc_flags; /* Flags as passed from user */ > + unsigned int fc_extents_mapped; /* Number of mapped extents */ > + unsigned int fc_extents_max; /* Size of fiemap_extent array */ > void *fc_data; > fiemap_fill_cb fc_cb; > u64 fc_start; This makes me wonder if we shouldn't just have kept the existing structure and massaged it into the new form. The two just look very, very similar..
> > struct fiemap_cache cache = { 0 }; > > - struct fiemap_extent_info *fieinfo = f_ctx->fc_data; > > int end = 0; > > u64 em_start = 0; > > u64 em_len = 0; > > @@ -4561,7 +4560,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx) > > } else if (em->block_start == EXTENT_MAP_DELALLOC) { > > flags |= (FIEMAP_EXTENT_DELALLOC | > > FIEMAP_EXTENT_UNKNOWN); > > - } else if (fieinfo->fi_extents_max) { > > + } else if (f_ctx->fc_extents_max) { > > u64 bytenr = em->block_start - > > (em->start - em->orig_start); > > Shouldn't this be earlier, when or just after when we introduce > f_ctx->fc_extents_max? hmm, not sure if I understand you here, fc_extents_max is added in this very patch, well, actually moved from fiemap_extent_info, to the new fiemap_ctx. > > > return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0; > > Can you use a plain old if statment here to make the code a little > more readable? > > if (flags & FIEMAP_EXTENT_LAST) > return 1; > return 0; Sure, no problem. > > > struct fiemap_ctx { > > unsigned int fc_flags; /* Flags as passed from user */ > > + unsigned int fc_extents_mapped; /* Number of mapped extents */ > > + unsigned int fc_extents_max; /* Size of fiemap_extent array */ > > void *fc_data; > > fiemap_fill_cb fc_cb; > > u64 fc_start; > > This makes me wonder if we shouldn't just have kept the existing > structure and massaged it into the new form. The two just look > very, very similar.. I can give it a try, most of the work is done, so it won't take so long to do write a different version re-using fiemap_extent_info. I'll give it a shot.
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 6a8bc502bc04..88b8a4416dfc 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -4436,7 +4436,6 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx) struct btrfs_path *path; struct btrfs_root *root = BTRFS_I(inode)->root; struct fiemap_cache cache = { 0 }; - struct fiemap_extent_info *fieinfo = f_ctx->fc_data; int end = 0; u64 em_start = 0; u64 em_len = 0; @@ -4561,7 +4560,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx) } else if (em->block_start == EXTENT_MAP_DELALLOC) { flags |= (FIEMAP_EXTENT_DELALLOC | FIEMAP_EXTENT_UNKNOWN); - } else if (fieinfo->fi_extents_max) { + } else if (f_ctx->fc_extents_max) { u64 bytenr = em->block_start - (em->start - em->orig_start); diff --git a/fs/ioctl.c b/fs/ioctl.c index cbc1b21c4f7c..71d11201a06b 100644 --- a/fs/ioctl.c +++ b/fs/ioctl.c @@ -78,17 +78,16 @@ static int ioctl_fibmap(struct file *filp, int __user *p) int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical, u64 phys, u64 len, u32 flags) { - struct fiemap_extent_info *fieinfo = f_ctx->fc_data; struct fiemap_extent extent; - struct fiemap_extent __user *dest = fieinfo->fi_extents_start; + struct fiemap_extent __user *dest = f_ctx->fc_data; /* only count the extents */ - if (fieinfo->fi_extents_max == 0) { - fieinfo->fi_extents_mapped++; + if (f_ctx->fc_extents_max == 0) { + f_ctx->fc_extents_mapped++; return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0; } - if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max) + if (f_ctx->fc_extents_mapped >= f_ctx->fc_extents_max) return 1; if (flags & SET_UNKNOWN_FLAGS) @@ -104,12 +103,12 @@ int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical, extent.fe_length = len; extent.fe_flags = flags; - dest += fieinfo->fi_extents_mapped; + dest += f_ctx->fc_extents_mapped; if (copy_to_user(dest, &extent, sizeof(extent))) return -EFAULT; - fieinfo->fi_extents_mapped++; - if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max) + f_ctx->fc_extents_mapped++; + if (f_ctx->fc_extents_mapped == f_ctx->fc_extents_max) return 1; return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0; } @@ -189,7 +188,6 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg) { struct fiemap fiemap; struct fiemap __user *ufiemap = (struct fiemap __user *) arg; - struct fiemap_extent_info fieinfo = { 0, }; struct inode *inode = file_inode(filp); struct super_block *sb = inode->i_sb; struct fiemap_ctx f_ctx; @@ -210,19 +208,18 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg) if (error) return error; - fieinfo.fi_extents_max = fiemap.fm_extent_count; - fieinfo.fi_extents_start = ufiemap->fm_extents; - + f_ctx.fc_extents_max = fiemap.fm_extent_count; + f_ctx.fc_data = ufiemap->fm_extents; + f_ctx.fc_extents_mapped = 0; f_ctx.fc_flags = fiemap.fm_flags; - f_ctx.fc_data = &fieinfo; f_ctx.fc_start = fiemap.fm_start; f_ctx.fc_len = len; f_ctx.fc_cb = fiemap_fill_usr_extent; if (fiemap.fm_extent_count != 0 && - !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start, - fieinfo.fi_extents_max * sizeof(struct fiemap_extent))) + !access_ok(VERIFY_WRITE, f_ctx.fc_data, + f_ctx.fc_extents_max * sizeof(struct fiemap_extent))) return -EFAULT; if (f_ctx.fc_flags & FIEMAP_FLAG_SYNC) @@ -230,7 +227,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg) error = inode->i_op->fiemap(inode, &f_ctx); fiemap.fm_flags = f_ctx.fc_flags; - fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped; + fiemap.fm_mapped_extents = f_ctx.fc_extents_mapped; if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap))) error = -EFAULT; diff --git a/include/linux/fs.h b/include/linux/fs.h index 9a538bc0dac2..4c6dee908a38 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1691,18 +1691,13 @@ extern bool may_open_dev(const struct path *path); struct fiemap_ctx; -struct fiemap_extent_info { - unsigned int fi_extents_mapped; /* Number of mapped extents */ - unsigned int fi_extents_max; /* Size of fiemap_extent array */ - struct fiemap_extent __user *fi_extents_start; /* Start of - fiemap_extent array */ -}; - typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical, u64 phys, u64 len, u32 flags); struct fiemap_ctx { unsigned int fc_flags; /* Flags as passed from user */ + unsigned int fc_extents_mapped; /* Number of mapped extents */ + unsigned int fc_extents_max; /* Size of fiemap_extent array */ void *fc_data; fiemap_fill_cb fc_cb; u64 fc_start; diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 7ce84be499d6..21643bcf6104 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -9,7 +9,6 @@ #include <linux/mm_types.h> struct address_space; -struct fiemap_extent_info; struct inode; struct iov_iter; struct kiocb;
With the goal of using FIEMAP infra-structure for FIBMAP ioctl, and with the updates being done previously into the FIEMAP interface, the fiemap_extent_info structure is no longer necessary, so, move fi_extents_mapped and fi_extents_max up to fiemap_ctx. and use fiemap_ctx.fc_data to hold a pointer to struct fiemap_extent. Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> --- fs/btrfs/extent_io.c | 3 +-- fs/ioctl.c | 29 +++++++++++++---------------- include/linux/fs.h | 9 ++------- include/linux/iomap.h | 1 - 4 files changed, 16 insertions(+), 26 deletions(-)