All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yu Kuai <yukuai1@huaweicloud.com>
To: Christoph Hellwig <hch@infradead.org>, Yu Kuai <yukuai1@huaweicloud.com>
Cc: tj@kernel.org, josef@toxicpanda.com, axboe@kernel.dk,
	cgroups@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org, yi.zhang@huawei.com,
	"yukuai (C)" <yukuai3@huawei.com>
Subject: Re: [PATCH -next 1/4] blk-iocost: track whether iocg is still online
Date: Mon, 26 Dec 2022 09:10:41 +0800	[thread overview]
Message-ID: <edb84ee2-3dd5-8bb4-4417-ed42744d793f@huaweicloud.com> (raw)
In-Reply-To: <Y6LhD8CEkcgLUJoQ@infradead.org>

Hi, Christoph

在 2022/12/21 18:33, Christoph Hellwig 写道:
> On Sat, Dec 17, 2022 at 11:05:24AM +0800, Yu Kuai wrote:
>> @@ -459,6 +459,8 @@ struct ioc_gq {
>>   	struct blkg_policy_data		pd;
>>   	struct ioc			*ioc;
>>   
>> +	bool online;
> 
> Nit: maybe tab align this field like the fields above it.
> 
>> +static void ioc_pd_offline(struct blkg_policy_data *pd)
>> +{
>> +	struct ioc_gq *iocg = pd_to_iocg(pd);
>> +	struct ioc *ioc = iocg->ioc;
>> +	unsigned long flags;
>> +
>> +	if (ioc) {
> 
> How could ioc be NULL here?
> 

As I explained in another thread.. pd_offline_fn() can be called without
pd_init_fn(), which is a bug from upper layer...

blkcg_activate_policy
  spin_lock_irq(&q->queue_lock);
  list_for_each_entry_reverse(blkg, &q->blkg_list
   pd_alloc_fn(GFP_NOWAIT | __GFP_NOWARN,...) -> failed

   spin_unlock_irq(&q->queue_lock);
   // release queue_lock here is problematic, this will cause
pd_offline_fn called without pd_init_fn.
   pd_alloc_fn(__GFP_NOWARN,...)

Thanks,
Kuai
> .
> 


WARNING: multiple messages have this Message-ID (diff)
From: Yu Kuai <yukuai1-XF6JlduFytWkHkcT6e4Xnw@public.gmane.org>
To: Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
	Yu Kuai <yukuai1-XF6JlduFytWkHkcT6e4Xnw@public.gmane.org>
Cc: tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	josef-DigfWCa+lFGyeJad7bwFQA@public.gmane.org,
	axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	yi.zhang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
	"yukuai (C)" <yukuai3-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Subject: Re: [PATCH -next 1/4] blk-iocost: track whether iocg is still online
Date: Mon, 26 Dec 2022 09:10:41 +0800	[thread overview]
Message-ID: <edb84ee2-3dd5-8bb4-4417-ed42744d793f@huaweicloud.com> (raw)
In-Reply-To: <Y6LhD8CEkcgLUJoQ-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>

Hi, Christoph

ÔÚ 2022/12/21 18:33, Christoph Hellwig дµÀ:
> On Sat, Dec 17, 2022 at 11:05:24AM +0800, Yu Kuai wrote:
>> @@ -459,6 +459,8 @@ struct ioc_gq {
>>   	struct blkg_policy_data		pd;
>>   	struct ioc			*ioc;
>>   
>> +	bool online;
> 
> Nit: maybe tab align this field like the fields above it.
> 
>> +static void ioc_pd_offline(struct blkg_policy_data *pd)
>> +{
>> +	struct ioc_gq *iocg = pd_to_iocg(pd);
>> +	struct ioc *ioc = iocg->ioc;
>> +	unsigned long flags;
>> +
>> +	if (ioc) {
> 
> How could ioc be NULL here?
> 

As I explained in another thread.. pd_offline_fn() can be called without
pd_init_fn(), which is a bug from upper layer...

blkcg_activate_policy
  spin_lock_irq(&q->queue_lock);
  list_for_each_entry_reverse(blkg, &q->blkg_list
   pd_alloc_fn(GFP_NOWAIT | __GFP_NOWARN,...) -> failed

   spin_unlock_irq(&q->queue_lock);
   // release queue_lock here is problematic, this will cause
pd_offline_fn called without pd_init_fn.
   pd_alloc_fn(__GFP_NOWARN,...)

Thanks,
Kuai
> .
> 


  reply	other threads:[~2022-12-26  1:10 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-17  3:05 [PATCH -next 0/4] blk-iocost: make sure parent iocg is exited before child Yu Kuai
2022-12-17  3:05 ` Yu Kuai
2022-12-17  3:05 ` [PATCH -next 1/4] blk-iocost: track whether iocg is still online Yu Kuai
2022-12-17  3:05   ` Yu Kuai
2022-12-21 10:33   ` Christoph Hellwig
2022-12-26  1:10     ` Yu Kuai [this message]
2022-12-26  1:10       ` Yu Kuai
2022-12-17  3:05 ` [PATCH -next 2/4] blk-iocost: don't throttle bio if iocg is offlined Yu Kuai
2022-12-19 21:28   ` Tejun Heo
2022-12-20  9:38     ` Yu Kuai
2022-12-20  9:38       ` Yu Kuai
2022-12-17  3:05 ` [PATCH -next 3/4] blk-iocost: dispatch all throttled bio in ioc_pd_offline Yu Kuai
2022-12-19 21:31   ` Tejun Heo
2022-12-19 21:31     ` Tejun Heo
2022-12-17  3:05 ` [PATCH -next 4/4] blk-iocost: guarantee the exit order of iocg Yu Kuai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=edb84ee2-3dd5-8bb4-4417-ed42744d793f@huaweicloud.com \
    --to=yukuai1@huaweicloud.com \
    --cc=axboe@kernel.dk \
    --cc=cgroups@vger.kernel.org \
    --cc=hch@infradead.org \
    --cc=josef@toxicpanda.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=yi.zhang@huawei.com \
    --cc=yukuai3@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.