Re: [RFC PATCH 1/9] block: add block_device_operations methods to set and get reserved space

Dave Chinner
2016-03-21T21:53:41Z

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@redhat.com>
> >
> > Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> > ---
> > 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@fromorbit.com