----- Original Message ----- | On Mon, May 23, 2016 at 04:09:26PM -0400, Bob Peterson wrote: | > I've been looking at this again. Where are the calls to the fs-specific | > bits | > for fiemap? | | In the iomap_ops structure passed to iomap_fiemap. | | > It looks like iomap_fiemap calls iomap_apply, which calls | > iomap_fiemap_actor, but that doesn't call any ops->iomap_get_iomap or | > similar. | > It calls the iomap_begin (which BTW has a comment that says "Execute a | > iomap | > write" which is probably wrong and should be more generic, as for cases | > like | > fiemap) and it calls iomap_end. But it never calls an fs-specific actor | > anywhere. Am I missing something? My earlier version passed in the actor | > function, as per Dave Chinner's request, but yours doesn't. | | The iomap_begin callback is where you do the mapping. the iomap_end | callback does any required cleanup, which in case of GFS2 probably | would be dropping the cluster lock protecting the mapping. | Okay, got it. So a couple things: 1. I verified that the vfs bits of the patch set work properly for GFS2 using a modified iomap-based fiemap. And it's fast. 2. I'm not sure I like the fact that instead of begin->main->end it is essentially begin->end with begin doing all the work. It works, and it's better than we have today. But I'd prefer either renaming the first function from "iomap_begin" to something that indicates it's more than just a precursor to the actual function? Or else split it into begin->main->end? I was kinda hoping to pass in the iomap_actor somehow. It's not a tragic loss, but the way I've got the gfs2 function coded, the begin function does: locking plus the main functionality, and all the fiemap_end function basically does is unlock. 3. I had to do something like this to get "make menuconfig" to work: index 276fcfb..daa129c 100644 --- a/fs/Kconfig +++ b/fs/Kconfig @@ -11,7 +11,7 @@ config DCACHE_WORD_ACCESS if BLOCK config FS_IOMAP - bool + bool "File IOMAP Support" 4. I don't suppose you could split this patch set up so that the vfs bits are independent, so someone like Al Viro could grab them rather than getting pulled along size the xfs-specific bits? (Which is essentially what I've done for GFS2; I did not drag in all the xfs bits). Regards, Bob Peterson Red Hat File Systems -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
