On Mon 11-01-16 16:07:29, Bob Peterson wrote:
> This patch moves the iomap infrastructure from its current location in
> exportfs.h to a new iomap.h. This may be used not only by nfs, but also by
> other file systems. This also adds an iomap function call to the
> address_space_operations. This will facilitate future improvements such
> as a more efficient fiemap for holey files. Hopefully it will one day be
> used for multipage writes as well.
>
> Signed-off-by: Bob Peterson <rpeterso@xxxxxxxxxx>
Thanks for the patch and it's good someone is picking this up!
> @@ -403,6 +405,8 @@ struct address_space_operations {
> unsigned long);
> void (*is_dirty_writeback) (struct page *, bool *, bool *);
> int (*error_remove_page)(struct address_space *, struct page *);
> + int (*iomap)(struct address_space *mapping, loff_t pos,
> + ssize_t length, struct iomap *iomap, int cmd);
>
> /* swapfile support */
> int (*swap_activate)(struct swap_info_struct *sis, struct file *file,
I think the interface is good but I would not add iomap method to
address_space_operations. Instead, similarly to how get_blocks() callbacks
are used, we can pass appropriate iomap() function to the generic helper
which then calls it in appropriate places. Filesystems would like to use
different mapping functions for different contexts.
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> new file mode 100644
> index 0000000..8da4c1e
> --- /dev/null
> +++ b/include/linux/iomap.h
> @@ -0,0 +1,39 @@
> +#ifndef _IOMAP_H
> +#define _IOMAP_H
> +
> +/* ->iomap a_op command types */
> +#define IOMAP_READ 0x01 /* read the current mapping starting at the
> + given position, trimmed to a maximum length.
> + FS's should use this to obtain and lock
> + resources within this range */
> +#define IOMAP_RESERVE 0x02 /* reserve space for an allocation that spans
> + the given iomap */
> +#define IOMAP_ALLOCATE 0x03 /* allocate space in a given iomap - must have
> + first been reserved */
> +#define IOMAP_UNRESERVE 0x04 /* return unused reserved space for the given
> + iomap and used space. This will always be
> + called after a IOMAP_READ so as to allow the
> + FS to release held resources. */
And IMHO trying to fit all different things various filesystems might want
to do to a range in a file into a single 'cmd' argument is a way to hell.
We would soon have approximately bazzilion of these cmds and different
filesystem would have slightly different ideas how to react to these
commands. So let the filesystem pass appropriate iomap function for the
current context and be done with that. Maybe a flag whether this is for
read or write (similarly as map_blocks() in export_operations have it)
makes sense to avoid duplication in the most common cases but I'm looking
very sceptically at anything more sophisticated.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
--
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
