From: Doug Ledford <dledford@redhat.com>
To: Leon Romanovsky <leon@kernel.org>
Cc: Chuck Lever <chuck.lever@oracle.com>,
Michal Kalderon <Michal.Kalderon@cavium.com>,
linux-rdma <linux-rdma@vger.kernel.org>
Subject: Re: qedr memory leak report
Date: Sat, 31 Aug 2019 13:17:05 -0400 [thread overview]
Message-ID: <ee70c5983baee6ed42bd74b7b3dacfdb8bd035af.camel@redhat.com> (raw)
In-Reply-To: <20190831151945.GJ12611@unreal>
[-- Attachment #1: Type: text/plain, Size: 2041 bytes --]
On Sat, 2019-08-31 at 18:19 +0300, Leon Romanovsky wrote:
> On Sat, Aug 31, 2019 at 10:33:13AM -0400, Doug Ledford wrote:
> > On Sat, 2019-08-31 at 10:30 +0300, Leon Romanovsky wrote:
> > > Doug,
> > >
> > > I think that it can be counted as good example why allowing memory
> > > leaks
> > > in drivers (HNS) is not so great idea.
> >
> > Crashing the machine is worse.
>
> The problem with it that you are "punishing" whole subsystem
> because of some piece of crap which anyway users can't buy.
No I'm not. The patch in question was in the hns driver and only leaked
resources assigned to the hns card when the hns card timed out in
freeing those resources. That doesn't punish the entire subsystem, it
only punishes the users of that card, and then only if the card has
flaked out.
> If HNS wants to have memory leaks, they need to do it outside
> of upstream kernel.
Nope.
> In general, if users buy shitty hardware, they need to be ready
> to have kernel panics too. It works with faulty DRAM where kernel
> doesn't hide such failures, so don't see any rationale to invent
> something special for ib_device.
What you are advocating for is not "shitty DRAM crashing the machine",
you are advocating for "having ECC DRAM and then intentionally turning
the ECC off and then crashing the machine". Please repeat after me: WE
DONT CRASH MACHINES. PERIOD. If it is avoidable, we avoid it. That's
why BUG_ONs have to go and why they piss Linus off so much. If you
crash the machine, people are left scratching their head and asking why.
If you don't crash the machine, they have a chance to debug the issue
and resolve it. The entire idea that you are advocating for crashing
the machine as being preferable to leaking a few resources is ludicrous.
WE DONT CRASH MACHINES. PERIOD. Please repeat that until it fully
sinks in.
> Thanks
--
Doug Ledford <dledford@redhat.com>
GPG KeyID: B826A3330E572FDD
Fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2019-08-31 17:17 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-30 18:03 qedr memory leak report Chuck Lever
2019-08-30 18:27 ` Chuck Lever
2019-08-31 7:30 ` Leon Romanovsky
2019-08-31 14:33 ` Doug Ledford
2019-08-31 15:19 ` Leon Romanovsky
2019-08-31 17:17 ` Doug Ledford [this message]
2019-08-31 18:55 ` Leon Romanovsky
2019-09-02 7:53 ` [EXT] " Michal Kalderon
2019-09-03 12:53 ` Chuck Lever
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=ee70c5983baee6ed42bd74b7b3dacfdb8bd035af.camel@redhat.com \
--to=dledford@redhat.com \
--cc=Michal.Kalderon@cavium.com \
--cc=chuck.lever@oracle.com \
--cc=leon@kernel.org \
--cc=linux-rdma@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;
as well as URLs for NNTP newsgroup(s).