linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 0/2] ->bmap interface retirement
@ 2018-09-05 13:57 Carlos Maiolino
  2018-09-05 13:57 ` [PATCH 1/2] fs: Replace direct ->bmap calls by bmap() Carlos Maiolino
  2018-09-05 13:57 ` [PATCH 2/2] Use fiemap internal infra-structure to handle FIBMAP Carlos Maiolino
  0 siblings, 2 replies; 17+ messages in thread
From: Carlos Maiolino @ 2018-09-05 13:57 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hch, sandeen, david

Hi,

This series aims to deprecate the ->bmap interface, by using FIEMAP
infra-structure to handle FIBMAP calls. It doesn't change the FIBMAP api, so the
implementation is supposed to be transparent for users.

Although this patchset is working as I expected and it survived a few xfstests
runs, I have some questions about the way I implemented it that I'd like to
discuss, so, I'm tagging it as RFC.

The first patch is relatively simple, just replacing direct ->bmap calls by
bmap() function (as suggested by Dave Chinner), the second patch actually
changes bmap() function to call ->fiemap when available, falling back to ->bmap
when ->fiemap isn't implemented. So, in a (hopefully) not so far future, we can
get rid of the whole ->bmap interface.

These are the main points where I've doubts about:

I implemented it reutilizing the same infra-structure used by userspace FIEMAP
calls to avoid code duplication if possible, the problem is, to setup the struct
fiemap_extent_info to pass it to ->fiemap, I need to use (__force) casting, so
the compiler doesn't complain about the
"struct fiemap_extent __user *fi_extents_start" into it.
I saw a few other places in kernel doing something similar, so, that's why I
thought it would be ok to use such strategy.

I added a new FIEMAP flag (FIEMAP_KERNEL_FIBMAP), to signal the ->fiemap call
is supposed to be internal only and fiemap_fill_next_extent() can do a simple
memcpy() instead of a copy_to_user(). I added this flag to include/uapi, but I
suppose kernel only flags are not supposed to be in uapi, so, suggestions to
where I could place it are welcome.

Different filesystems will answer FIEMAP calls in slightly different ways. when
the requested range is somewhere in the middle of the extent, some filesystems
will set the map starting at the beginning of the extent (like ext4 for
example), and some (the ones based on iomap for example), will set the map
starting at the offset requested. So, to comply with both cases, bmap() checks
the logical offset returned by the filesystem, and take the appropriate action
if the filesystem returns the beginning of the extent, or not.

What you guys think?
Cheers.

Carlos Maiolino (2):
  fs: Replace direct ->bmap calls by bmap()
  Use fiemap internal infra-structure to handle FIBMAP

 fs/cachefiles/rdwr.c        |  5 ++---
 fs/ecryptfs/mmap.c          |  5 ++---
 fs/inode.c                  | 38 +++++++++++++++++++++++++++++++++++++-
 fs/ioctl.c                  | 25 +++++++++++++++++--------
 include/uapi/linux/fiemap.h |  1 +
 5 files changed, 59 insertions(+), 15 deletions(-)

-- 
2.14.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-09-26 19:47 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-05 13:57 [PATCH RFC 0/2] ->bmap interface retirement Carlos Maiolino
2018-09-05 13:57 ` [PATCH 1/2] fs: Replace direct ->bmap calls by bmap() Carlos Maiolino
2018-09-05 14:23   ` Eric Sandeen
2018-09-05 18:55     ` Christoph Hellwig
2018-09-06  8:04       ` Carlos Maiolino
2018-09-06  8:03     ` Carlos Maiolino
2018-09-05 13:57 ` [PATCH 2/2] Use fiemap internal infra-structure to handle FIBMAP Carlos Maiolino
2018-09-05 14:31   ` Matthew Wilcox
2018-09-05 18:56     ` Christoph Hellwig
2018-09-06  8:31       ` Carlos Maiolino
2018-09-10  7:50         ` Christoph Hellwig
2018-09-10 10:31           ` Carlos Maiolino
2018-09-26 13:34           ` Carlos Maiolino
2018-09-06  8:18     ` Carlos Maiolino
2018-09-05 14:40   ` Eric Sandeen
2018-09-06  8:12     ` Carlos Maiolino
2018-09-06 15:53       ` Eric Sandeen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).