On Wed, May 09, 2018 at 09:46:28AM -0700, Darrick J. Wong wrote:
> On Wed, May 09, 2018 at 09:48:07AM +0200, Christoph Hellwig wrote:
> > This adds a simple iomap-based implementation of the legacy ->bmap
> > interface. Note that we can't easily add checks for rt or reflink
> > files, so these will have to remain in the callers. This interface
> > just needs to die..
>
> You /can/ check these...
>
> if (iomap->bdev != inode->i_sb->s_bdev)
> return 0;
> if (iomap->flags & IOMAP_F_SHARED)
> return 0;
The latter only checks for a shared extent, not a file with possibly
shared extents. I'd rather keep the check for a file with possible
shared extents.
> > +static loff_t
> > +iomap_bmap_actor(struct inode *inode, loff_t pos, loff_t length,
> > + void *data, struct iomap *iomap)
> > +{
> > + sector_t *bno = data;
> > +
> > + if (iomap->type == IOMAP_MAPPED)
> > + *bno = (iomap->addr + pos - iomap->offset) >> inode->i_blkbits;
>
> Does this need to be careful w.r.t. overflow on systems where sector_t
> is a 32-bit unsigned long?
>
> Also, ioctl_fibmap() typecasts the returned sector_t to an int, which
> also seems broken. I agree the interface needs to die, but ioctls take
> a long time to deprecate.
Not much we can do about the interface.
