All of lore.kernel.org
 help / color / mirror / Atom feed
From: Viresh Kumar <viresh.kumar@linaro.org>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 2/2] syscalls/pidfd_open*.c: Drop .min_kver flag
Date: Tue, 5 May 2020 09:05:07 +0530	[thread overview]
Message-ID: <20200505033507.oerhoby6aif3leea@vireshk-i7> (raw)
In-Reply-To: <b74eac89-11fc-9a7c-c337-ffe6f19ec7b8@163.com>

On 04-05-20, 20:49, Xiao Yang wrote:
> Yes, It is unrelated change and just a small cleanup.
> 
> My commit message has mentioned it and I don't want to do the cleanup in
> seperate patch.

Removing usage of TEST() is not cleanup but just a choice of the
developer of how to write code, it wasn't my choice and I have been
asked to do it by maintainers, so removing it like that isn't correct.
If you want to do it, please send a separate patch and don't mix with
unrelated changes. There should be a separate patch normally for
different changes.

> > 
> > >   	TST_CHECKPOINT_WAKE(0);
> > > @@ -49,8 +47,14 @@ static void run(void)
> > >   		tst_res(TPASS, "pidfd_open() passed");
> > >   }
> > > +static void setup(void)
> > > +{
> > > +	// Check if pidfd_open(2) is not supported
> > > +	tst_syscall(__NR_pidfd_open, -1, 0);
> > > +}
> > > +
> > >   static struct tst_test test = {
> > > -	.min_kver = "5.3",
> > > +	.setup = setup,
> > >   	.test_all = run,
> > >   	.forks_child = 1,
> > >   	.needs_checkpoints = 1,
> > Please have a look at fsopen_supported_by_kernel() in lapi/fsmount.h
> > and make such a helper.
> 
> First, I want to explain my check point:
> 
> Passing invalid argument can check the support of pidfd_open(2) by ENOSYS
> errno and we don't need to close the pidfd.
> 
> Second, I don't like the implementation of fsopen_supported_by_kernel() and
> give some suggestions:
> 
> a) syscall()/tst_syscall() is enough to check the support of pidfd_open(2)
> and 'tst_kvercmp(5, 2, 0)) < 0' will skip the check
> 
> ??? if a kernel on distribution is newer than v5.2 but drop the support of
> pidfd_open(2) on purpose.

I don't think kernel can drop support of syscalls just like that, we
can't break user space. And if that is done, we need to fix userspace
again separately for that.

We came to the implementation of fsopen_supported_by_kernel() after a
lot of reviews and decided on that and so I asked you for the sake of
keeping similar code throughout LTP (of course there will be old
usages which are different) to have a similar implementation.

Anyway, I will leave it to Cyril to decide on that :)

-- 
viresh

  reply	other threads:[~2020-05-05  3:35 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-30  8:57 [LTP] [PATCH 1/2] syscalls/pidfd_open01.c: Add check for close-on-exec flag Xiao Yang
2020-04-30  8:57 ` [LTP] [PATCH 2/2] syscalls/pidfd_open*.c: Drop .min_kver flag Xiao Yang
2020-05-04  5:11   ` Viresh Kumar
2020-05-04 12:49     ` Xiao Yang
2020-05-05  3:35       ` Viresh Kumar [this message]
2020-05-05  9:30         ` Xiao Yang
2020-05-05  9:51           ` Viresh Kumar
2020-05-12 14:32           ` Xiao Yang
2020-06-12 14:30         ` Cyril Hrubis
2020-05-04  5:09 ` [LTP] [PATCH 1/2] syscalls/pidfd_open01.c: Add check for close-on-exec flag Viresh Kumar
2020-05-04  8:30   ` =?unknown-8bit?b?5p2o5pmT?=
2020-05-04 11:32     ` Xiao Yang
2020-05-04 11:31   ` Xiao Yang
2020-05-05  3:28     ` Viresh Kumar
2020-05-05  8:44       ` Xiao Yang
2020-05-12 14:25         ` Xiao Yang
2020-06-12 14:24       ` Cyril Hrubis

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=20200505033507.oerhoby6aif3leea@vireshk-i7 \
    --to=viresh.kumar@linaro.org \
    --cc=ltp@lists.linux.it \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.