> > 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.
--
Carlos
