On 12/11/20 2:51 PM, Al Viro wrote:
> On Fri, Dec 11, 2020 at 11:50:12AM -0700, Jens Axboe wrote:
>
>> I could filter on O_TRUNC (and O_CREAT) in the caller from the io_uring
>> side, and in fact we may want to do that in general for RESOLVE_LOOKUP
>> as well.
>
> You do realize that it covers O_RDWR as well, right? If the object is on
> a frozen filesystem, mnt_want_write() will block until the thing gets thawed.
I do, current patch does have that handled. I was only referring to the
fact that I don't consider O_TRUNC interesting enough to fold in non-block
support for it, I'm quite happy just letting that be as it is and just
disallow it in the flags directly.
>>> AFAICS, without that part it is pretty much worthless. And details
>>> of what you are going to do in the missing bits *do* matter - unlike the
>>> pathwalk side (which is trivial) it has potential for being very
>>> messy. I want to see _that_ before we commit to going there, and
>>> a user-visible flag to openat2() makes a very strong commitment.
>>
>> Fair enough. In terms of patch flow, do you want that as an addon before
>> we do RESOLVE_NONBLOCK, or do you want it as part of the core
>> LOOKUP_NONBLOCK patch?
>
> I want to understand how it will be done.
Of course. I'll post what I have later, easier to discuss an actual
series of patches.
>> Agree, if we're going bool, we should make it the more usually followed
>> success-on-false instead. And I'm happy to see you drop those
>> likely/unlikely as well, not a huge fan. I'll fold this into what I had
>> for that and include your naming change.
>
> BTW, I wonder if the compiler is able to figure out that
>
> bool f(void)
> {
> if (unlikely(foo))
> return false;
> if (unlikely(bar))
> return false;
> return true;
> }
>
> is unlikely to return false. We can force that, obviously (provide an inlined
> wrapper and slap likely() there), but...
Not sure, it _should_, but reality may differ with that guess.
--
Jens Axboe
