On Thu, Jul 22, 2021 at 7:34 AM Eric Biggers <[email protected]> wrote:
>
> On Thu, Jul 22, 2021 at 01:42:05AM -0700, Daeho Jeong wrote:
> > On Wed, Jul 21, 2021 at 6:56 PM Eric Biggers <[email protected]> wrote:
> > >
> > > On Wed, Jul 21, 2021 at 06:40:00PM -0700, Daeho Jeong wrote:
> > > > On Wed, Jul 21, 2021 at 6:15 PM Eric Biggers <[email protected]>
> > > > wrote:
> > > > >
> > > > > On Wed, Jul 21, 2021 at 06:04:22PM -0700, Daeho Jeong wrote:
> > > > > > >
> > > > > > > How f2fs stores the mapping information doesn't matter. That's an
> > > > > > > implementation detail that shouldn't be exposed to userspace.
> > > > > > > The only thing
> > > > > > > that should be exposed is the actual mapping, and for that it
> > > > > > > seems natural to
> > > > > > > report the physical blocks first.
> > > > > > >
> > > > > > > There is no perfect solution for how to handle the remaining
> > > > > > > logical blocks,
> > > > > > > given that the fiemap API was not designed for compressed files,
> > > > > > > but I think we
> > > > > > > should just go with extending the length of the last compressed
> > > > > > > extent in the
> > > > > > > cluster to cover the remaining logical blocks, i.e.:
> > > > > > >
> > > > > > > [0..31]: 2683128..2683159 flag(0x1009) -> merged, encoded,
> > > > > > > last_extent
> > > > > > >
> > > > > > > That's what btrfs does on compressed files.
> > > > > > >
> > > > > > > - Eric
> > > > > >
> > > > > > I also agree that that's an implementation detail that shouldn't be
> > > > > > exposed to userspace.
> > > > > >
> > > > > > I want to make it more clear for better appearance.
> > > > > >
> > > > > > Do you think we have to remove "unwritten" information below? I also
> > > > > > think it might be unnecessary information for the user.
> > > > > > [0..31]: 2683128..2683159 flag(0x1009) -> merged, encoded,
> > > > > > last_extent
> > > > > > (unwritten?)
> > > > >
> > > > > FIEMAP_EXTENT_UNWRITTEN already has a specific meaning; see
> > > > > Documentation/filesystems/fiemap.rst. It means that the data is all
> > > > > zeroes, and
> > > > > the disk space is preallocated but the data hasn't been written to
> > > > > disk yet.
> > > > >
> > > > > In this case, the data is *not* necessarily all zeroes. So I think
> > > > > FIEMAP_EXTENT_UNWRITTEN shouldn't be used here.
> > > > >
> > > > > > Do you want f2fs to print out the info on a cluster basis, even when
> > > > > > the user asks for one block information?
> > > > > > Like
> > > > > > If the user asks for the info of [8..15], f2fs will return the info
> > > > > > of [0..31]?
> > > > >
> > > > > Yes, since that's how FS_IOC_FIEMAP is supposed to work; see
> > > > > Documentation/filesystems/fiemap.rst:
> > > > >
> > > > > All offsets and lengths are in bytes and mirror those on
> > > > > disk. It is
> > > > > valid for an extents logical offset to start before the
> > > > > request or its
> > > > > logical length to extend past the request.
> > > > >
> > > > > (That being said, the f2fs compression+encryption tests I've written
> > > > > don't
> > > > > exercise this case; they only map the whole file at once.)
> > > > >
> > > > > - Eric
> > > >
> > > > My last question is.
> > > > How about a discontinuous cluster like [0..31] maps to discontinuous
> > > > three blocks like physical address 0x4, 0x14 and 0x24.
> > > > I think we have to return three extents for the one logical region
> > > > like the below. What do you think?
> > > > [0..31] -> 0x4 (merged, encoded)
> > > > [0..31] -> 0x14 (merged, encoded)
> > > > [0..31] -> 0x24 (merged, encoded, last_extent)
> > >
> > > No, please don't do that. struct fiemap_extent only has a single length
> > > field,
> > > not separate lengths for fe_logical and fe_physical, so with your
> > > proposal there
> > > would be no way to know how many physical blocks to take from each
> > > extent. It
> > > would be reporting the same part of the file in contradictory ways.
> > >
> > > Like I suggested originally, I think this case should be reported like:
> > >
> > > fe_logical=0 fe_physical=16384 length=4096
> > > fe_logical=4096 fe_physical=81920 length=4096
> > > fe_logical=8192 fe_physical=147456 length=8192
> > >
> >
> > Hi Eric,
> >
> > I think we need to separate one more extent in the example to figure
> > out that the 4 block cluster turned into 3 compressed blocks.
> >
> > fe_logical=0 fe_physical=16384 length=4096
> > fe_logical=4096 fe_physical=81920 length=4096
> > fe_logical=8192 fe_physical=147456 length=4096
> > fe_logical=12288 fe_physical=0 length=4096
> >
> > What do you think?
> >
>
> [+linux-f2fs-devel, not sure why the list was dropped...]
>
> I think that would work too, but what flags would you use in the last entry?
>
> - Eric
I think it matches with FIEMAP_EXTENT_UNWRITTEN. Otherwise, we should
shorten the last extent like below.
fe_logical=0 fe_physical=16384 length=4096
fe_logical=4096 fe_physical=81920 length=4096
fe_logical=8192 fe_physical=147456 length=4096
Unwritten extent - the extent is allocated but its data has not been
initialized. This indicates the extent's data will be all zero if read
through the filesystem but the contents are undefined if read directly from
the device.
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
