Re: [PATCH RFC v2 3/7] block: export some API

[Yu Kuai]

Hi,

在 2024/06/18 15:14, Christoph Hellwig 写道:
> On Tue, Jun 18, 2024 at 11:17:47AM +0800, Yu Kuai wrote:
> >   bool blkcg_debug_stats = false;
> > +EXPORT_SYMBOL_GPL(blkcg_debug_stats);
>
> exporting variables is lways a bad idea.a

Yes, export variable is the easiest way here. Will it be acceptable to
make this variable static and add a helper to access it?

> >   
> >   static DEFINE_RAW_SPINLOCK(blkg_stat_lock);
> >   
> > @@ -688,6 +689,7 @@ const char *blkg_dev_name(struct blkcg_gq *blkg)
> >   		return NULL;
> >   	return bdi_dev_name(blkg->q->disk->bdi);
> >   }
> > +EXPORT_SYMBOL_GPL(blkg_dev_name);
>
> And this helper is just horribly to be honest.  The bdi_dev_name
> should not be used anywhere in block code, and something like this
> certainly should not be exported even if we have to keep it for
> legacy reasons.

And I can reuse __blkg_prfill_u64() for iocost like bfq did, then this
can be removed.

> >   /**
> >    * blkcg_print_blkgs - helper for printing per-blkg data
> > @@ -815,6 +817,7 @@ int blkg_conf_open_bdev(struct blkg_conf_ctx *ctx)
> >   	ctx->bdev = bdev;
> >   	return 0;
> >   }
> > +EXPORT_SYMBOL_GPL(blkg_conf_open_bdev);
>
> This again is a horrible function papeing over blk-cgroup design
> mistakes.  I absolutely should not be exported.

There is already bigger helper blkg_conf_prep() exported, as used by bfq
for a long time already, and this helper is introduced for code
optimization, as iocost configuration doesn't need to access the blkg.

Perhaps this should be another discussion about "design mistakes",
however, I'm not sure why. :( Do you have previous discussions?

Thanks,
Kuai

> .