From: Hui Su <sh_def@163.com>
To: Hugh Dickins <hughd@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
Stephen Rothwell <sfr@canb.auug.org.au>
Cc: hughd@google.com, mm-commits@vger.kernel.org, sh_def@163.com,
linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: + mm-shmem-use-kmem_cache_zalloc-in-shmem_alloc_inode.patch added to -mm tree
Date: Sat, 21 Nov 2020 03:32:46 +0800 [thread overview]
Message-ID: <20201120193246.GA19677@rlk> (raw)
In-Reply-To: <alpine.LSU.2.11.2011162149210.1082@eggly.anvils>
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
>
prev parent reply other threads:[~2020-11-20 19:34 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[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 message]
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=20201120193246.GA19677@rlk \
--to=sh_def@163.com \
--cc=akpm@linux-foundation.org \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mm-commits@vger.kernel.org \
--cc=sfr@canb.auug.org.au \
/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;
as well as URLs for NNTP newsgroup(s).