On Sat, Sep 19, 2020 at 09:13:17PM -0700, John Hubbard wrote: > On 9/19/20 8:03 PM, Souptick Joarder wrote: > > On Thu, Sep 17, 2020 at 1:11 PM Dan Carpenter <dan.carpen...@oracle.com> > > wrote: > > > On Wed, Sep 16, 2020 at 11:57:06PM -0700, John Hubbard wrote: > > > > As suggested by Dan Carpenter, fortify unpin_user_pages() just a bit, > > > > against a typical caller mistake: check if the npages arg is really a > > > > -ERRNO value, which would blow up the unpinning loop: WARN and return. > > > > > > > > If this new WARN_ON() fires, then the system *might* be leaking pages > > > > (by leaving them pinned), but probably not. More likely, gup/pup > > > > returned a hard -ERRNO error to the caller, who erroneously passed it > > > > here. > ... > > > > Do we need a similar check inside unpin_user_pages_dirty_lock(), > > when make_dirty set to false ? > > > Maybe not. This call is rarely if ever used for error handling, but > rather, for finishing up a successful use of the pages. > > There is a balance between protecting against buggy callers and just > fixing any buggy callers. There is also a limit to how much code one can > write in hopes of avoiding bugs in...code that one writes. :) Which is > why static analysis, unit and regression tests, code reviews are > important too. > > Here, I submit that that we're about to cross the line and go too far. > But if you have any examples of buggy callers for > unpin_user_pages_dirty_lock(), that might shift the line.
I checked for buggy uses of unpin_user_pages_dirty_lock() using Smatch and didn't find anything. (Which doesn't mean that there aren't any). regards, dan carpenter
