On Mon, Mar 21, 2016 at 01:08:29PM +0100, Carlos Maiolino wrote: > Hi, > > Good news about this interface, I just have a small suggestion in this patch: > > On Thu, Mar 17, 2016 at 10:30:29AM -0400, Brian Foster wrote: > > From: Mike Snitzer <snitzer@xxxxxxxxxx> > > > > Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx> > > --- > > fs/block_dev.c | 20 ++++++++++++++++++++ > > include/linux/blkdev.h | 5 +++++ > > 2 files changed, 25 insertions(+) > > > > diff --git a/fs/block_dev.c b/fs/block_dev.c > > index 826b164..375a2e4 100644 > > --- a/fs/block_dev.c > > +++ b/fs/block_dev.c > > @@ -497,6 +497,26 @@ long bdev_direct_access(struct block_device *bdev, struct blk_dax_ctl *dax) > > } > > EXPORT_SYMBOL_GPL(bdev_direct_access); > > > > +int blk_reserve_space(struct block_device *bdev, sector_t nr_sects) > > +{ > > + const struct block_device_operations *ops = bdev->bd_disk->fops; > > + > > + if (!ops->reserve_space) > > + return -EOPNOTSUPP; > > + return ops->reserve_space(bdev, nr_sects); > > +} > > +EXPORT_SYMBOL_GPL(blk_reserve_space); > > Wouldn't be better to have this function name standardized accordingly to the > next one? Something like blk_set_reserved_space() maybe? Personally I see no point in wrappers like this. We don't add wrappers for ops methods in any other layers of the stack, filesystems are quite capable of checking if the method is available directly, so it seems pretty pointless to me... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html