Received-SPF: pass (google.com: domain of linux-kernel+bounces-4561-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Subject: Re: [PATCH v2 1/2] md: Fix overflow in is_mddev_idle
To: Song Liu <song@kernel.org>, Yu Kuai <yukuai1@huaweicloud.com>
Cc: linan666@huaweicloud.com, axboe@kernel.dk, linux-raid@vger.kernel.org,
 linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
 yi.zhang@huawei.com, houtao1@huawei.com, yangerkun@huawei.com,
 "yukuai (C)" <yukuai3@huawei.com>
References: <20231215013931.3329455-1-linan666@huaweicloud.com>
 <20231215013931.3329455-2-linan666@huaweicloud.com>
 <CAPhsuW6VTvXy3L9CUhTrSC3+_-_n9FDVrtdzQ7SWWkukoQg13Q@mail.gmail.com>
 <be8d9147-4f7f-2fab-da2a-bb4cde46fd12@huaweicloud.com>
 <CAPhsuW6kv7FRB_1NoheiDqvmLmongiJ-ty9mYRNvFw3yecE_Ug@mail.gmail.com>
 <baf95bd0-0378-9b3a-9ab9-473baa35ebbc@huaweicloud.com>
 <CAPhsuW61yzkH5Tp0ku37DBjnQzT81yJUx0F4bag4xdq1rX5gsA@mail.gmail.com>
From: Yu Kuai <yukuai1@huaweicloud.com>
Message-ID: <8713b617-db39-113a-377f-f97e123dbf9b@huaweicloud.com>
Date: Tue, 19 Dec 2023 09:39:51 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <CAPhsuW61yzkH5Tp0ku37DBjnQzT81yJUx0F4bag4xdq1rX5gsA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi,

在 2023/12/19 0:04, Song Liu 写道:
> On Sun, Dec 17, 2023 at 5:39 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>>
> [...]
>>>
>>> We only use  this for idle or not check, the behavior is OK (I think).
>>> However, this logic is error prone.
>>>
>>> On 64-bit systems, there is a 4-byte hole behind sync_io. I think we can
>>> just use it for atomic64_t so that we don't have to worry about overflow.
>>
>> I'm not sure about this, because other than this ubsan warning, this
>> overflow doesn't have any impact on functionality to me.
> 
> Fixing warnings for zero or low cost is always a good idea. It helps boost
> the signal when UBSAN (and other debug features) detects real issues.
> 
>> If we care about this 'hole', there are lots of holes in gendisk, and
>> can be avoiled, for example, moving 'sync_io' near  to 'node_id'.
> 
> The point was not "let's fill the hole", but "we can use atomic64_t
> without extra memory cost". In general, I don't think we care too
> much about holes in "struct gendisk".
> 
> Does this make sense?

Of course, I don't have strong preference on this. Because our syzkaller
reported lots of UBSAN warnings, hence only fix real issues is how we do
it. For upstream, I'm good at fix this warning with zero or low cost.

Thanks,
Kuai

> 
> Thanks,
> Song
> .
>