On Thu, May 10, 2018 at 08:42:50AM +0200, Christoph Hellwig wrote: > 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. <nod> > > > +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. Yes, the interface is fubar, but if file /foo maps to block 8589934720 then do we return the truncated result 128? --D -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html