Re: ioctl FIBMAP for dax gone in v4.17-rc1

Re: ioctl FIBMAP for dax gone in v4.17-rc1

[Christoph Hellwig]

On Tue, Apr 17, 2018 at 10:40:59PM +0800, Xiong Zhou wrote:
> We got these in v4.17-rc1:
> 6e2608d xfs, dax: introduce xfs_dax_aops
> fb094c9 ext2, dax: introduce ext2_dax_aops
> 5f0663b ext4, dax: introduce ext4_dax_aops
> 
> And we don't have ->bmap call in these aops, which may lead
> to the ioctl call failure.
> 
> Do we have any plan of adding/supporting it ?
> 
> xfstests generic/223 covers this issue. If we are not going
> to support this call for dax, we need to fix the testcase.

Not supporting ->bmap is a good thing as it is hightly dangerous.

The t_stripetest tool used by generic/223 should really be
rewritten to suppot FIEMAP.

Re: ioctl FIBMAP for dax gone in v4.17-rc1

[Dan Williams]

On Tue, Apr 17, 2018 at 9:10 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Apr 17, 2018 at 10:40:59PM +0800, Xiong Zhou wrote:
>> We got these in v4.17-rc1:
>> 6e2608d xfs, dax: introduce xfs_dax_aops
>> fb094c9 ext2, dax: introduce ext2_dax_aops
>> 5f0663b ext4, dax: introduce ext4_dax_aops
>>
>> And we don't have ->bmap call in these aops, which may lead
>> to the ioctl call failure.
>>
>> Do we have any plan of adding/supporting it ?
>>
>> xfstests generic/223 covers this issue. If we are not going
>> to support this call for dax, we need to fix the testcase.
>
> Not supporting ->bmap is a good thing as it is hightly dangerous.

I take this to mean "don't fix, it is another casualty of dax being
experimental and it won't be coming back". I can get on board with
that.

Otherwise, I was about to send a series adding bmap to {xfs,ext2,ext4}_dax_ops.

Re: ioctl FIBMAP for dax gone in v4.17-rc1

[Darrick J. Wong]

On Tue, Apr 17, 2018 at 09:53:47AM -0700, Dan Williams wrote:
> On Tue, Apr 17, 2018 at 9:10 AM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Tue, Apr 17, 2018 at 10:40:59PM +0800, Xiong Zhou wrote:
> >> We got these in v4.17-rc1:
> >> 6e2608d xfs, dax: introduce xfs_dax_aops
> >> fb094c9 ext2, dax: introduce ext2_dax_aops
> >> 5f0663b ext4, dax: introduce ext4_dax_aops
> >>
> >> And we don't have ->bmap call in these aops, which may lead
> >> to the ioctl call failure.
> >>
> >> Do we have any plan of adding/supporting it ?
> >>
> >> xfstests generic/223 covers this issue. If we are not going
> >> to support this call for dax, we need to fix the testcase.
> >
> > Not supporting ->bmap is a good thing as it is hightly dangerous.
> 
> I take this to mean "don't fix, it is another casualty of dax being
> experimental and it won't be coming back". I can get on board with
> that.
> 
> Otherwise, I was about to send a series adding bmap to {xfs,ext2,ext4}_dax_ops.

Frankly I'd rather see the swapfile code learn how to iomap and then we
can get rid of bmap in xfs entirely.

--D

> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ioctl FIBMAP for dax gone in v4.17-rc1

[Dan Williams]

On Tue, Apr 17, 2018 at 9:57 AM, Darrick J. Wong
<darrick.wong@oracle.com> wrote:
> On Tue, Apr 17, 2018 at 09:53:47AM -0700, Dan Williams wrote:
>> On Tue, Apr 17, 2018 at 9:10 AM, Christoph Hellwig <hch@infradead.org> wrote:
>> > On Tue, Apr 17, 2018 at 10:40:59PM +0800, Xiong Zhou wrote:
>> >> We got these in v4.17-rc1:
>> >> 6e2608d xfs, dax: introduce xfs_dax_aops
>> >> fb094c9 ext2, dax: introduce ext2_dax_aops
>> >> 5f0663b ext4, dax: introduce ext4_dax_aops
>> >>
>> >> And we don't have ->bmap call in these aops, which may lead
>> >> to the ioctl call failure.
>> >>
>> >> Do we have any plan of adding/supporting it ?
>> >>
>> >> xfstests generic/223 covers this issue. If we are not going
>> >> to support this call for dax, we need to fix the testcase.
>> >
>> > Not supporting ->bmap is a good thing as it is hightly dangerous.
>>
>> I take this to mean "don't fix, it is another casualty of dax being
>> experimental and it won't be coming back". I can get on board with
>> that.
>>
>> Otherwise, I was about to send a series adding bmap to {xfs,ext2,ext4}_dax_ops.
>
> Frankly I'd rather see the swapfile code learn how to iomap and then we
> can get rid of bmap in xfs entirely.

Right, all I am trying to determine is if this is a regression or not.
It seems not supporting bmap on dax going forward is a feature not a
bug.

Re: ioctl FIBMAP for dax gone in v4.17-rc1

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1681 bytes --]

