On Sun, 26 May 2024 at 12:27, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> Not really. The real reason is different - there is a constraint on
> possible values of struct fd. No valid instance can ever have NULL
> file and non-zero flags.
>
> The usual pattern is this:
[ snip snip ]
Ugh. I still hate it, including your new version. I suspect it will
easily generate the extra test at fd_empty() time, and your new
version would instead just move that extra test at fdput() time
instead.
Hopefully in most cases the compiler sees the previous test for
fd.file, realizes the new test is unnecessary and optimizes it away.
Except we most definitely pass around 'struct fd *' in some places (at
least ovlfs), so I doubt that will be the case everywhere.
What would make more sense is if you make the "fd_empty()" test be
about the _flags_, and then both the fp_empty() test and the test
inside fdput() would be testing the same things.
Sadly, we'd need another bit in the flags. One option is easy enough -
we'd just have to make 'struct file' always be 8-byte aligned, which
it effectively always is.
Or we'd need to make the rule be that FDPUT_POS_UNLOCK only gets set
if FDPUT_FPUT is set.
Because I think we could have even a two-bit tag value have that "no fd" case:
00 - no fd
01 - fd but no need for fput
10 - fd needs fput
11 - fd needs pos unlock and fput
but as it is, that's not what we have. Right now we have
00 - no fd or fd with no need for fput ("look at fd.file to decide")
01 - fd needs fput
10 - fd pos unlock but no fput
11 - fd pos unlock and fput
but that 10 case looks odd to me. Why would we ever need a pos unlock
but no fput? The reason we don't need an fput is that we're the only
thread that has access to the file pointer, but that also implies that
we shouldn't need to lock the position.
So now I've just confused myself. Why *do* we have that 10 pattern?
Adding a separate bit would certainly avoid any complexity, and then
you'd have "flags==0 means no file pointer" and the "fd_empty()" test
would then make the fdput) test obviously unnecessary in the usual
pattern.
Linus
