Hi Jens,
thanks a lot for the review!
On Wed, Jun 26, 2024 at 01:46:50PM -0600, Jens Axboe wrote:
> On 6/25/24 8:51 PM, Daniel Golle wrote:
> > +static int blk_call_notifier_add(struct device *dev)
> > +{
> > + struct blk_device_list *new_blkdev;
> > +
> > + new_blkdev = kmalloc(sizeof(*new_blkdev), GFP_KERNEL);
> > + if (!new_blkdev)
> > + return -ENOMEM;
> > +
> > + new_blkdev->dev = dev;
> > + mutex_lock(&blk_notifier_lock);
> > + list_add_tail(&new_blkdev->list, &blk_devices);
> > + raw_notifier_call_chain(&blk_notifier_list, BLK_DEVICE_ADD, dev);
> > + mutex_unlock(&blk_notifier_lock);
> > +
> > + return 0;
> > +}
>
> Nit: redundant newline.
I'll remove the newline before the 'return' statement then, right?
>
> > +device_initcall(blk_notifications_init);
> > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > index b2f1362c4681..8d22ba03e3e1 100644
> > --- a/include/linux/blkdev.h
> > +++ b/include/linux/blkdev.h
> > @@ -1687,4 +1687,12 @@ static inline bool bdev_can_atomic_write(struct block_device *bdev)
> >
> > #define DEFINE_IO_COMP_BATCH(name) struct io_comp_batch name = { }
> >
> > +
> > +#ifdef CONFIG_BLOCK_NOTIFIERS
>
> #if defined(CONFIG_BLOCK_NOTIFIERS)
>
> > +#define BLK_DEVICE_ADD 1
> > +#define BLK_DEVICE_REMOVE 2
> > +void blk_register_notify(struct notifier_block *nb);
> > +void blk_unregister_notify(struct notifier_block *nb);
> > +#endif
>
> Surely these helpers should have a !CONFIG_BLOCK_NOTIFIERS failure case
> definition? Either that, or dummies. As it stands, any caller would need
> to check if it's enabled or not.
Makes sense. I'll add dummies to the header and always define
the macros for notification types.
Note that what I'm planning to do is to have the block nvmem provider
select CONFIG_BLOCK_NOTIFIERS in Kconfig, as without that it simply
won't work at all.
Cheers
Daniel