On Apr 17, 2018, at 10:57 AM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> On Tue, Apr 17, 2018 at 09:53:47AM -0700, Dan Williams wrote:
>> On Tue, Apr 17, 2018 at 9:10 AM, Christoph Hellwig <hch@infradead.org> wrote:
>>> On Tue, Apr 17, 2018 at 10:40:59PM +0800, Xiong Zhou wrote:
>>>> We got these in v4.17-rc1:
>>>> 6e2608d xfs, dax: introduce xfs_dax_aops
>>>> fb094c9 ext2, dax: introduce ext2_dax_aops
>>>> 5f0663b ext4, dax: introduce ext4_dax_aops
>>>> 
>>>> And we don't have ->bmap call in these aops, which may lead
>>>> to the ioctl call failure.
>>>> 
>>>> Do we have any plan of adding/supporting it ?
>>>> 
>>>> xfstests generic/223 covers this issue. If we are not going
>>>> to support this call for dax, we need to fix the testcase.
>>> 
>>> Not supporting ->bmap is a good thing as it is hightly dangerous.
>> 
>> I take this to mean "don't fix, it is another casualty of dax being
>> experimental and it won't be coming back". I can get on board with
>> that.
>> 
>> Otherwise, I was about to send a series adding bmap to {xfs,ext2,ext4}_dax_ops.
> 
> Frankly I'd rather see the swapfile code learn how to iomap and then we
> can get rid of bmap in xfs entirely.

Is anyone still using LILO to boot?  It needed FIBMAP support to map the
kernel image for booting.  I don't know if Grub needs FIBMAP support for
the early boot stages or not (it has minimal filesystem support in the
later stages), but it would be a shame if it wasn't possible to boot an
all-NVRAM system as a result of a missing ->bmap() method.  Alternately,
convince the Grub folks to use FIEMAP if that is available...

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: ioctl FIBMAP for dax gone in v4.17-rc1

[Dan Williams]

On Tue, Apr 17, 2018 at 10:40 AM, Andreas Dilger <adilger@dilger.ca> wrote:
> On Apr 17, 2018, at 10:57 AM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
>>
>> On Tue, Apr 17, 2018 at 09:53:47AM -0700, Dan Williams wrote:
>>> On Tue, Apr 17, 2018 at 9:10 AM, Christoph Hellwig <hch@infradead.org> wrote:
>>>> On Tue, Apr 17, 2018 at 10:40:59PM +0800, Xiong Zhou wrote:
>>>>> We got these in v4.17-rc1:
>>>>> 6e2608d xfs, dax: introduce xfs_dax_aops
>>>>> fb094c9 ext2, dax: introduce ext2_dax_aops
>>>>> 5f0663b ext4, dax: introduce ext4_dax_aops
>>>>>
>>>>> And we don't have ->bmap call in these aops, which may lead
>>>>> to the ioctl call failure.
>>>>>
>>>>> Do we have any plan of adding/supporting it ?
>>>>>
>>>>> xfstests generic/223 covers this issue. If we are not going
>>>>> to support this call for dax, we need to fix the testcase.
>>>>
>>>> Not supporting ->bmap is a good thing as it is hightly dangerous.
>>>
>>> I take this to mean "don't fix, it is another casualty of dax being
>>> experimental and it won't be coming back". I can get on board with
>>> that.
>>>
>>> Otherwise, I was about to send a series adding bmap to {xfs,ext2,ext4}_dax_ops.
>>
>> Frankly I'd rather see the swapfile code learn how to iomap and then we
>> can get rid of bmap in xfs entirely.
>
> Is anyone still using LILO to boot?  It needed FIBMAP support to map the
> kernel image for booting.  I don't know if Grub needs FIBMAP support for
> the early boot stages or not (it has minimal filesystem support in the
> later stages), but it would be a shame if it wasn't possible to boot an
> all-NVRAM system as a result of a missing ->bmap() method.  Alternately,
> convince the Grub folks to use FIEMAP if that is available...

For boot the recommendation is to use the BTT
(block-translation-table) to provide a sector-atomic block device for
the system-partition. Such a configuration does not support DAX mode
operation, so there should be no conflict. The EFI system partition is
also FAT in most cases which does not support DAX.

Re: ioctl FIBMAP for dax gone in v4.17-rc1

[Darrick J. Wong]

