From: Sahil <icegambit91@gmail.com>
To: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
Quentin Monnet <quentin@isovalent.com>
Cc: martin.lau@linux.dev, eddyz87@gmail.com, song@kernel.org,
yonghong.song@linux.dev, john.fastabend@gmail.com,
kpsingh@kernel.org, sdf@google.com, haoluo@google.com,
jolsa@kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next] bpftool: Mount bpffs on provided dir instead of parent dir
Date: Sat, 02 Mar 2024 01:58:07 +0530 [thread overview]
Message-ID: <3290360.44csPzL39Z@valdaarhun> (raw)
In-Reply-To: <d61e8537-e291-434c-b401-2b020b2b610d@isovalent.com>
Hi,
On Thursday, February 29, 2024 8:29:07 PM IST Quentin Monnet wrote:
> [...]
> Perhaps it would be clearer to split the logics of mount_bpffs_for_pin()
> into two subfunctions, one for directories, one for file paths. This way
> we would avoid to call malloc() and dirname() when "name" is already a
> directory, and it would be easier to follow the different cases.
>
I was working on these changes here, and I have got a question. In the
description of the github issue [1], one scenario is when the given directory
does not exist but its parent directory is bpffs. In this scenario no mounting
should be done.
But to check whether the parent dir is bpffs, the malloc and dirname will still
have to be done.
In the file subfunction too, the malloc and dirname will have to be done if the
given file does not already exist.
If my understanding above is right, should the mount_bpffs_for_pin() function
still be split?
Assuming that the function is split into two subfunctions, there's another
question that I have got.
> if (is_dir && is_bpffs(name))
> return err;
The above condition was added in commit 2a36c26fe3b8 (patch submission [2]).
If the function is to be split into two subfunctions for dirs and files, is it ok to
remove the above function entirely in the file subfunction?
If "is_bpffs(name)" returns false, then that could imply that the file exists and its
parent dir is not bpffs, or that the file does not exist and no comment can be
made on the parent dir. In either case the malloc and dirname will have to be
done.
On the other hand if the file exists and is part of the bpffs then this condition
will allow the function to exit immediately without doing a malloc and dirname.
But this can be determined without the condition as well, since the file being
part of the bpffs implies that the dir will be bpffs.
Thanks,
Sahil
[1] https://github.com/libbpf/bpftool/issues/100
[2] https://lore.kernel.org/bpf/1683197138-1894-1-git-send-email-yangpc@wangsu.com/
next prev parent reply other threads:[~2024-03-01 20:28 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-29 13:05 [PATCH bpf-next] bpftool: Mount bpffs on provided dir instead of parent dir Sahil Siddiq
2024-02-29 14:59 ` Quentin Monnet
2024-02-29 19:50 ` Sahil
2024-03-01 20:28 ` Sahil [this message]
2024-03-04 13:50 ` Quentin Monnet
2024-03-04 20:34 ` Sahil
2024-03-05 10:35 ` Quentin Monnet
2024-03-07 20:12 ` Sahil
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=3290360.44csPzL39Z@valdaarhun \
--to=icegambit91@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=martin.lau@linux.dev \
--cc=quentin@isovalent.com \
--cc=sdf@google.com \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/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