linux-kernel - Re: [PATCH v3 3/4] block: add support for notifications

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZnxyQgYyM31Xq2xV@makrotopia.org>
Date: Wed, 26 Jun 2024 20:55:46 +0100
From: Daniel Golle <daniel@...rotopia.org>
To: Jens Axboe <axboe@...nel.dk>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Ulf Hansson <ulf.hansson@...aro.org>,
	Hauke Mehrtens <hauke@...ke-m.de>, Felix Fietkau <nbd@....name>,
	Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
	Dave Chinner <dchinner@...hat.com>, Jan Kara <jack@...e.cz>,
	Christian Brauner <brauner@...nel.org>,
	Thomas Weißschuh <linux@...ssschuh.net>,
	Al Viro <viro@...iv.linux.org.uk>,
	Li Lingfeng <lilingfeng3@...wei.com>,
	Christian Heusel <christian@...sel.eu>,
	Min Li <min15.li@...sung.com>, Avri Altman <avri.altman@....com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Hannes Reinecke <hare@...e.de>,
	Mikko Rapeli <mikko.rapeli@...aro.org>, Yeqi Fu <asuk4.q@...il.com>,
	Victor Shih <victor.shih@...esyslogic.com.tw>,
	Christophe JAILLET <christophe.jaillet@...adoo.fr>,
	Li Zhijian <lizhijian@...itsu.com>,
	"Ricardo B. Marliere" <ricardo@...liere.net>,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-mmc@...r.kernel.org, linux-block@...r.kernel.org
Subject: Re: [PATCH v3 3/4] block: add support for notifications

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