On Tue, Apr 17, 2018 at 11:40:00AM -0600, Andreas Dilger wrote:
> On Apr 17, 2018, at 10:57 AM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> > 
> > On Tue, Apr 17, 2018 at 09:53:47AM -0700, Dan Williams wrote:
> >> On Tue, Apr 17, 2018 at 9:10 AM, Christoph Hellwig <hch@infradead.org> wrote:
> >>> On Tue, Apr 17, 2018 at 10:40:59PM +0800, Xiong Zhou wrote:
> >>>> We got these in v4.17-rc1:
> >>>> 6e2608d xfs, dax: introduce xfs_dax_aops
> >>>> fb094c9 ext2, dax: introduce ext2_dax_aops
> >>>> 5f0663b ext4, dax: introduce ext4_dax_aops
> >>>> 
> >>>> And we don't have ->bmap call in these aops, which may lead
> >>>> to the ioctl call failure.
> >>>> 
> >>>> Do we have any plan of adding/supporting it ?
> >>>> 
> >>>> xfstests generic/223 covers this issue. If we are not going
> >>>> to support this call for dax, we need to fix the testcase.
> >>> 
> >>> Not supporting ->bmap is a good thing as it is hightly dangerous.
> >> 
> >> I take this to mean "don't fix, it is another casualty of dax being
> >> experimental and it won't be coming back". I can get on board with
> >> that.
> >> 
> >> Otherwise, I was about to send a series adding bmap to {xfs,ext2,ext4}_dax_ops.
> > 
> > Frankly I'd rather see the swapfile code learn how to iomap and then we
> > can get rid of bmap in xfs entirely.
> 
> Is anyone still using LILO to boot?  It needed FIBMAP support to map the
> kernel image for booting.  I don't know if Grub needs FIBMAP support for
> the early boot stages or not (it has minimal filesystem support in the
> later stages), but it would be a shame if it wasn't possible to boot an
> all-NVRAM system as a result of a missing ->bmap() method.  Alternately,
> convince the Grub folks to use FIEMAP if that is available...

grub2 usually uses its own fs drivers (for better or for worse) to read
the fs.  The blockchaining mode tries FIEMAP and falls back to FIBMAP if
not available.

syslinux does FIEMAP w/ FIBMAP fallback.

lilo & elilo still use FIBMAP only, but lilo hasn't seen a release since
2015 and elilo 2013.  I prefer not to add FIBMAP support in XFS-DAX for
the singular case of booting off pmem via lilo.

(Sorry Dave...)

--D

> 
> Cheers, Andreas
> 
> 
> 
> 
> 

Re: ioctl FIBMAP for dax gone in v4.17-rc1

[Theodore Y. Ts'o]

On Tue, Apr 17, 2018 at 10:50:07AM -0700, Darrick J. Wong wrote:
> syslinux does FIEMAP w/ FIBMAP fallback.
> 
> lilo & elilo still use FIBMAP only, but lilo hasn't seen a release since
> 2015 and elilo 2013.  I prefer not to add FIBMAP support in XFS-DAX for
> the singular case of booting off pmem via lilo.

It shouldn't be that hard to write a userspace emulation layer which
exports a FIBMAP-like functional interface but gets the information
from the kernel using the FIMAP ioctl....

					- Ted

Re: ioctl FIBMAP for dax gone in v4.17-rc1

[Dave Chinner]

On Tue, Apr 17, 2018 at 10:50:07AM -0700, Darrick J. Wong wrote:
> On Tue, Apr 17, 2018 at 11:40:00AM -0600, Andreas Dilger wrote:
> > On Apr 17, 2018, at 10:57 AM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> > > 
> > > On Tue, Apr 17, 2018 at 09:53:47AM -0700, Dan Williams wrote:
> > >> On Tue, Apr 17, 2018 at 9:10 AM, Christoph Hellwig <hch@infradead.org> wrote:
> > >>> On Tue, Apr 17, 2018 at 10:40:59PM +0800, Xiong Zhou wrote:
> > >>>> We got these in v4.17-rc1:
> > >>>> 6e2608d xfs, dax: introduce xfs_dax_aops
> > >>>> fb094c9 ext2, dax: introduce ext2_dax_aops
> > >>>> 5f0663b ext4, dax: introduce ext4_dax_aops
> > >>>> 
> > >>>> And we don't have ->bmap call in these aops, which may lead
> > >>>> to the ioctl call failure.
> > >>>> 
> > >>>> Do we have any plan of adding/supporting it ?
> > >>>> 
> > >>>> xfstests generic/223 covers this issue. If we are not going
> > >>>> to support this call for dax, we need to fix the testcase.
> > >>> 
> > >>> Not supporting ->bmap is a good thing as it is hightly dangerous.
> > >> 
> > >> I take this to mean "don't fix, it is another casualty of dax being
> > >> experimental and it won't be coming back". I can get on board with
> > >> that.
> > >> 
> > >> Otherwise, I was about to send a series adding bmap to {xfs,ext2,ext4}_dax_ops.
> > > 
> > > Frankly I'd rather see the swapfile code learn how to iomap and then we
> > > can get rid of bmap in xfs entirely.
> > 
> > Is anyone still using LILO to boot?

Yup. I do.

[ because grub has a history of corrupting filesystems on my
machines with it's obnoxious "probe for bootable OS images on update
by attempting to mount every block device in the system" behaviour. ]

> > It needed FIBMAP support to map the
> > kernel image for booting.

There's a difference between FIBMAP and ->bmap. One's a syscall
parameter under ABI constraints and the other is an internal
implementation.

We have to keep supporting FIBMAP forever, but we can re-implement
it internally using ->fiemap pretty damn easily - just replace the
bmap() implementation with a ->fiemap call and replace all the
direct ->bmap() calls with bmap(). Hence FIBMAP will continue to
work on all filesystems (including DAX!) and we get the ->bmap
crap out of the kernel.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com