Re: [PATCH v6 2/4] block: add blk_interposer

From: Christoph Hellwig
Date: Wed Mar 10 2021 - 05:05:53 EST


On Wed, Mar 10, 2021 at 07:53:13AM +0300, Sergei Shtepa wrote:
> > Please avoid the overly long line.
> >
> > > + int ret = 0;
> > > +
> > > + if (WARN_ON(!interposer))
> >
> > WARN_ON_ONCE?
>
> This function should be called quite rarely, and the absence of the interposer
> parameter indicates that the function is being used incorrectly.
> I would like to see this warning every time.

Yes. Most kernel code would in fact just remove the check entirely
and let the kernel crash to indicate this. Maybe that is an even
better option for such a grave API usage mistake.

> > > +struct bdev_interposer {
> > > + ip_submit_bio_t ip_submit_bio;
> > > + struct block_device *bdev;
> >
> > Do we need the ip_ prefix here? Also we probably don't really the
> > the typedef for the function pointer.
>
> Ok. Maybe submit_bio_hook would be better? or submit_bio_interposer.

Or interpose_bio?