linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Vernet <void@manifault.com>
To: Markus Elfring <Markus.Elfring@web.de>
Cc: kernel-janitors@vger.kernel.org, linux-kselftest@vger.kernel.org,
	cgroups@vger.kernel.org, linux-mm@kvack.org,
	Jay Kamat <jgkamat@fb.com>, Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@kernel.org>,
	Muchun Song <muchun.song@linux.dev>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Shakeel Butt <shakeelb@google.com>, Shuah Khan <shuah@kernel.org>,
	Tejun Heo <tj@kernel.org>, Zefan Li <lizefan.x@bytedance.com>,
	Lorenzo Stoakes <lstoakes@gmail.com>,
	cocci@inria.fr, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] selftests: cgroup: Fix exception handling in test_memcg_oom_group_score_events()
Date: Mon, 27 Mar 2023 04:13:27 -0500	[thread overview]
Message-ID: <20230327091327.GK363182@maniforge> (raw)
In-Reply-To: <c46dbb48-259b-1de9-2364-9bfaf1061944@web.de>

On Mon, Mar 27, 2023 at 07:56:03AM +0200, Markus Elfring wrote:
> >> 2. Can a cg_destroy() call ever work as expected if a cg_create() call failed?
> >
> > Perhaps next time you can answer your own question by spending 30
> > seconds actually reading the code you're "fixing":
> >
> > int cg_destroy(const char *cgroup)
> > {
> …
> >         ret = rmdir(cgroup);
> …
> >         if (ret && errno == ENOENT) <<< that case is explicitly handled here
> >                 ret = 0;
> >
> >         return ret;
> > }
> 
> Is it interesting somehow that a non-existing directory (which would occasionally
> not be found) is tolerated so far?
> https://elixir.bootlin.com/linux/v6.3-rc3/source/tools/testing/selftests/cgroup/cgroup_util.c#L285
> 
> Should such a function call be avoided because of a failed cg_create() call?

The point is that (a) you were wrong that this is fixing anything, and
(b) this patch is functionally useless. Sure, we could move some goto's
around and subjectively improve "something". Why?  What's the point?
It's highly debatable that what you're doing is even an improvement, and
I'm not interested in wasting time pontificating about the merits of a
trivial "fix" for a test cleanup function that isn't even broken.

Several people have already either advised or directly asked you to stop
sending these patches. I'm not sure why you're choosing to ignore them,
but I'll throw my hat in the ring regardless and do the same. Please
stop sending these fake cleanup patches.


      reply	other threads:[~2023-03-27  9:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <f9303bdc-b1a7-be5e-56c6-dfa8232b8b55@web.de>
2023-03-23 17:30 ` [PATCH] mm/mempolicy: Fix exception handling in shared_policy_replace() Markus Elfring
2023-03-24 17:30   ` Vlastimil Babka
2023-03-24 18:03     ` Markus Elfring
2023-03-25 18:30 ` [PATCH] selftests: cgroup: Fix exception handling in test_memcg_oom_group_score_events() Markus Elfring
2023-03-25 19:24   ` Lorenzo Stoakes
2023-03-26  8:15     ` Markus Elfring
2023-03-26 21:39       ` David Vernet
2023-03-27  5:56         ` Markus Elfring
2023-03-27  9:13           ` David Vernet [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=20230327091327.GK363182@maniforge \
    --to=void@manifault.com \
    --cc=Markus.Elfring@web.de \
    --cc=cgroups@vger.kernel.org \
    --cc=cocci@inria.fr \
    --cc=hannes@cmpxchg.org \
    --cc=jgkamat@fb.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lizefan.x@bytedance.com \
    --cc=lstoakes@gmail.com \
    --cc=mhocko@kernel.org \
    --cc=muchun.song@linux.dev \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeelb@google.com \
    --cc=shuah@kernel.org \
    --cc=tj@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