Hello Chuansheng,
On 3/17/22 06:46, Chuansheng Liu wrote:
> Easily hit the below list corruption:
> ==
> list_add corruption. prev->next should be next (ffffffffc0ceb090), but
> was ffffec604507edc8. (prev=ffffec604507edc8).
> WARNING: CPU: 65 PID: 3959 at lib/list_debug.c:26
> __list_add_valid+0x53/0x80
> CPU: 65 PID: 3959 Comm: fbdev Tainted: G U
> RIP: 0010:__list_add_valid+0x53/0x80
> Call Trace:
> <TASK>
> fb_deferred_io_mkwrite+0xea/0x150
> do_page_mkwrite+0x57/0xc0
> do_wp_page+0x278/0x2f0
> __handle_mm_fault+0xdc2/0x1590
> handle_mm_fault+0xdd/0x2c0
> do_user_addr_fault+0x1d3/0x650
> exc_page_fault+0x77/0x180
> ? asm_exc_page_fault+0x8/0x30
> asm_exc_page_fault+0x1e/0x30
> RIP: 0033:0x7fd98fc8fad1
> ==
>
> Figure out the race happens when one process is adding &page->lru into
> the pagelist tail in fb_deferred_io_mkwrite(), another process is
> re-initializing the same &page->lru in fb_deferred_io_fault(), which is
> not protected by the lock.
>
> This fix is to init all the page lists one time during initialization,
> it not only fixes the list corruption, but also avoids INIT_LIST_HEAD()
> redundantly.
>
> Fixes: 105a940416fc ("fbdev/defio: Early-out if page is already
> enlisted")
> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> Signed-off-by: Chuansheng Liu <chuansheng.liu@xxxxxxxxx>
> ---
This makes sense to me. If you address Geert comment and post a v2,
feel free to add:
Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
