Received-SPF: pass (google.com: domain of linux-kernel+bounces-200114-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <be478487-8509-4bfe-9354-c1c3f6246489@vivo.com>
Date: Tue, 4 Jun 2024 14:38:01 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2] sbitmap: fix io hung due to race on
 sbitmap_word::cleared
Content-Language: en-US
To: Ming Lei <ming.lei@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>, Andrew Morton <akpm@linux-foundation.org>,
 Pavel Begunkov <asml.silence@gmail.com>, linux-kernel@vger.kernel.org,
 linux-block@vger.kernel.org
References: <20240604031124.2261-1-yang.yang@vivo.com>
 <CAFj5m9KV7OJ4_KjbSkpdtfrKamoLzV6EH-mJP3=y+VvoYOzC3w@mail.gmail.com>
From: YangYang <yang.yang@vivo.com>
In-Reply-To: <CAFj5m9KV7OJ4_KjbSkpdtfrKamoLzV6EH-mJP3=y+VvoYOzC3w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?ZCt5RVc2dWlYNCtOQkwwSTJVaGduYUoxejJuZlNtQ0Z0TE81Q1VCa1Q3S0Jv?=
 =?utf-8?B?OGFLVGdKb0hJRnFYUlhDU2ROWFNqTHNtdFBzZ1FLVXgxVkd3b1lPTTlRUmd3?=
 =?utf-8?B?QThtclVibXNGclVpOGprUUJRV2NubGczekF4RVpIVEM1aFpQVmw5NGw3alkx?=
 =?utf-8?B?dEJJNWhOSEVwVWFZTnFUd25LQ3NxekVLTDJTZlJSSWJXZTJFQlIvRmZoRDlR?=
 =?utf-8?B?dWc2dElFOWx2M05kajJ3THIvRkRhYVYwOWFLSTJVL3BIRWFUNUxOeUJwQUlZ?=
 =?utf-8?B?M3dXT2pNWURKWEo2b0xPZW1veXFhNEFFK1RJREZkQUUzY001OEIyVW5oYXlG?=
 =?utf-8?B?TE8wKzZIRHpjemtwMEJ3V3B2VldaZzRmcmtkZWpNdlJwSGRJQTJ5MVRjS2Ro?=
 =?utf-8?B?RXZsS0dqSjFxQzBOS04rWWlZeUV5MzVqcmkxaXRMKzhVRVBrUWdLdG1zWXdk?=
 =?utf-8?B?TjNydk0zSmpEUXZlMVc2YmNxVzVsTG9HbUFPVVlLbW1BMjBDdTB6OVlZRFBJ?=
 =?utf-8?B?Z3ljZWF3d3hTY1QxajF2bDZMZ0hEM0U0OFV5dmJ2cVNKdU5SemxVd3pEU2o1?=
 =?utf-8?B?QXRuQmM1a08vMGoxTGVLVzhnM0F0VzQyWkdTSTQvN0dVc1RUQkRFUU1ibVdR?=
 =?utf-8?B?cGE4dkRmcGpzUmxNUU5SVTFHd3dTRnNpTTIxVmM0N2lBdnVXdW9xTU5BYjV4?=
 =?utf-8?B?TGlURkM4QTd3NGkyK2Y2Z3hBaGk5V3VQUlJxKzNjRGF1OEdub3ZvMUVRZ0NH?=
 =?utf-8?B?KzYxenRZNjZLS1ZKQSs1cnl5RFM2dnZuOFA0VUJ1UXdPUytOdFY0ZUplZzFM?=
 =?utf-8?B?Ym9kV3RYWklxVmF6enBSbDZ6NElzeGpSQlZ3U0l1Y084b2JBVkpyck1KNWU1?=
 =?utf-8?B?MlpEdEFSaHZJcTlJNUY1QU9mREx6TTJJRUtqNkZWV2M1ellCVGZzbkVTMk5L?=
 =?utf-8?B?a1l4eEtDOEFkTGdpWkJlL1gzSGVYbzlBMlpUa2dkK1Y1dmVKeCsyOGJzMzJt?=
 =?utf-8?B?N3UvbEZWTFNRdXpvZmpJMnQwSVFLemRpcDJIam5kaTljT002b056UVR1ZDV0?=
 =?utf-8?B?MkVLMnNwekwxek53RWoyN204VVZyd1pvTFMzcTZUZjdoc1IvWVVaVjRiemNm?=
 =?utf-8?B?VHBsVi9tRHJaOHdoYjZKbVVvYkZVcG1aR3gxZ1daQnBJQ2puWWpVVHNpQ3Nq?=
 =?utf-8?B?SHlDdzBUYWlDOGU0V0x1QWJKR2p6cmYxV0lHcEwzcS84WVdiUVZVenk4d0hq?=
 =?utf-8?B?bG9aZDZSSHpZL2Z4Z1FnblAyYW0xVUY2VkpybWFyN0EvOHFMN0dFZWZ1VVAv?=
 =?utf-8?B?SkxVb0o2bDU0QmJzQW1NbDg1c3VMVVlzMnpKNUhieUd5aEJBMmtxN2NTNlY1?=
 =?utf-8?B?QXpXSnhOQzBmSjZBVGhOZVQyNzV5Ykpha2tIVjZOUVNncSt3bnlCcm9OdEF0?=
 =?utf-8?B?MTJramhEMGVjSzMyQVRLd3BxZmtUQ0VEWENiVzZrUXdDc2dFOWxwY3ZCZEpK?=
 =?utf-8?B?SzRmSENrTXozYkIvSmd1Y3R2T0dZalFyM3FPQU5acW9vNWttN2VUdlNmN1k0?=
 =?utf-8?B?RHI5VnlCbi83eXUzSGtCZTI2bFlieXpvdSs5SUtFNDRhQlhSWGEvVEZwb2V5?=
 =?utf-8?B?WXd1SU1ZTDB4MnkzUjRVZ1RlVVd3dW12dWI3RjFaMU5sWUxySEJTeFlzd05K?=
 =?utf-8?B?cVVoV0l6QjhqVEN4OTJpbzN5NmlZY1JuYWtwZzRYTnA5aE9qL2hnbUprYTlu?=
 =?utf-8?B?bi9WcEIxbGZKcjFNcW5KSVJMa2NmNmVOdGlla1d4cDI4YmhiNElPaDhMVXFP?=
 =?utf-8?B?K05tWC9rY0I5N3dmRndNYUs5VGxPZDBvNTE2VlA0OGlSY2JtemlOUStkVUQ4?=
 =?utf-8?B?bHNlcElaakxON0lZakJrKzVkdGpHMGdoOCsrYTFyRjFwUnJIUGVtVDNnajdR?=
 =?utf-8?B?d290N29EVVRVKzN4RDRZVTl6NFA0N3ZjMkZnMTdWQmU0dlRwSUlGVXBRMzZh?=
 =?utf-8?B?UTZhcUlSa2VFSnUvR1R1Vy9SOXowbktxTmluVmo0dWdJbWVBVG9RTWZmenY4?=
 =?utf-8?B?YkJ5eFVvTlBqZ05xVXpEU01Lbm5hdEFNSXlXMnQrbHNtSDhialJCQTFhbUFt?=
 =?utf-8?Q?9BQ3aOYVLY5O+WUZWZ0PN89so?=
