* Re: + mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode.patch added to -mm tree [not found] <20201116230526.NkEfmeDCH%akpm@linux-foundation.org> @ 2020-11-17 6:17 ` Hugh Dickins 2020-11-17 7:12 ` Stephen Rothwell 2020-11-20 19:32 ` Hui Su 0 siblings, 2 replies; 4+ messages in thread From: Hugh Dickins @ 2020-11-17 6:17 UTC (permalink / raw) To: Andrew Morton, Stephen Rothwell Cc: hughd, mm-commits, sh_def, linux-kernel, linux-mm On Mon, 16 Nov 2020, akpm@linux-foundation.org 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@163.com> > 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@163.com> 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@google.com> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > --- > > 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@163.com 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: + mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode.patch added to -mm tree 2020-11-17 6:17 ` + mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode.patch added to -mm tree Hugh Dickins @ 2020-11-17 7:12 ` Stephen Rothwell 2020-11-17 15:31 ` Hui Su 2020-11-20 19:32 ` Hui Su 1 sibling, 1 reply; 4+ messages in thread From: Stephen Rothwell @ 2020-11-17 7:12 UTC (permalink / raw) To: Hugh Dickins; +Cc: Andrew Morton, mm-commits, sh_def, linux-kernel, linux-mm [-- Attachment #1: Type: text/plain, Size: 1617 bytes --] Hi Hugh, On Mon, 16 Nov 2020 22:17:20 -0800 (PST) Hugh Dickins <hughd@google.com> wrote: > > On Mon, 16 Nov 2020, akpm@linux-foundation.org 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@163.com> > > 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. Thanks for that. I have also bisected my boot failures to that commit and so have reverted it from linux-next today. -- Cheers, Stephen Rothwell [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: + mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode.patch added to -mm tree 2020-11-17 7:12 ` Stephen Rothwell @ 2020-11-17 15:31 ` Hui Su 0 siblings, 0 replies; 4+ messages in thread From: Hui Su @ 2020-11-17 15:31 UTC (permalink / raw) To: Stephen Rothwell, hughd Cc: Andrew Morton, mm-commits, sh_def, linux-kernel, linux-mm On Tue, Nov 17, 2020 at 06:12:46PM +1100, Stephen Rothwell wrote: > Hi Hugh, > > On Mon, 16 Nov 2020 22:17:20 -0800 (PST) Hugh Dickins <hughd@google.com> wrote: > > > > On Mon, 16 Nov 2020, akpm@linux-foundation.org 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@163.com> > > > 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. > > Thanks for that. I have also bisected my boot failures to that commit > and so have reverted it from linux-next today. > > -- > Cheers, > Stephen Rothwell Sorry for not full testing the change. Please ignore this change. Thanks. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: + mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode.patch added to -mm tree 2020-11-17 6:17 ` + mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode.patch added to -mm tree Hugh Dickins 2020-11-17 7:12 ` Stephen Rothwell @ 2020-11-20 19:32 ` Hui Su 1 sibling, 0 replies; 4+ messages in thread From: Hui Su @ 2020-11-20 19:32 UTC (permalink / raw) To: Hugh Dickins, Andrew Morton, Stephen Rothwell Cc: hughd, mm-commits, sh_def, linux-kernel, linux-mm On Mon, Nov 16, 2020 at 10:17:20PM -0800, Hugh Dickins wrote: > On Mon, 16 Nov 2020, akpm@linux-foundation.org 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@163.com> > > 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.) > Sorry, I thought it was a small change before, so I forgot to test it. It's SO STUPID. I'm really sorry. > > > > Link: https://lkml.kernel.org/r/20201115174026.GA365412@rlk > > Signed-off-by: Hui Su <sh_def@163.com> > > NAK. Hui Su, please test your "cleanups" before sending them. > I will remember it, thanks. I have spent two evenings building a patch automated test platform using qemu and jenkins. > 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 > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-20 19:33 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20201116230526.NkEfmeDCH%akpm@linux-foundation.org> 2020-11-17 6:17 ` + mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode.patch added to -mm tree Hugh Dickins 2020-11-17 7:12 ` Stephen Rothwell 2020-11-17 15:31 ` Hui Su 2020-11-20 19:32 ` Hui Su
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).