Linux-Next discussions
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Mark Brown <broonie@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux Next Mailing List <linux-next@vger.kernel.org>,
	Hugh Dickins <hughd@google.com>,
	Baolin Wang <baolin.wang@linux.alibaba.com>, <linux-mm@kvack.org>
Subject: Re: linux-next: manual merge of the mm-unstable tree with the drm-misc-fixes tree
Date: Mon, 20 Jul 2026 14:58:34 -0700	[thread overview]
Message-ID: <al6aCizqqilqQGZ4@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <20260720144141.GA16699@lst.de>

On Mon, Jul 20, 2026 at 04:41:41PM +0200, Christoph Hellwig wrote:
> On Mon, Jul 20, 2026 at 02:31:04PM +0100, Mark Brown wrote:
> > Hi all,
> > 
> > Today's linux-next merge of the mm-unstable tree got a conflict in:
> > 
> >   drivers/gpu/drm/ttm/ttm_backup.c
> > 
> > between commit:
> > 
> >   a3fdf74ffa596 ("drm/ttm/pool: back up at native page order")
> > 
> > from the drm-misc-fixes tree and commit:
> > 
> >   c6bfdcf16f131 ("shmem: provide a shmem_write_folio wrapper")
> > 
> > from the mm-unstable tree.
> > 
> > I fixed it up (see below) and can carry the fix as necessary. This
> > is now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your tree
> > is submitted for merging.  You may also want to consider cooperating
> > with the maintainer of the conflicting tree to minimise any particularly
> > complex conflicts.
> 
> Much of the code here really should sit in shmem.c instead of having
> random drivers/subsystems poke into the internals of shmem mappings
> and folios.  But I feel like a broken record for saying that again
> and again without any action :(
> 

Do you have a suggestion of what parts to move over to shmem.c?

Pretty much all of this? I can take a look at this in a follow up?

Matt

> > 
> > diff --combined drivers/gpu/drm/ttm/ttm_backup.c
> > index 3c067aadc52de,c5b813a563e7f..0000000000000
> > --- a/drivers/gpu/drm/ttm/ttm_backup.c
> > +++ b/drivers/gpu/drm/ttm/ttm_backup.c
> > @@@ -6,10 -6,9 +6,10 @@@
> >   #include <drm/ttm/ttm_backup.h>
> >   
> >   #include <linux/export.h>
> >  -#include <linux/page-flags.h>
> >   #include <linux/swap.h>
> >   
> >  +#include "ttm_pool_internal.h"
> >  +
> >   /*
> >    * Need to map shmem indices to handle since a handle value
> >    * of 0 means error, following the swp_entry_t convention.
> > @@@ -69,23 -68,17 +69,23 @@@ int ttm_backup_copy_page(struct file *b
> >   }
> >   
> >   /**
> >  - * ttm_backup_backup_page() - Backup a page
> >  + * ttm_backup_backup_folio() - Backup a folio
> >    * @backup: The struct backup pointer to use.
> >  - * @page: The page to back up.
> >  - * @writeback: Whether to perform immediate writeback of the page.
> >  + * @folio: The folio to back up.
> >  + * @order: The allocation order of @folio.  Since TTM allocates higher-order
> >  + *         pages without __GFP_COMP, folio_nr_pages(@folio) would always
> >  + *         return 1; the caller must pass the true order explicitly.
> >  + * @writeback: Whether to perform immediate writeback of the folio's pages.
> >    * This may have performance implications.
> >  - * @idx: A unique integer for each page and each struct backup.
> >  + * @idx: A unique integer for the first page of the folio and each struct backup.
> >    * This allows the backup implementation to avoid managing
> >    * its address space separately.
> >  - * @page_gfp: The gfp value used when the page was allocated.
> >  - * This is used for accounting purposes.
> >  + * @folio_gfp: The gfp value used when the folio was allocated.
> >  + * Currently unused.
> >    * @alloc_gfp: The gfp to be used when allocating memory.
> >  + * @nr_pages_backed: Output. On a successful return, set to the number of
> >  + * pages actually backed up, which may be less than (1 << @order)
> >  + * if an -ENOMEM was encountered mid-folio.
> >    *
> >    * Context: If called from reclaim context, the caller needs to
> >    * assert that the shrinker gfp has __GFP_FS set, to avoid
> > @@@ -94,87 -87,53 +94,87 @@@
> >    * that the shrinker gfp has __GFP_IO set, since without it,
> >    * we're not allowed to start backup IO.
> >    *
> >  - * Return: A handle on success. Negative error code on failure.
> >  - *
> >  - * Note: This function could be extended to back up a folio and
> >  - * implementations would then split the folio internally if needed.
> >  - * Drawback is that the caller would then have to keep track of
> >  - * the folio size- and usage.
> >  + * Return: A handle for the first backed-up page on success (handles for
> >  + * subsequent pages follow sequentially). -ENOMEM if no pages could be backed
> >  + * up. Any other negative error code if a non-ENOMEM failure occurred; in that
> >  + * case any pages backed up so far are truncated before returning.
> >    */
> >   s64
> >  -ttm_backup_backup_page(struct file *backup, struct page *page,
> >  -		       bool writeback, pgoff_t idx, gfp_t page_gfp,
> >  -		       gfp_t alloc_gfp)
> >  +ttm_backup_backup_folio(struct file *backup, struct folio *folio,
> >  +			unsigned int order, bool writeback, pgoff_t idx,
> >  +			gfp_t folio_gfp, gfp_t alloc_gfp,
> >  +			pgoff_t *nr_pages_backed)
> >   {
> >   	struct address_space *mapping = backup->f_mapping;
> >  -	unsigned long handle = 0;
> >  +	int nr_pages = 1 << order;
> >   	struct folio *to_folio;
> >  -	int ret;
> >  +	int ret, i;
> >   
> >  -	to_folio = shmem_read_folio_gfp(mapping, idx, alloc_gfp);
> >  -	if (IS_ERR(to_folio))
> >  -		return PTR_ERR(to_folio);
> >  +	*nr_pages_backed = 0;
> >   
> >  -	folio_mark_accessed(to_folio);
> >  -	folio_lock(to_folio);
> >  -	folio_mark_dirty(to_folio);
> >  -	copy_highpage(folio_file_page(to_folio, idx), page);
> >  -	handle = ttm_backup_shmem_idx_to_handle(idx);
> >  +	for (i = 0; i < nr_pages; ) {
> >  +		int to_nr, j;
> >   
> >  -	if (writeback && !folio_mapped(to_folio) &&
> >  -	    folio_clear_dirty_for_io(to_folio)) {
> >  -		folio_set_reclaim(to_folio);
> >  -		ret = shmem_write_folio(to_folio);
> >  -		if (!folio_test_writeback(to_folio))
> >  -			folio_clear_reclaim(to_folio);
> >   		/*
> >  -		 * If writeout succeeds, it unlocks the folio.	errors
> >  -		 * are otherwise dropped, since writeout is only best
> >  -		 * effort here.
> >  +		 * Only inject past the first subpage so *nr_pages_backed is
> >  +		 * always > 0 here, matching a genuine mid-compound -ENOMEM
> >  +		 * and driving the caller's reactive split fallback instead
> >  +		 * of an early, no-progress failure.
> >   		 */
> >  -		if (ret)
> >  +		if (IS_ENABLED(CONFIG_FAULT_INJECTION) && i &&
> >  +		    ttm_backup_fault_inject_folio())
> >  +			to_folio = ERR_PTR(-ENOMEM);
> >  +		else
> >  +			to_folio = shmem_read_folio_gfp(mapping, idx + i, alloc_gfp);
> >  +		if (IS_ERR(to_folio)) {
> >  +			int err = PTR_ERR(to_folio);
> >  +
> >  +			if (err == -ENOMEM && *nr_pages_backed)
> >  +				return ttm_backup_shmem_idx_to_handle(idx);
> >  +
> >  +			if (*nr_pages_backed) {
> >  +				shmem_truncate_range(file_inode(backup),
> >  +						     (loff_t)idx << PAGE_SHIFT,
> >  +						     ((loff_t)(idx + i) << PAGE_SHIFT) - 1);
> >  +				/*
> >  +				 * The pages just truncated are no longer
> >  +				 * backed up; don't let the caller mistake
> >  +				 * them for valid handles.
> >  +				 */
> >  +				*nr_pages_backed = 0;
> >  +			}
> >  +			return err;
> >  +		}
> >  +
> >  +		to_nr = min_t(int, nr_pages - i,
> >  +			      folio_next_index(to_folio) - (idx + i));
> >  +
> >  +		folio_mark_accessed(to_folio);
> >  +		folio_lock(to_folio);
> >  +		folio_mark_dirty(to_folio);
> >  +
> >  +		for (j = 0; j < to_nr; j++)
> >  +			copy_highpage(folio_file_page(to_folio, idx + i + j),
> >  +				      folio_page(folio, i + j));
> >  +
> >  +		if (writeback && !folio_mapped(to_folio) &&
> >  +		    folio_clear_dirty_for_io(to_folio)) {
> >  +			folio_set_reclaim(to_folio);
> > ++			ret = shmem_write_folio(to_folio);
> >  +			if (!folio_test_writeback(to_folio))
> >  +				folio_clear_reclaim(to_folio);
> >  +			if (ret == AOP_WRITEPAGE_ACTIVATE)
> >  +				folio_unlock(to_folio);
> >  +		} else {
> >   			folio_unlock(to_folio);
> >  -	} else {
> >  -		folio_unlock(to_folio);
> >  +		}
> >  +
> >  +		folio_put(to_folio);
> >  +		i += to_nr;
> >  +		*nr_pages_backed = i;
> >   	}
> >   
> >  -	folio_put(to_folio);
> >  -
> >  -	return handle;
> >  +	return ttm_backup_shmem_idx_to_handle(idx);
> >   }
> >   
> >   /**
> 
> 
> ---end quoted text---

  parent reply	other threads:[~2026-07-20 21:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-20 13:31 linux-next: manual merge of the mm-unstable tree with the drm-misc-fixes tree Mark Brown
2026-07-20 14:41 ` Christoph Hellwig
2026-07-20 14:46   ` Matthew Wilcox
2026-07-20 22:08     ` Matthew Brost
2026-07-20 21:58   ` Matthew Brost [this message]
  -- strict thread matches above, loose matches on Subject: below --
2025-08-13  1:11 Stephen Rothwell
2025-08-13  3:59 ` Andrew Morton
2025-08-13  9:07   ` Danilo Krummrich
2025-08-13  8:50 ` Danilo Krummrich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=al6aCizqqilqQGZ4@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=broonie@kernel.org \
    --cc=hch@lst.de \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-next@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox