From: Stanislav Fomichev <sdf@fomichev.me>
To: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Stanislav Fomichev <sdf@google.com>,
netdev@vger.kernel.org, linux-kselftest@vger.kernel.org,
ast@kernel.org, daniel@iogearbox.net, shuah@kernel.org,
quentin.monnet@netronome.com, guro@fb.com,
jiong.wang@netronome.com, bhole_prashant_q7@lab.ntt.co.jp,
john.fastabend@gmail.com, jbenc@redhat.com,
treeze.taeung@gmail.com, yhs@fb.com, osk@fb.com,
sandipan@linux.vnet.ibm.com
Subject: Re: [PATCH bpf-next 2/3] libbpf: cleanup after partial failure in bpf_object__pin
Date: Wed, 7 Nov 2018 15:25:16 -0800 [thread overview]
Message-ID: <20181107232516.qiy24ccbq3hh4ail@mini-arch> (raw)
In-Reply-To: <20181107150605.59a8ac1c@cakuba.netronome.com>
On 11/07, Jakub Kicinski wrote:
> On Wed, 7 Nov 2018 15:00:21 -0800, Stanislav Fomichev wrote:
> > > > +err_unpin_programs:
> > > > + bpf_object__for_each_program(prog, obj) {
> > > > + char buf[PATH_MAX];
> > > > + int len;
> > > > +
> > > > + len = snprintf(buf, PATH_MAX, "%s/%s", path,
> > > > + prog->section_name);
> > > > + if (len < 0)
> > > > + continue;
> > > > + else if (len >= PATH_MAX)
> > > > + continue;
> > > > +
> > > > + unlink(buf);
> > >
> > > I think that's no bueno, if pin failed because the file already exists
> > > you'll now remove that already existing file.
> >
> > How about we check beforehand and bail early if we are going to
> > overwrite something?
>
> Possible, although the most common way to handle situation like this in
> the kernel is to "continue the iteration in reverse" over the list.
> I.e. walk the list back. I think the objects are on a double linked
> list. You may need to add the appropriate foreach macro and helper..
That sounds more complicated than just ensuring that the top directory
for the pins doesn't exist and then rm -rf it on failure.
I'm thinking about copy-pasting rm_rf from perf
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/util.c#n119).
Thoughts?
Btw, current patch won't work because of those /0 added by bpf_program__pin.
next prev parent reply other threads:[~2018-11-08 8:57 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-07 22:43 [PATCH v2 bpf-next 0/3] bpftool: support loading flow dissector Stanislav Fomichev
2018-11-07 22:43 ` [PATCH bpf-next 1/3] selftests/bpf: rename flow dissector section to flow_dissector Stanislav Fomichev
2018-11-07 22:43 ` [PATCH bpf-next 2/3] libbpf: cleanup after partial failure in bpf_object__pin Stanislav Fomichev
2018-11-07 22:56 ` Jakub Kicinski
2018-11-07 23:00 ` Stanislav Fomichev
2018-11-07 23:06 ` Jakub Kicinski
2018-11-07 23:25 ` Stanislav Fomichev [this message]
2018-11-07 23:38 ` Jakub Kicinski
2018-11-07 22:43 ` [PATCH bpf-next 3/3] bpftool: support loading flow dissector Stanislav Fomichev
2018-11-07 23:09 ` Jakub Kicinski
2018-11-07 23:13 ` Stanislav Fomichev
2018-11-07 23:29 ` Jakub Kicinski
2018-11-07 23:34 ` Stanislav Fomichev
2018-11-07 23:41 ` Jakub Kicinski
2018-11-08 0:40 ` Stanislav Fomichev
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=20181107232516.qiy24ccbq3hh4ail@mini-arch \
--to=sdf@fomichev.me \
--cc=ast@kernel.org \
--cc=bhole_prashant_q7@lab.ntt.co.jp \
--cc=daniel@iogearbox.net \
--cc=guro@fb.com \
--cc=jakub.kicinski@netronome.com \
--cc=jbenc@redhat.com \
--cc=jiong.wang@netronome.com \
--cc=john.fastabend@gmail.com \
--cc=linux-kselftest@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=osk@fb.com \
--cc=quentin.monnet@netronome.com \
--cc=sandipan@linux.vnet.ibm.com \
--cc=sdf@google.com \
--cc=shuah@kernel.org \
--cc=treeze.taeung@gmail.com \
--cc=yhs@fb.com \
/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