Re: [PATCH v6 2/2] mm: support large folios swap-in for zRAM-like devices

[Barry Song <21cnbao@gmail.com>]

On Mon, Aug 12, 2024 at 8:27 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> The subject feels wrong.  Nothing particular about zram, it is all about
> SWP_SYNCHRONOUS_IO, so the Subject and commit log should state that.

right.

This is absolutely for sync io, zram is the most typical one which is
widely used in Android and embedded systems.  Others could be
nvdimm, brd.

>
> On Sat, Aug 03, 2024 at 12:20:31AM +1200, Barry Song wrote:
> > From: Chuanhua Han <hanchuanhua@oppo.com>
> >
> > Currently, we have mTHP features, but unfortunately, without support for large
> > folio swap-ins, once these large folios are swapped out, they are lost because
> > mTHP swap is a one-way process. The lack of mTHP swap-in functionality prevents
>
> Please wrap your commit logs after 73 characters to make them readable.

ack.

>
> > +/*
> > + * check a range of PTEs are completely swap entries with
> > + * contiguous swap offsets and the same SWAP_HAS_CACHE.
> > + * ptep must be first one in the range
> > + */
>
> Please capitalize the first character of block comments, make them full
> sentences and use up all 80 characters.

ack.

>
> > +     for (i = 1; i < nr_pages; i++) {
> > +             /*
> > +              * while allocating a large folio and doing swap_read_folio for the
>
> And also do not go over 80 characters for them, which renders them
> really hard to read.
>
> > +static struct folio *alloc_swap_folio(struct vm_fault *vmf)
> > +{
> > +     struct vm_area_struct *vma = vmf->vma;
> > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>
> Please stub out the entire function.

I assume you mean the below?

#ifdef CONFIG_TRANSPARENT_HUGEPAGE
static struct folio *alloc_swap_folio(struct vm_fault *vmf)
{
}
#else
static struct folio *alloc_swap_folio(struct vm_fault *vmf)
{
}
#endif

If so, this is fine to me. the only reason I am using the current
pattern is that i am trying to follow the same pattern with

static struct folio *alloc_anon_folio(struct vm_fault *vmf)
{
        struct vm_area_struct *vma = vmf->vma;
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
#endif
        ...
}

Likely we also want to change that one?

Thanks
Barry