c4rc4s has joined #spectrum
cole-h has quit [Ping timeout: 240 seconds]
zgrep has joined #spectrum
pastbytes has quit [Quit: Leaving]
<
qyliss>
would anybody like to help me out by testing something?
<
qyliss>
compile and run the t_name_to_handle_at.c example in name_to_handle_at(2) (with a path as the argument)
<
qyliss>
let me know if it works or not
<
qyliss>
non-NixOS data points are probably especially helpful, but also it could be something to do with the particular kernel I'm running
<
qyliss>
bet it isn't supported on ZFS
<
Profpatsch>
qyliss: can you provide a .nix? e.g. via writers.writeC
<
qyliss>
Profpatsch: it's okay, I figured it out :{
<
qyliss>
I'm hacking on strace today, because I keep running into VMM-specific ioctls and stuff it doesn't know how to decode
<
qyliss>
and not having strace slows me down a lot
<
qyliss>
so i've decided it's time to just fix strace, rather than hacking around it
<
lukegb>
strace has a .io domain? huh.
hooway has joined #spectrum
TheJollyRoger has quit [Ping timeout: 240 seconds]
TheJollyRoger has joined #spectrum
hooway_ has joined #spectrum
hooway has quit [Read error: Connection reset by peer]
cole-h has joined #spectrum
<
puck>
<qyliss> bet it isn't supported on ZFS <- it .. should be?
<
qyliss>
puck: oh yeah lol the situation has developed from there
<
puck>
iinteresting
<
puck>
i saw the mailing list link
<
qyliss>
oh yeah more since there
<
qyliss>
this is wild
<
qyliss>
you're gonna love it
<
qyliss>
puck: offsetof there is returning some huge number
<
qyliss>
one time I got 1242447564
<
qyliss>
but it varies by the build
<
qyliss>
need to wait for a kernel to build before I can investigate further
<
puck>
hrm, offsetof ~ ((size_t)&(((s *)0)->m)))
<
puck>
or, more clearly, offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
<
qyliss>
I'm wondering if one of those names is getting shadowed
<
puck>
qyliss: i think it is trying to resolve .. &((fid_t *)0)->un._fid.data)
<
puck>
which should be .. approx 10?
<
puck>
the issue is:
<
qyliss>
how are you so damn fast
<
puck>
how does ((size_t) &((fid_t *)0)->un._fid.data)) parenthesize
<
puck>
my guess, for the lulz, is
<
qyliss>
I did wonder if it would be something like that
<
puck>
(&((fid_t *)0)->un)._fid.data wouldn't parse
<
puck>
hrm, clang properly turns it into 2..
<
puck>
qyliss: oh, red herring maybe
<
puck>
qyliss: the manpage example is very slightly broken, compounded by a bug in the ZFS code
<
puck>
oh, no, the manpage isn't broken
<
puck>
but the code only sets max_len if max_len > 0
<
puck>
because it errors out early if it can't even fit the fid header
<
qyliss>
puck: I have:
<
qyliss>
if (len_bytes < offsetof(fid_t, fid_data)) {
<
qyliss>
pr_warn("len_bytes: %d, offset: %d", offsetof(fid_t, fid_data));
<
qyliss>
and the pr_warn gets hit, with len_bytes = 2
<
puck>
because you are missing a printf argument
<
puck>
len_bytes must be a multiple of 4
<
puck>
because it is *max_len times sizeof(u32)
<
qyliss>
how did my build not fail >:(
<
qyliss>
I guess because printk is a custom thingy
<
puck>
because it doesn't have the printf-alike attribute probably
<
puck>
i think if the buffer can't fit the file pointer, you have to stack-allocate one and use that to measure the output size?
<
puck>
something like that
<
qyliss>
so should ZFS be multiplying by 4?
<
puck>
the issue is probably that zfs_fid needs a buffer to store the size in
<
puck>
and if you pass 0, well, then len_bytes won't fit :p
<
puck>
it still has to calculate the size of the FID
<
qyliss>
puck: oh, because > On error @max_len contains the minimum
<
qyliss>
* size(in 4 byte unit) needed to encode the file handle.
<
puck>
this was not caught in 10 years of this code existing
<
qyliss>
i mean who uses name_to_handle_at
<
qyliss>
hmm, would expect nfs to be backed by zfs quite a bit
<
qyliss>
maybe not on linux I guess?
<
puck>
i think this is just an edge case that nfs doesn't hit
<
puck>
qyliss: yeah, the nfs file handle probably isn't ever 0
<
puck>
so it doesn't hit the edge case
<
qyliss>
hmm, this might be a bit tricky to fix
<
qyliss>
wait never mind
<
qyliss>
I guess zfs_fid/zfsctl_fid must be what actually calculate the fid size?
<
qyliss>
because otherwise all this does is subtract the offset then add it back again
<
puck>
this interestingly means that fid_t* might not fully exist
<
qyliss>
that's a bit unfortunate, means the fix isn't going to be very clean
<
puck>
even funnier
<
puck>
the FID size is hard-coded
<
puck>
in that function
<
puck>
or well, in zfsctl_fid and zfs_fid
<
puck>
i thought it would be dynamically sized!
<
qyliss>
zfid->zf_len = SHORT_FID_LEN;, yeah mean?
<
puck>
hrmmm. thou this adds a question
<
qyliss>
i hate this code
<
qyliss>
why is there so much aliasing going on
<
puck>
when do you have a LONG_FID_LEN
<
puck>
snapshot dirs?!
<
qyliss>
zfsctl_snapdir_fid... yes
<
qyliss>
so I do have to actually calculate the fid size
<
puck>
this is because of an NFSv2 protocol limitation, lmao
<
puck>
and yeah, this is not great quality code
hooway_ has quit [Quit: Gone fishing.]
cole-h has quit [Quit: Goodbye]
cole-h has joined #spectrum
cole-h has quit [Quit: Goodbye]
cole-h has joined #spectrum
<
qyliss>
oh, Linux uses -fno-strict-aliasing
<
qyliss>
that's going to make this a lot easier
<
qyliss>
actually it wouldn't have been valid before otherwise I guess
<
qyliss>
puck: so NFS just knows ahead of time how big the handle needs to be, you think?
<
qyliss>
and that's why it doesn't hit this bug? the code path where the handle isn't big enough is just never hit?
<
qyliss>
I really wish people didn't put CONTRIBUTING files in .github
<
qyliss>
it's not just GitHub that's going to be looking for that file
<
qyliss>
well, I rebooted to test my ZFS patch, and now running the test program freezes my computer
<
qyliss>
puck: ughh, zfs_fid() does not check the fid length
<
qyliss>
only zfsctl_fid
<
qyliss>
that was a mildly annoying way to spend an afternoon
<
qyliss>
(and evening)
mvnetbiz_ has joined #spectrum
Noxert has joined #spectrum
Noxert has quit [Ping timeout: 252 seconds]