On Wed, 5 Jun 2019, Jens Axboe wrote:
> 
> How about the following plan - if folks are happy with this sched patch,
> we can queue it up for 5.3. Once that is in, I'll kill the block change
> that special cases the polled task wakeup. For 5.2, we go with Oleg's
> patch for the swap case.

I just hit the do_task_dead() kernel BUG at kernel/sched/core.c:3463!
while heavy swapping on 5.2-rc7: it looks like Oleg's patch intended
for 5.2 was not signed off, and got forgotten.

I did hit the do_task_dead() BUG (but not at all easily) on early -rcs
before seeing Oleg's patch, then folded it in and and didn't hit the BUG
again; then just tried again without it, and luckily hit in a few hours.

So I can give it an enthusiastic
Acked-by: Hugh Dickins <hu...@google.com>
because it makes good sense to avoid the get/blk_wake/put overhead on
the asynch path anyway, even if it didn't work around a bug; but only
Half-Tested-by: Hugh Dickins <hu...@google.com>
since I have not been exercising the synchronous path at all.

Hugh, requoting Oleg:

> 
> I don't understand this code at all but I am just curious, can we do
> something like incomplete patch below ?
> 
> Oleg.
> 
> --- x/mm/page_io.c
> +++ x/mm/page_io.c
> @@ -140,8 +140,10 @@ int swap_readpage(struct page *page, bool synchronous)
>       unlock_page(page);
>       WRITE_ONCE(bio->bi_private, NULL);
>       bio_put(bio);
> -     blk_wake_io_task(waiter);
> -     put_task_struct(waiter);
> +     if (waiter) {
> +             blk_wake_io_task(waiter);
> +             put_task_struct(waiter);
> +     }
>  }
>  
>  int generic_swapfile_activate(struct swap_info_struct *sis,
> @@ -398,11 +400,12 @@ int swap_readpage(struct page *page, boo
>        * Keep this task valid during swap readpage because the oom killer may
>        * attempt to access it in the page fault retry time check.
>        */
> -     get_task_struct(current);
> -     bio->bi_private = current;
>       bio_set_op_attrs(bio, REQ_OP_READ, 0);
> -     if (synchronous)
> +     if (synchronous) {
>               bio->bi_opf |= REQ_HIPRI;
> +             get_task_struct(current);
> +             bio->bi_private = current;
> +     }
>       count_vm_event(PSWPIN);
>       bio_get(bio);
>       qc = submit_bio(bio);

Reply via email to