Re: [linux-next PATCH] rapidio: Fix error handling path

From: John Hubbard
Date: Fri Sep 18 2020 - 02:33:25 EST


On 9/17/20 7:21 PM, Souptick Joarder wrote:
On Thu, Sep 17, 2020 at 11:17 PM John Hubbard <jhubbard@xxxxxxxxxx> wrote:
...
I sort of feel like returning partial successes is not working. We
could easily make a wrapper which either pins everything or it returns
an error code.

Yes we could. And I have the same feeling about this API. It's generated a
remarkable amount of bug fixes, several of which ended up being partial or
wrong in themselves. And mostly this is due to the complicated tristate
return code: instead of 0 or -ERRNO, it also can return "N pages that is
less than what you requested", and there are no standard helpers in the kernel
to make that easier to deal with

There was some discussion on removing return value 0 from one of the
gup variants [1].
I think it might be partially relevant to the current discussion.

[1] https://patchwork.kernel.org/patch/11529795/


Yes, although as I mentioned above, I'm thinking of a 0 or -ERRNO return value,
and not even return nr_pages at all.

But in any case, as a practical matter, I'm not sure if it's a good idea to
actually change all the callsites, or not. If we just fix the remaining buggy
callers, maybe that's better than the churn associated with another API change.

On the other-other hand, there does seem to be more churn coming anyway, with
talk of actually doing a [get|pin]_user_bvec(), for example. So maybe it's better
to head off the coming mess.

This is something that should be discussed on linux-mm.



I guess the question is are there drivers which will keep working (or limp
along?) on partial pins? A quick search of a driver I thought did this does
not apparently any more... So it sounds good to me from 30,000 feet! :-D

It sounds good to me too--and from just a *few hundred feet* (having touched most
of the call sites at some point)! haha :)

I think the wrapper should be short-term, though, just until all the callers
are converted to the simpler API. Then change the core gup/pup calls to the simpler
API. There are more than enough gup/pup API entry points as it is, that's for sure.


thanks,
--
John Hubbard
NVIDIA

thanks,
--
John Hubbard
NVIDIA