Messages in this thread | | | Date | Tue, 7 Feb 2023 03:52:53 +0000 | From | Matthew Wilcox <> | Subject | Re: [RFC 1/2] shmem: set shmem_writepage() variables early |
| |
On Mon, Feb 06, 2023 at 06:52:58PM -0800, Luis Chamberlain wrote: > shmem_writepage() sets up variables typically used *after* a possible > huge page split. However even if that does happen the address space > mapping should not change. So it should be safe to set that from > the beginning.
Yes, we can get mapping from the folio early. It doesn't change on split.
> The folio should always be locked from the start as well. It however > was not clear if the folio address can / should change, as well as > the first inode. > > This commit makes no functional changes other a double check on the > folio locking which might be superflous. This change should help make > the subsequent patch easier to review.
You don't need to check that the folio's locked, and you don't need to reassign inode after the split.
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > --- > mm/shmem.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/mm/shmem.c b/mm/shmem.c > index 28f3c699c8ce..a2c6aa11aab8 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -1332,11 +1332,13 @@ static int shmem_writepage(struct page *page, struct writeback_control *wbc) > { > struct folio *folio = page_folio(page); > struct shmem_inode_info *info; > - struct address_space *mapping; > - struct inode *inode; > + struct address_space *mapping = folio->mapping; > + struct inode *inode = mapping->host; > swp_entry_t swap; > pgoff_t index; > > + BUG_ON(!folio_test_locked(folio)); > + > /* > * If /sys/kernel/mm/transparent_hugepage/shmem_enabled is "always" or > * "force", drivers/gpu/drm/i915/gem/i915_gem_shmem.c gets huge pages, > @@ -1351,8 +1353,8 @@ static int shmem_writepage(struct page *page, struct writeback_control *wbc) > folio_clear_dirty(folio); > } > > + /* Can the folio or first inode change on after a split? */ > BUG_ON(!folio_test_locked(folio)); > - mapping = folio->mapping; > index = folio->index; > inode = mapping->host; > info = SHMEM_I(inode); > -- > 2.39.0 >
| |