On Mon 30-10-23 09:37:10, Vlastimil Babka wrote:
> On 10/30/23 08:37, Mikulas Patocka wrote:
> > On Sun, 29 Oct 2023, Vlastimil Babka wrote:
> >
> >> Haven't found any. However I'd like to point out some things I noticed in
> >> crypt_alloc_buffer(), although they are probably not related.
> >>
> >> > static struct bio *crypt_alloc_buffer(struct dm_crypt_io *io, unsigned int size)
> >> > {
> >> > struct crypt_config *cc = io->cc;
> >> > struct bio *clone;
> >> > unsigned int nr_iovecs = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> >> > gfp_t gfp_mask = GFP_NOWAIT | __GFP_HIGHMEM;
> >> > unsigned int remaining_size;
> >> > unsigned int order = MAX_ORDER - 1;
> >> >
> >> > retry:
> >> > if (unlikely(gfp_mask & __GFP_DIRECT_RECLAIM))
> >> > mutex_lock(&cc->bio_alloc_lock);
> >>
> >> What if we end up in "goto retry" more than once? I don't see a matching
> >
> > It is impossible. Before we jump to the retry label, we set
> > __GFP_DIRECT_RECLAIM. mempool_alloc can't ever fail if
> > __GFP_DIRECT_RECLAIM is present (it will just wait until some other task
> > frees some objects into the mempool).
>
> Ah, missed that. And the traces don't show that we would be waiting for
> that. I'm starting to think the allocation itself is really not the issue
> here. Also I don't think it deprives something else of large order pages, as
> per the sysrq listing they still existed.
>
> What I rather suspect is what happens next to the allocated bio such that it
> works well with order-0 or up to costly_order pages, but there's some
> problem causing a deadlock if the bio contains larger pages than that?
Hum, so in all the backtraces presented we see that we are waiting for page
writeback to complete but I don't see anything that would be preventing the
bios from completing. Page writeback can submit quite large bios so it kind
of makes sense that it trips up some odd behavior. Looking at the code
I can see one possible problem in crypt_alloc_buffer() but it doesn't
explain why reducing initial page order would help. Anyway: Are we
guaranteed mempool has enough pages for arbitrarily large bio that can
enter crypt_alloc_buffer()? I can see crypt_page_alloc() does limit the
number of pages in the mempool to dm_crypt_pages_per_client plus I assume
the percpu counter bias in cc->n_allocated_pages can limit the really
available number of pages even further. So if a single bio is large enough
to trip percpu_counter_read_positive(&cc->n_allocated_pages) >=
dm_crypt_pages_per_client condition in crypt_page_alloc(), we can loop
forever? But maybe this cannot happen for some reason...
If this is not it, I think we need to find out why the writeback bios are
not completeting. Probably I'd start with checking what is kcryptd,
presumably responsible for processing these bios, doing.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
