mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: John Hubbard <jhubbard@nvidia.com>
To: Souptick Joarder <jrdr.linux@gmail.com>
Cc: Ira Weiny <ira.weiny@intel.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	<mporter@kernel.crashing.org>, <alex.bou9@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	<gustavoars@kernel.org>, <madhuparnabhowmik10@gmail.com>,
	<linux-kernel@vger.kernel.org>,
	"Matthew Wilcox" <willy@infradead.org>
Subject: Re: [linux-next PATCH] rapidio: Fix error handling path
Date: Thu, 17 Sep 2020 23:33:19 -0700	[thread overview]
Message-ID: <c6719871-6dda-9ec1-936d-c9afd6d45cac@nvidia.com> (raw)
In-Reply-To: <CAFqt6zYT83JrkdnYx03+KbRzZ1wMcV0mNOUNmmtWg7hdr-B+1g@mail.gmail.com>

On 9/17/20 7:21 PM, Souptick Joarder wrote:
> On Thu, Sep 17, 2020 at 11:17 PM John Hubbard <jhubbard@nvidia.com> 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

  reply	other threads:[~2020-09-18  6:33 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-16  3:42 Souptick Joarder
2020-09-16  6:37 ` John Hubbard
2020-09-16 10:02 ` Dan Carpenter
2020-09-16 10:07   ` Dan Carpenter
2020-09-16 15:16   ` Ira Weiny
2020-09-16 15:27     ` Dan Carpenter
2020-09-17  6:57   ` [PATCH] mm/gup: protect unpin_user_pages() against npages==-ERRNO John Hubbard
2020-09-17  7:40     ` Dan Carpenter
2020-09-20  3:03       ` Souptick Joarder
2020-09-20  4:13         ` John Hubbard
2020-09-21  9:34           ` Dan Carpenter
2020-09-17 12:39   ` [linux-next PATCH] rapidio: Fix error handling path Dan Carpenter
2020-09-17 17:34     ` Ira Weiny
2020-09-17 17:47       ` John Hubbard
2020-09-18  2:21         ` Souptick Joarder
2020-09-18  6:33           ` John Hubbard [this message]
2020-09-18  2:25     ` Souptick Joarder
2020-09-18  6:15       ` Dan Carpenter
2020-09-16 15:20 ` Ira Weiny

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c6719871-6dda-9ec1-936d-c9afd6d45cac@nvidia.com \
    --to=jhubbard@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex.bou9@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=gustavoars@kernel.org \
    --cc=ira.weiny@intel.com \
    --cc=jrdr.linux@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=madhuparnabhowmik10@gmail.com \
    --cc=mporter@kernel.crashing.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®