Re: [PATCH RFC v3 1/6] exterr: Introduce extended syscall error reporting

[Alexander Shishkin <alexander.shishkin@linux.intel.com>]

Jonathan Corbet <corbet@lwn.net> writes:

> So, a couple of comments as I try to figure this stuff out...

Thanks for taking the time to look at this.

> On Fri, 11 Sep 2015 19:00:00 +0300
> Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote:
>
>> +int ext_err_copy_to_user(void __user *buf, size_t size)
>> +{
>> +	struct ext_err_domain_desc *dom = ext_err_domain_find(current->ext_err_code);
>> +	struct ext_err_site *site = ext_err_site_find(current->ext_err_code);
>
> Here too you use ext_err_code without making sure it has a useful value.
> This will also result in two calls to ext_err_domain_find(), and, thus, a
> redundant search of the domains array.  Why not just pass dom to the second
> call?

Yes, this is a bit too much of a quick and dirty rfc (as you can
probably guess from the way it's written), you're absolutely right.

> [...]
>
>> +	if (!ret)
>> +		current->ext_err_code = 0;
>
> Back to this code: here is the only place you ever clear it, and this seems
> to be an error response in its own right.  So, if I read this correctly,
> once an extended error has been signalled, it will remain forever in the
> task state until another extended error overwrites it, right?

Yes, and this brings us to an important point (below).

> What that says to me is that there will be no way to know whether an error
> description returned from prctl() corresponds to an error just reported to
> the application or not; it could be an old error from last week.  That
> strikes me as being less useful than it could otherwise be.

It seems to make sense to allow the program to clear it (via a flag in
that prctl(), for example). That is, if we get an error, we try to fetch
the extended description, clear it and forget about it.

Then, this prctl() may be a part of the syscall wrapper (or a library
function that uses that syscall), which might or might not want to leave
the extended error code for the main program to inspect. Or a debugger
might call this prctl() for its debugging purposes, but still keep it
around for the main program.

> It seems to me that current->ext_err_code needs to be cleared on each
> system call entry (except for your special prctl() of course!).

I'd say, it should be up to the program to decide for how long they want
to keep the extended error code around.

Thanks,
--
Alex