On Fri 28-09-18 20:12:33, John Hubbard wrote:
> static inline void release_user_pages(struct page **pages,
> - unsigned long npages)
> + unsigned long npages,
> + bool set_dirty)
> {
> - while (npages)
> - put_user_page(pages[--npages]);
> + if (set_dirty)
> + release_user_pages_dirty(pages, npages);
> + else
> + release_user_pages_basic(pages, npages);
> +}
Is there a good reason to have this with set_dirty argument? Generally bool
arguments are not great for readability (or greppability for that matter).
Also in this case callers can just as easily do:
if (set_dirty)
release_user_pages_dirty(...);
else
release_user_pages(...);
And furthermore it makes the code author think more whether he needs
set_page_dirty() or set_page_dirty_lock(), rather than just passing 'true'
and hoping the function magically does the right thing for him.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