X-OriginatorOrg: vivo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d98840df-e6bf-494a-8f84-08dc8460e34c
X-MS-Exchange-CrossTenant-AuthSource: KL1PR06MB7401.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jun 2024 06:38:04.6638
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 923e42dc-48d5-4cbe-b582-1a797a6412ed
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: arSq7abAfUMOgZ4Dfjzd9xd98QOrbIsSHzwbDbjaL9cIQ64j+e6orH0GP0xKaRP8Z9Z6rS4hW4t9Yg4M5CGI7A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SEZPR06MB6741

On 2024/6/4 11:25, Ming Lei wrote:
> On Tue, Jun 4, 2024 at 11:12 AM Yang Yang <yang.yang@vivo.com> wrote:
>>
>> Configuration for sbq:
>>    depth=64, wake_batch=6, shift=6, map_nr=1
>>
>> 1. There are 64 requests in progress:
>>    map->word = 0xFFFFFFFFFFFFFFFF
>> 2. After all the 64 requests complete, and no more requests come:
>>    map->word = 0xFFFFFFFFFFFFFFFF, map->cleared = 0xFFFFFFFFFFFFFFFF
>> 3. Now two tasks try to allocate requests:
>>    T1:                                       T2:
>>    __blk_mq_get_tag                          .
>>    __sbitmap_queue_get                       .
>>    sbitmap_get                               .
>>    sbitmap_find_bit                          .
>>    sbitmap_find_bit_in_word                  .
>>    __sbitmap_get_word  -> nr=-1              __blk_mq_get_tag
>>    sbitmap_deferred_clear                    __sbitmap_queue_get
>>    /* map->cleared=0xFFFFFFFFFFFFFFFF */     sbitmap_find_bit
>>      if (!READ_ONCE(map->cleared))           sbitmap_find_bit_in_word
>>        return false;                         __sbitmap_get_word -> nr=-1
>>      mask = xchg(&map->cleared, 0)           sbitmap_deferred_clear
>>      atomic_long_andnot()                    /* map->cleared=0 */
>>                                                if (!(map->cleared))
>>                                                  return false;
>>                                       /*
>>                                        * map->cleared is cleared by T1
>>                                        * T2 fail to acquire the tag
>>                                        */
>>
>> 4. T2 is the sole tag waiter. When T1 puts the tag, T2 cannot be woken
>> up due to the wake_batch being set at 6. If no more requests come, T1
>> will wait here indefinitely.
>>
>> To fix this issue, simply revert commit 661d4f55a794 ("sbitmap:
>> remove swap_lock"), which causes this issue.
> 
> I'd suggest to add the following words in commit log:
> 
> Check on ->cleared and update on both ->cleared and ->word need to be
> done atomically, and using spinlock could be the simplest solution.

Thank you. I will handle this in V3.

> 
> Otherwise, the patch looks fine for me.
> 
> Thanks,
>