On Mon, 16 Nov 2020, akpm@xxxxxxxxxxxxxxxxxxxx wrote: > > The patch titled > Subject: mm/shmem: use kmem_cache_zalloc in shmem_alloc_inode() > has been added to the -mm tree. Its filename is > mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode.patch > > This patch should soon appear at > https://ozlabs.org/~akpm/mmots/broken-out/mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode.patch > and later at > https://ozlabs.org/~akpm/mmotm/broken-out/mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode.patch > > Before you just go and hit "reply", please: > a) Consider who else should be cc'ed > b) Prefer to cc a suitable mailing list as well > c) Ideally: find the original patch on the mailing list and do a > reply-to-all to that, adding suitable additional cc's > > *** Remember to use Documentation/process/submit-checklist.rst when testing your code *** > > The -mm tree is included into linux-next and is updated > there every 3-4 working days > > ------------------------------------------------------ > From: Hui Su <sh_def@xxxxxxx> > Subject: mm/shmem: use kmem_cache_zalloc in shmem_alloc_inode() Andrew, Stephen, please revert this untested "cleanup" from your trees a.s.a.p: it's a disaster for anyone using shmem/tmpfs. > > in shmem_get_inode(): > new_inode(); > new_inode_pseudo(); > alloc_inode(); > ops->alloc_inode(); -> shmem_alloc_inode() > kmem_cache_alloc(); > > memset(info, 0, (char *)inode - (char *)info); > > So use kmem_cache_zalloc() in shmem_alloc_inode(), > and remove the memset in shmem_get_inode(). I could not follow that argument at all. The shmem_inode_cachep uses a constructor, and the memset shown is of only a portion of the whole inode. zeroing the entire inode quickly crashes the kernel, after showing errors. (If you're lucky enough to have a readable display at that point: I did not, but got on better with framebuffer than drm/i915; and I wonder if there's a separate bug in that area too, because fixing this shmem issue is not enough to get my drm/i915 rc4-mm1 booting.) > > Link: https://lkml.kernel.org/r/20201115174026.GA365412@rlk > Signed-off-by: Hui Su <sh_def@xxxxxxx> NAK. Hui Su, please test your "cleanups" before sending them. I'm sorry for being slow to respond, but the priority appeared to be to get Matthew Wilcox's series running reliably, so I had not got around to checking the less urgent shmem patches before they slipped into mmotm - there may well be more that I want to NAK, but this is the dangerous one. Thanks, Hugh > Cc: Hugh Dickins <hughd@xxxxxxxxxx> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > --- > > mm/shmem.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > --- a/mm/shmem.c~mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode > +++ a/mm/shmem.c > @@ -2331,7 +2331,6 @@ static struct inode *shmem_get_inode(str > inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode); > inode->i_generation = prandom_u32(); > info = SHMEM_I(inode); > - memset(info, 0, (char *)inode - (char *)info); > spin_lock_init(&info->lock); > atomic_set(&info->stop_eviction, 0); > info->seals = F_SEAL_SEAL; > @@ -3851,7 +3850,7 @@ static struct kmem_cache *shmem_inode_ca > static struct inode *shmem_alloc_inode(struct super_block *sb) > { > struct shmem_inode_info *info; > - info = kmem_cache_alloc(shmem_inode_cachep, GFP_KERNEL); > + info = kmem_cache_zalloc(shmem_inode_cachep, GFP_KERNEL); > if (!info) > return NULL; > return &info->vfs_inode; > _ > > Patches currently in -mm which might be from sh_def@xxxxxxx are > > mmslab_common-use-list_for_each_entry-in-dump_unreclaimable_slab.patch > mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode.patch > mm-page_counter-use-page_counter_read-in-page_counter_set_max.patch > mm-hugetlbc-just-use-put_page_testzero-instead-of-page_count.patch > mm-compaction-move-compaction_suitables-comment-to-right-place.patch > mm-oom_kill-change-comment-and-rename-is_dump_unreclaim_slabs.patch > acctc-use-elif-instead-of-end-and-elif.patch > mm-memcontrol-rewrite-mem_cgroup_page_lruvec.patch
