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

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: Windows password security audit tool. GUI, reports in PDF.

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

Date: Tue, 18 Jun 2024 15:58:54 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Christoph Hellwig <hch@...radead.org>, Yu Kuai <yukuai1@...weicloud.com>
Cc: axboe@...nel.dk, tj@...nel.org, gregkh@...uxfoundation.org,
 bvanassche@....org, josef@...icpanda.com, lizefan.x@...edance.com,
 hannes@...xchg.org, linux-block@...r.kernel.org,
 linux-kernel@...r.kernel.org, cgroups@...r.kernel.org, yi.zhang@...wei.com,
 yangerkun@...wei.com, "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH RFC v2 3/7] block: export some API

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

> 
> .
>